CF_CFDP_DecodeAllTlv reports success and drops TLVs after the fourth
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 77/100
- Issue type
- Bug
- Clarity
- Clearly specified
- Activity status
- Active
- Tech stack
- c
- Domain
- backend, embedded-iot
Research direction
Start with fsw/src/cf_codec.c, especially CF_CFDP_DecodeAllTlv and its callers CF_CFDP_DecodeEof and CF_CFDP_DecodeFin; the report identifies the loop condition that stops at the TLV limit. Check how decoder state and remaining bytes are handled when the limit is reached. Done means extra or malformed TLVs are not silently accepted, with a regression check for the reported cases.
Written by the indexing model from the issue text.
Description
Checklist (Please check before submitting)
- I reviewed the Contributing Guide.
- I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.
Describe the bug
CF_CFDP_DecodeAllTlv stops after limit (CF_PDU_MAX_TLV, 4) TLVs and returns with the decoder still OK, even when bytes remain. So an EOF or FIN with more than 4 TLVs decodes as successful with the extra TLVs silently dropped, and a malformed TLV after the fourth is never examined.
To Reproduce
Decoding FIN PDUs with cf_codec.c from dev (0dbc2b7; the file is the same on main) under ASan and UBSan:
| FIN with | Decoder OK | TLVs kept | Bytes left unread |
|---|---|---|---|
| 4 entity-ID TLVs | yes | 4 | 0 |
| 5 entity-ID TLVs | yes | 4 | 3 |
| 6 entity-ID TLVs | yes | 4 | 6 |
| 2 TLVs, second malformed (length 9, 1 byte present) | no | 1 | 1 |
| 5 TLVs, fifth malformed | yes | 4 | 3 |
Expected behavior
Either reject the PDU when TLV bytes remain after the limit (as the in-limit malformed case already is), or record that TLVs were dropped (an event or counter). The num_tlv >= CF_PDU_MAX_TLV branch inside the loop is never reached: both callers (CF_CFDP_DecodeEof and CF_CFDP_DecodeFin) pass CF_PDU_MAX_TLV as limit, so the loop condition stops first.
Code snips
fsw/src/cf_codec.c, CF_CFDP_DecodeAllTlv: while (limit > 0 && CF_CODEC_GET_REMAIN(state) != 0).
System observed on:
Linux x86_64, gcc, harness around cf_codec.c.
Additional context
Low impact today, since CF only uses the entity-ID (fault location) TLV. Found while comparing CF with F´'s CfdpManager, which has the same behavior (nasa/fprime#6330). CLA: cFS Apps corporate CLA filed by Space Ocean Corp.
- Dominant language
- C
- Stars
- 122
- Forks
- 81
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 4
Getting set up
- No Dockerfile or Docker Compose file
- Has a pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from nasa/CF
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
-
Assert checks non-existent variable (`pt`) in `CF_CFDP_ProcessPlaybackDirectory()`May be free again A pull request for this issue was closed without being merged. Openinvalid
Difficulty 1/5 Under an hour Newbie friendliness 68/100
-
Transaction priority sorted incorrectlyPossibly taken @dmknutsen claimed this 896 days ago. Openbug
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
-
CF_Chunks_Insert drops all data after the chunk list fills, so a lossy Class 2 upload is mostly resentPossibly taken @SpaceOceanCorp claimed this 3 days ago. Open
Difficulty 3/5 1-2 days Newbie friendliness 35/100
-
Static Code Analysis Declaration Not in Header WarningPossibly taken @arielswalker claimed this 25 days ago. Open
Similar issues
-
SPIDevice selects an active-high device as soon as it is createdPossibly taken @mikeysklar claimed this today. Open
Difficulty 2/5 1-3 hours Newbie friendliness 73/100
adafruit/circuitpython#11869 ·
Maintainers usually reply within 1 day
-
Difficulty 1/5 Under an hour Newbie friendliness 78/100
gnif/LookingGlass#1352 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 85/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 62/100
bazel-contrib/rules_go#4784 · 1 comment ·
Maintainers usually reply within 1 day