[Security] cfe/modules/msg — 4 novel findings (HIGH: memset OOB write + broken priext macro; MEDIUM: checksum OOB read driven by wire-controlled length)

[GaltRanch]

Summary

Four findings in cfe/modules/msg — the CCSDS message framing layer used by every cFS application for command and telemetry messages. Two are HIGH (one out-of-bounds write, one mission-build-config-dependent silently-broken header initializer), one MEDIUM (out-of-bounds READ in checksum walk driven by attacker-supplied wire length), one LOW (fragile arithmetic that is benign today but breaks under a foreseeable refactor).

The findings were produced by an LLM-driven source-aware audit of the same files KCode's deterministic pattern library considers clean (KCode skip-verify on cfe/modules/msg returned 0 candidates). None of the four are matchable by regex / AST / structural pattern rules — they require reading the function in context, tracing argument flow from caller, or recognizing a syntactic error inside a macro that no compiler warning catches because the macro itself is not currently exercised in the default build profile.

Severity

HIGH for findings #1 and #2; MEDIUM for finding #3; LOW for finding #4.

For finding #1, exploitability requires only that some caller of CFE_MSG_Init receive an attacker-influenced Size argument — a pattern that is easy to introduce in mission-specific applications (table-driven message sizing, command-driven allocations).

For finding #2, exploitability is gated by the mission's choice of header configuration (MSG_HDR_FILE). If the priext configuration is ever selected, the table commands declared with CFE_MSG_CMD_HDR_INIT may not compile, or — worse — silently emit incorrect headers depending on how the C compiler tolerates the malformed initializer.

For finding #3, exploitability requires an upstream caller that hands an unvalidated wire packet to CFE_MSG_ValidateChecksum. The function's comment ("Message already checked, no error case reachable") is itself the defect — the routine inherits trust from its caller without verifying it.

Affected component(s)

• cfe/modules/msg/fsw/src/cfe_msg_init.c:36 — CFE_MSG_Init
• cfe/modules/msg/option_inc/default_cfe_msg_hdr_priext.h:57-73 — CFE_MSG_CMD_HDR_INIT macro (priext build configuration)
• cfe/modules/msg/fsw/src/cfe_msg_sechdr_checksum.c:36-50 — CFE_MSG_ComputeCheckSum
• cfe/modules/msg/fsw/src/cfe_msg_ccsdsext.c:224 — CFE_MSG_GetSystem (also: cfe_msg_ccsdspri.c CFE_MSG_GetSize)

Details

#1 — cfe/modules/msg/fsw/src/cfe_msg_init.c:36 Out-of-bounds memset on attacker-influenced Size (HIGH, CWE-787)

CFE_Status_t CFE_MSG_Init(CFE_MSG_Message_t *MsgPtr, CFE_SB_MsgId_t MsgId, CFE_MSG_Size_t Size)
{
    int32 status;

    if (MsgPtr == NULL)
    {
        return CFE_MSG_BAD_ARGUMENT;
    }

    /* Clear and set defaults */
    memset(MsgPtr, 0, Size);                /* <-- (1) executes before Size validation */
    CFE_MSG_InitDefaultHdr(MsgPtr);

    /* Set values input */
    /* Allow "CFE_SB_INVALID_MSG_ID" here - in some cases the real msgid is filled in later */
    /* however the size should always be a valid value */
    status = CFE_MSG_SetSize(MsgPtr, Size);  /* <-- (2) Size enforced here, too late */
    if (status != CFE_SUCCESS)
    {
        return status;
    }
    /* ... */
}

Size is CFE_MSG_Size_t (an unsigned type wide enough to express the CCSDS Length field). At point (1) the function has not yet checked that Size is within the buffer pointed to by MsgPtr. At point (2) CFE_MSG_SetSize enforces a maximum of CFE_MSG_MAX_MESSAGE_SIZE (= 0xFFFF + 7 = 65 542 bytes) — but by then the memset has already executed using the caller's unvalidated value.

Exploit path. Any caller that derives Size from attacker-influenced input — a table load entry, a command-driven allocation length, a forwarded-from-network value — and passes it into CFE_MSG_Init without first clamping against the actual buffer size triggers an out-of-bounds write into adjacent memory. The memset zeroes the writes (so this is not a direct content-injection primitive), but the OOB region is fully under the attacker's control via Size, and the cleared region likely contains structured data (other components' state, function pointers in BSS / data layout) that is consequential when zeroed.

Fix.

