Conversation
CF_CFDP_R_ProcessFd() seeked to and wrote at the offset carried in a received File Data PDU without ever comparing that offset, or offset plus data length, against txn->fsize, the size the Metadata PDU declares. Nothing downstream notices: CF_CFDP_R_CalcCrcStart() only compares the MD-declared size against the EOF-declared size, and CF_CFDP_R_CheckComplete() and CF_CFDP_R_CalcCrcChunk() only look at the first txn->fsize bytes. A receiver retained a 2130706433-byte file for a transfer declared as 4 bytes and reported it successful. A peer controls PDU order, so the bound is applied in two places: * CF_CFDP_R_ProcessFd() rejects an FD PDU whose offset plus data length exceeds txn->fsize, once the MD PDU has been received. Before that the size is unknown, and file data legitimately arrives before metadata, which keeps class 1 and out-of-order class 2 reception working. It is written as a subtraction rather than a sum so offset plus length cannot wrap where CF_FileSize_t and size_t are the same width. * CF_CFDP_R_SubstateRecvMd() applies the same bound the moment the size becomes known, comparing the highest end-of-data offset already written against the declared txn->fsize. Without it, sending that File Data PDU before the Metadata PDU bypasses the bound entirely. The mark is kept in txn->state_data.recv_top, not derived from the chunk list, because CF_ChunkListAdd() drops entries once the list is full, and it saturates rather than wrapping. Both sites use the existing CF_TxnStatus_FILE_SIZE_ERROR fault path and file_size_mismatch counter, so the state machine tears the transaction down and removes the temporary file. One new event ID is added, CF_CFDP_R_FD_SIZE_ERR_EID (167). CF has no receive size limit, so file data that arrives before the Metadata PDU, or inside a large size the Metadata PDU declares, still produces a sparse temporary file until the channel inactivity timer expires. That is unchanged by this commit and is not addressed here.
2 tasks done
dzbaker
marked this pull request as ready for review
September 28, 2026 12:18
dzbaker
marked this pull request as draft
September 28, 2026 12:18
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist (Please check before submitting)
Describe the contribution
Fixes #520.
CF_CFDP_R_ProcessFd()seeked to and wrote at a received File Data PDU's offset without comparing that offset, or offset plus data length, againsttxn->fsize, the size the Metadata PDU declares. A transfer declaring 4 bytes retained a 2,130,706,433-byte file and reported success. The file is sparse where the filesystem supports it: 2.0G apparent, 8192 bytes on disk on tmpfs, so the impact is a wrong file length and a false success report, not the 4 GiB of storage exhaustion #520 claims. Bounding the offset closes the whole defect, soFixesis deliberate.The fix has two parts, because a peer controls PDU order.
CF_CFDP_R_ProcessFd()now rejects an FD PDU whose offset plus data length exceedstxn->fsize, once the MD PDU has been received.CF_CFDP_R_SubstateRecvMd()applies the same bound the moment the size becomes known, against a saturating high-water mark of what was written, the newtxn->state_data.recv_top; without it, sending that FD PDU first bypasses the bound.Testing performed
Four orderings, each with a benign 4-byte control over the same path, injected over
ci_labon UDP 1234 and decoded by the realCF_CFDP_ReceiveMessagepath. Run twice at each word size, against basea9c36d6and against this change.a9c36d60x7F000000files_recv+1,file_size_mismatch+0, at both word sizesfile_size_mismatch+1, EID 167, at both word sizes0xFFFFFF00failed to seek offset -256, onfile_seekfile_size_mismatch+1, EID 167, and at 32-bit the offset is reported correctlyCase B, before and after, same input:
One limit:
ctestis 14 of 14 with no new warnings,apps/cf/fsw/srchas zero missed lines and zero missed branches, andcf_cfdp_r.chas 100 percent condition coverage.Expected behavior changes
CF_TxnStatus_FILE_SIZE_ERRORpath,file_size_mismatchincrements and the file is not retained. Transfers within the declared size are unaffected.CF_CFDP_R_FD_SIZE_ERR_EID(167), and one new internal field,recv_topinCF_StateData_t, in no telemetry or table structure.file_size_mismatchhas had no writer infsw/sincea0914c4, so a quiet counter starts moving.System(s) tested on
a9c36d6ondev, cFE, OSAL and PSP from the cFS bundle atd195847. Both word sizes with ASan and UBSan, 32-bit (-m32) again with no sanitizer.Additional context
I will post that correction on #520. The cFS-Apps Individual CLA was emailed separately, so please hold the merge until you can see it.
Third party code
None.
Contributor Info - All information REQUIRED for consideration of pull request
Eva Crystal (0xiviel), XSource Security