[SECURITY] SBN TCP receive path uses attacker-controlled message size before the boundary check, causing an out-of-bounds write into a fixed static buffer

[xmin-02]

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

The SBN TCP module reads the message-size field out of the peer's SBN header and uses it directly as the OS_read() length into a fixed per-connection static buffer, with no upper-bound check before the read. The only size validation that exists (*MsgSzPtr > CFE_MISSION_SB_MAX_SB_MSG_SIZE, inside SBN_UnpackMsg()) runs afterwards, so a peer that declares a size larger than the buffer overflows it before that check is ever reached.

The receive buffer is a static array in apps/sbn/modules/protocol/tcp/fsw/src/sbn_tcp_if.c:207:

static uint8 RecvBufs[SBN_MAX_PEER_CNT][SBN_MAX_PACKED_MSG_SZ];

Each row is SBN_MAX_PACKED_MSG_SZ = SBN_PACKED_HDR_SZ + CFE_MISSION_SB_MAX_SB_MSG_SIZE = 13 + 32768 = 32781 bytes. In the Recv() function (same file, line 408), once the 13-byte header has been received the body-read size is computed straight from the header and used as the read length:

// sbn_tcp_if.c:488
ToRead = CFE_MAKE_BIG32(*((SBN_MsgSz_t *)&RecvBufs[Conn->BufNum])) + SBN_PACKED_HDR_SZ - Conn->RecvSz;
// sbn_tcp_if.c:491
Received = OS_read(Conn->Socket, (char *)&RecvBufs[Conn->BufNum] + Conn->RecvSz, ToRead);

The size field is a big-endian uint32 from the first four bytes the peer sent, and there is no clamp on it before the read. The size is only checked later, in SBN_UnpackMsg() (apps/sbn/fsw/src/sbn_app.c:146), which is called at line 515 — after the write has already happened. Because the write target is a static array, the overflow lands in adjacent BSS (the next connection's RecvBufs row, RecvBufCnt, the SendBufs array, and other module globals) with attacker-controlled bytes. Conn->RecvSz accumulates across partial reads, so the write can also be driven past the end incrementally over several reads.

Connections are accepted without source filtering (CheckNet() -> OS_SocketAccept, sbn_tcp_if.c:288-291), and peer identity is only checked inside SBN_UnpackMsg(), i.e. after the overflowing read, so no prior handshake or authentication is required to reach the vulnerable path.

The same unchecked pattern exists in the serial protocol module (apps/sbn/modules/protocol/serial/fsw/src/sbn_serial_if.c:307). That path is not remotely reachable but should be fixed together.

To Reproduce

Steps to reproduce the behavior:

1. Build a native_std deployment and configure an SBN TCP net in sbn_conf_tbl, then run core-cpu1.
2. Open a TCP connection to the SBN listen port (no handshake required).
3. Send a 13-byte SBN header whose 4-byte size field is 0x00008100 (33024), then stream more than 32768 bytes of body.
4. See the out-of-bounds write: on an AddressSanitizer build the process aborts in OS_read / Recv; on a normal build the adjacent module globals are overwritten.

The size boundary was confirmed by compiling the real receive logic under AddressSanitizer and sweeping the declared size (attached boundary-sweep.txt). The write starts at offset 13 in the 32781-byte buffer, so anything above CFE_MISSION_SB_MAX_SB_MSG_SIZE (32768) runs past the end:

declared size =  32768  ->  OK (no overflow)      <- normal maximum, fills the buffer exactly
declared size =  32769  ->  CRASH (heap-buffer-overflow)
declared size =  65535  ->  CRASH (heap-buffer-overflow)

The overflow begins at exactly CFE_MISSION_SB_MAX_SB_MSG_SIZE + 1, the same limit SBN_UnpackMsg() enforces after the read. A normal peer never sends a size above that limit (the Software Bus message is itself capped at CFE_MISSION_SB_MAX_SB_MSG_SIZE, sbn_app.c:282), so this is not an arbitrary trigger value — it is the protocol's own maximum.

Expected behavior

The declared size should be validated before the body read, not after. Moving the existing CFE_MISSION_SB_MAX_SB_MSG_SIZE check ahead of the OS_read() in Recv() — and rejecting or disconnecting the peer when it is exceeded — closes the write. The same clamp should be applied in sbn_serial_if.c.

Code snips

• apps/sbn/modules/protocol/tcp/fsw/src/sbn_tcp_if.c:207 — RecvBufs static buffer declaration (32781 bytes per row)
• apps/sbn/modules/protocol/tcp/fsw/src/sbn_tcp_if.c:488 — size taken from header with no clamp
• apps/sbn/modules/protocol/tcp/fsw/src/sbn_tcp_if.c:491 — OS_read into the fixed buffer using that size
• apps/sbn/modules/protocol/tcp/fsw/src/sbn_tcp_if.c:515 -> apps/sbn/fsw/src/sbn_app.c:146 — the size check, which only runs after the read
• apps/sbn/modules/protocol/serial/fsw/src/sbn_serial_if.c:307 — same unchecked pattern (serial)

System observed on:

• Hardware: x86-64 (generic)
• OS: Ubuntu 22.04 (Linux 5.x)
• Versions: cFE 7.0 (equuleus-rc1+dev), SBN (nasa/SBN), OSAL and PSP from the same bundle, built for the pc-linux PSP

Additional context

Severity (self-assessed): CVSS 3.1 base 8.8 (High), vector AV:A/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H. The write is reachable pre-authentication from any peer that can connect to the SBN TCP port and places attacker-controlled bytes into adjacent module globals, so the realistic impact ranges from a guaranteed processor crash to control-flow corruption / potential code execution (code execution is not demonstrated in this report). Attack Vector is scored Adjacent because SBN is normally deployed on a mission/inter-node network rather than the open Internet; if a deployment exposes the TCP net more broadly the vector moves toward Network and the score rises accordingly. Final severity is of course NASA's to assign.

This is distinct from the SBN UDP information-disclosure issue: the UDP path over-reads within a bounded copy (disclosure), while this TCP path performs an unbounded OS_read() that writes past the end of the static buffer (memory corruption). A cursory search of nasa/cFS and nasa/SBN issues did not find this TCP body-length case (existing SBN tickets cover UDP truncation and subscription parsing, which are different code paths).

Attached:

• poc.c — standalone reproduction of the Recv() read logic; takes the declared size as an argument so the boundary can be swept
• boundary-sweep.txt — AddressSanitizer report for the overflowing write
• asan-output.txt — full 32768 vs 32769 boundary sweep

Reporter Info

xmin (YongKK team)