if (Size > CFE_MSG_MAX_MESSAGE_SIZE)
{
    return CFE_MSG_BAD_ARGUMENT;
}
/* Clear and set defaults */
memset(MsgPtr, 0, Size);

The same maximum is enforced one call down inside CFE_MSG_SetSize; hoisting the check above the memset resolves the bug.

#2 — cfe/modules/msg/option_inc/default_cfe_msg_hdr_priext.h:57-73 Broken CFE_MSG_CMD_HDR_INIT macro under priext build configuration (HIGH, CWE-665 / CWE-682)

#define CFE_MSG_CMD_HDR_INIT(mid, size, fc, cksum)      \
    {                                                   \
    .Msg.CCSDS = \
    { \
        .Pri = \
        { \
            .StreamId = {(((mid)&0x80)<<5, (mid)&0x7F}, \  /* (a) parses as initializer */
            .Sequence = {0xC0, 0}, \
            .Length = {((size)-7)>>8, ((size)-7)&0xFF} \
        }, \
        .Ext = \
        { \
            .Subsystem = {0, ((mid)>>8)&0xFF}, \
            .SystemId = {0, 0) \                           /* (b) ← '{' closed with ')' */
        } \
    }, \
    CFE_MSG_CMD_HDR_SEC_INIT(fc, cksum) \
}

The .SystemId = {0, 0) initializer (line marked (b)) opens with { and closes with ). This is not legal C — array initializers must close with }. A compiler will either reject the expansion, or — depending on which preceding ( it pairs the rogue ) with — silently mis-parse the entire CFE_MSG_CMD_HDR_INIT invocation as a different expression, emitting a header whose fields are assigned to different members than the macro author intended.

mission_build.cmake:13 sets MSG_HDR_FILE "default_cfe_msg_hdr_priext.h" for the priext build profile. CFE_MSG_CMD_HDR_INIT is referenced from real command tables — apps/hs/fsw/tables/hs_mat.c:71,78,82,89,96,103,110,118, apps/sc/fsw/tables/sc_ats*.c, and others. Under the priext header profile these tables either fail to build, or produce tables with incorrect CCSDS StreamId / SystemId fields silently in the bytes the flight image ships.

Exploit path. Two failure modes:

1. Build-break footgun. A mission integrator selecting priext for an extended-header build (e.g. for fields such as Subsystem / SystemId) discovers the build fails. Time-cost only.
2. Silent mis-initialization. If the compiler tolerates the malformed initializer by mis-pairing the stray ) with a preceding (, the produced table commands carry wrong CCSDS field values. On the flight image, commands run with the wrong opcode bytes; on the ground, telemetry matching against opcode-by-table-position will silently disagree with the actual on-wire values. This is the harder failure to debug — the symptom is "commands work in pri builds, mysteriously misbehave in priext builds".

Fix. Replace the trailing ) with }:

-            .SystemId = {0, 0) \
+            .SystemId = {0, 0} \

