fix: clamp PktLen in CFE_MSG_ComputeCheckSum to MAX_SB_MSG_SIZE

[stark256-spec]

Fixes nasa/cFS#2699. CFE_MSG_GetSize() reads the loop bound from the attacker-controlled CCSDS Length field. Without an upper-bound check, while(PktLen--) iterates over an attacker-chosen number of bytes regardless of the actual buffer, enabling an OOB heap read. Clamps PktLen to CFE_MISSION_SB_MAX_SB_MSG_SIZE before the loop; a larger value indicates a malformed message and is treated as zero.

[stark256-spec]

Following up on this — it's been open a few weeks with no review yet. Happy to address any feedback on the approach. If it would help move this toward the next CCB cycle, let me know if CCB:Ready should be applied or if I should tag a specific reviewer from the SME list.

[sylvesterkaczmarek]

Clamping to CFE_MISSION_SB_MAX_SB_MSG_SIZE bounds the worst-case read, but it does not prove the claimed packet length fits the actual backing buffer. A short buffer whose CCSDS length says 1 KB (still below the mission max) can still make this loop read past the object. Could this validation happen where the real buffer capacity is known, or otherwise make that size guarantee explicit?

[stark256-spec]

You're right, and thanks for the careful read. Clamping to CFE_MISSION_SB_MAX_SB_MSG_SIZE only bounds the worst case to the mission maximum — it doesn't establish that the declared CCSDS length fits the actual backing buffer, so a shorter buffer whose length field reads, say, 1 KB (still under the max) would still over-read. And CFE_MSG_ComputeCheckSum takes only MsgPtr with no capacity argument, so it can't detect that itself — the "message already checked" precondition really is the caller's responsibility.

Tracing it: within cFE, ComputeCheckSum is reached via GenerateChecksum (outbound, where the app owns the buffer) and ValidateChecksum (a public API, only exercised by tests in-tree). For SB-delivered messages the size is already bounded at the SB layer (CFE_SB rejects > CFE_MISSION_SB_MAX_SB_MSG_SIZE), which makes this clamp largely redundant there and — as you note — insufficient for the smaller-buffer case, where the real guarantee has to come from wherever the command is received into a known-capacity buffer.

So rather than leave a bound that implies more safety than it provides, I'd suggest either: (a) drop the clamp and make the size precondition explicit in the ComputeCheckSum/ValidateChecksum contract, or (b) if we want a genuine per-buffer bound in this path, thread the buffer capacity to the validation point. Happy to implement whichever you and the maintainers prefer.