Fix #277, Detect pdu truncation #310
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| */ | ||
| if (CF_CFDP_DecodeHeader(ph->pdec, &ph->pdu_header) != CFE_SUCCESS) | ||
| { | ||
| CFE_EVS_SendEvent(CF_EID_ERR_PDU_TRUNCATION, CFE_EVS_EventType_ERROR, |
Check warning
Code scanning / CodeQL-coding-standard
Unchecked return value
| { | ||
| CFE_EVS_SendEvent(CF_EID_ERR_PDU_TRUNCATION, CFE_EVS_EventType_ERROR, | ||
| "CF: pdu rejected due to eid/seq number field truncation"); | ||
| ++CF_AppData.hk.channel_hk[chan_num].counters.recv.error; |
Check warning
Code scanning / CodeQL-coding-standard
Unchecked function argument
| * are larger than the sizes configured in the cf platform config | ||
| * file, then reject the PDU. | ||
| */ | ||
| if (CF_CFDP_DecodeHeader(ph->pdec, &ph->pdu_header) != CFE_SUCCESS) |
Check warning
Code scanning / CodeQL-coding-standard
Side effect in a Boolean expression
| @@ -822,9 +822,10 @@ | |||
| * See description in cf_codec.h for argument/return detail | |||
| * | |||
| *-----------------------------------------------------------------*/ | |||
| void CF_CFDP_DecodeHeader(CF_DecoderState_t *state, CF_Logical_PduHeader_t *plh) | |||
| int32 CF_CFDP_DecodeHeader(CF_DecoderState_t *state, CF_Logical_PduHeader_t *plh) | |||
Check notice
Code scanning / CodeQL-coding-standard
Long function without assertion
| @@ -822,9 +822,10 @@ | |||
| * See description in cf_codec.h for argument/return detail | |||
| * | |||
| *-----------------------------------------------------------------*/ | |||
| void CF_CFDP_DecodeHeader(CF_DecoderState_t *state, CF_Logical_PduHeader_t *plh) | |||
| int32 CF_CFDP_DecodeHeader(CF_DecoderState_t *state, CF_Logical_PduHeader_t *plh) | |||
Check notice
Code scanning / CodeQL-coding-standard
Use of basic integral type
| { | ||
| const CF_CFDP_PduHeader_t *peh; /* for decoding fixed sized fields */ | ||
| int32 ret = CFE_SUCCESS; |
Check notice
Code scanning / CodeQL-coding-standard
Use of basic integral type
ghost
changed the title
|
Does anyone have an idea why the PR title check still fails on this request? I edited the PR title to add a comma, but it still says it fails with the old name. Is there anything I have to do outside of this page to edit that title? |
|
CCB 17 August 2022: Approved pending updates to new requirement:
|
Checklist (Please check before submitting)
Describe the contribution
Fixes #277
Detects and reports silent truncation of Entity ID or Transaction Sequence Number fields.
PDUs with EID/TSN fields that are too big for configured internal storage will be rejected and an event will be issued.
Testing performed
Updated and ran unit and coverage tests.
Tested with peer-to-peer CF setup with both correctly configured PDU fields were accepted, and PDUs with size mismatches were rejected rather than silently truncated.
Expected behavior changes
Now if the ground or a peer sends a PDU with either EID or TSN fields that are too big for the storage configured in the cf_platform_cfg.h file, the PDU will be rejected, the error counter will be incremented, and an event message will be issued. Before these PDUs would be silently accepted, and the error could depend on the data truncation that would occur.
System(s) tested on
Contributor Info - All information REQUIRED for consideration of pull request
Alan Cudmore, NASA/GSFC, Code 582.0