While in the macro, also clean up the .StreamId initializer if a comma-operator-style initialization was intended (the current form {((mid)&0x80)<<5, (mid)&0x7F} is valid C — a 2-element array initialized to those two byte values — but worth double-checking it was the author's intent rather than a similar typo from the same change).

#3 — cfe/modules/msg/fsw/src/cfe_msg_sechdr_checksum.c:36-50 Out-of-bounds READ in checksum walk driven by wire-controlled length (MEDIUM, CWE-125)

CFE_MSG_Checksum_t CFE_MSG_ComputeCheckSum(const CFE_MSG_Message_t *MsgPtr)
{
    CFE_MSG_Size_t     PktLen  = 0;
    const uint8       *BytePtr = (const uint8 *)MsgPtr;
    CFE_MSG_Checksum_t chksum  = 0xFF;

    /* Message already checked, no error case reachable */
    CFE_MSG_GetSize(MsgPtr, &PktLen);

    while (PktLen--)
    {
        chksum ^= *(BytePtr++);
    }

    return chksum;
}

CFE_MSG_GetSize reads the CCSDS Length field from the message header (bytes 4-5 of the primary header) and returns Length + CFE_MSG_SIZE_OFFSET — a value taken straight from the wire. The function then walks PktLen bytes from MsgPtr, XORing each into the checksum.

There is no comparison between PktLen and the actual buffer size the caller is holding. The comment "Message already checked, no error case reachable" expresses an invariant that the routine does NOT enforce — it inherits it.

Exploit path. Any flow that delivers a wire packet to a CI subsystem which then calls CFE_MSG_ValidateChecksum (which in turn calls CFE_MSG_ComputeCheckSum) on the received buffer is at risk. An attacker sets the CCSDS Length field to declare a packet much larger than the actual receive buffer. The XOR loop walks past the buffer end, reading adjacent memory. The XOR itself is the result, so this is an information-disclosure side channel (via the checksum byte returned to the dispatcher and possibly logged) and a potential trigger for downstream OOB reads when the dispatcher treats the buffer as "PktLen bytes long".

Fix. Add an explicit upper bound parameter and check it inside the function:

CFE_MSG_Checksum_t CFE_MSG_ComputeCheckSum(const CFE_MSG_Message_t *MsgPtr, CFE_MSG_Size_t BufLen)
{
    CFE_MSG_Size_t PktLen = 0;
    CFE_MSG_GetSize(MsgPtr, &PktLen);
    if (PktLen > BufLen) PktLen = BufLen;        /* refuse to walk past caller's buffer */
    /* ... */
}

A non-breaking alternative is to clamp inside CFE_MSG_GetSize against CFE_MSG_MAX_MESSAGE_SIZE and require all CFE_MSG callers to provide a buffer at least that large. The breaking-but-correct fix is the explicit BufLen argument above.

#4 — cfe/modules/msg/fsw/src/cfe_msg_ccsdsext.c:224 Use of + instead of | in byte concatenation (LOW, CWE-682)

*System = (MsgPtr->CCSDS.Ext.SystemId[0] << 8) + MsgPtr->CCSDS.Ext.SystemId[1];

/* identical shape in cfe_msg_ccsdspri.c CFE_MSG_GetSize: */
*Size = (MsgPtr->CCSDS.Pri.Length[0] << 8) + MsgPtr->CCSDS.Pri.Length[1] + CFE_MSG_SIZE_OFFSET;

SystemId[0] and SystemId[1] are uint8. Integer promotion makes them int for the arithmetic. The shifted value 0x0000–0xFF00 plus the low byte 0x00–0xFF cannot exceed 0xFFFF, so today the + result equals the | result. The code is functionally correct.

The defect is the use of + to express what is structurally a bit concatenation. If any future patch widens the byte storage type (e.g. for an extended-Length variant of the CCSDS header), or hand-inlines this calculation for performance and the inlined version sign-extends, the + form becomes a real bug while | would still be correct.

Fix. Spelled correctly:

*System = ((uint16)MsgPtr->CCSDS.Ext.SystemId[0] << 8) | MsgPtr->CCSDS.Ext.SystemId[1];

This is a hygiene fix; no immediate security impact in the current code.

Reproduction notes

Findings #1, #3 are reproducible by reading the cited functions in their current form on the dev branch as of commit 4a1c553. Finding #4 is the same kind of read-the-line fact.

Finding #2 requires building the project with the priext header profile selected. To reproduce:

git clone https://github.com/nasa/cFE.git
cd cFE
cmake -B build -DMSG_HDR_FILE=default_cfe_msg_hdr_priext.h
make -C build apps/hs/CMakeFiles/hs.dir/fsw/tables/hs_mat.c.o

The build fails (or, on a tolerant compiler, emits a warning and produces silently incorrect table contents).

Provenance

Discovered by Inquisitor, AstroLexis's autonomous source + binary security agent. The audit pipeline:

1. KCode (https://github.com/AstrolexisAI/KCode) — deterministic SAST pattern library. kcode audit cfe/modules/msg --skip-verify --json returned 0 candidates over 19 files. Pattern coverage: 100% of the rules in the library, none matched the four defects above.
2. Inquisitor SourceHunter / novel-pattern-hunt — LLM (Claude Opus 4.7) reading the same source bundle, instructed to look for vulnerability classes NOT covered by the deterministic library (cross-function flow, TOCTOU, integer truncation, silently-broken macros, header-derived-length OOB). Returned the four findings above with file:line anchors and quoted proof for each.

Per nasa/cFE SECURITY.md AI policy: discovery and report drafting used AI assistance. Discovery: Inquisitor (KCode static pass + LLM novel-pattern pass). Report drafted with AI assistance. All four findings are anchored in concrete source and quoted verbatim; finding #2 is contingent on the priext build configuration being selected by the integrating mission.

Best regards,

Bruno Aiub · AstroLexis · Inquisitor
contact@astrolexis.space