Skip to content

Fix out-of-bounds write in decode_type1033 (RTCM3 antenna/receiver descriptors) — closes #799 - #801

Open
tiarno wants to merge 1 commit into
tomojitakasu:masterfrom
tiarno:pr/clamp-master
Open

Fix out-of-bounds write in decode_type1033 (RTCM3 antenna/receiver descriptors) — closes #799#801
tiarno wants to merge 1 commit into
tomojitakasu:masterfrom
tiarno:pr/clamp-master

Conversation

@tiarno

@tiarno tiarno commented Aug 31, 2026

Copy link
Copy Markdown

Fixes the out-of-bounds write reported in #799: the five RTCM3 type 1033 descriptor lengths (n, m, n1, n2, n3) come straight off the wire and are used unclamped as strncpy lengths against 64-byte fields in sta_t. A declared length above 63 writes past the 64-byte destination field — everything from byte 31 onward is zero-padding rather than attacker content, since the 32-byte staging buffers' read loops never capture more than 31 real bytes. At 209 and up (n1, via rectype) the write reaches sta.pos — the base station's own coordinates — zeroing low-order mantissa bytes and silently corrupting a receiver's adopted base position while it continues to report a fixed solution. postpos.c's post-processing path guards the equivalent assignment with norm(sta[i].pos,3)>0.0; the real-time server path (rtksvr.c) does not.

The fix clamps each declared length to the staging buffers' actual capacity, immediately before the strncpy block:

if (n >31) n =31;
if (m >31) m =31;
if (n1>31) n1=31;
if (n2>31) n2=31;
if (n3>31) n3=31;

Verified against benign and malicious streams (declared lengths 211, 255, and an out-of-bounds variant targeting recsno): benign streams decode byte-identically before and after, all three attack variants land at the true coordinate instead of a corrupted one, and UBSan's array-bounds finding (index 255 out of bounds for type 'char [64]', matching the original report) is gone.

The same missing clamp exists unchanged on rtklib_2.4.3 (2.4.3 b34) — the branch this defect was actually measured against in detail, and what's shipped in Debian's rtklib package and in at least one commercial RTK receiver's firmware. The fix there is the same six lines, offset by one unrelated line present only on that branch; verified separately. Happy to open a second PR against that branch if useful.

Scope note: decode_type1007/1008 have the same unclamped-length pattern but the fields they write (antdes/antsno) sit too early in sta_t to reach pos or leave the struct — memory-safety-only, no positioning impact, and out of scope for this PR.

Fixes an out-of-bounds write in decode_type1033 (issue tomojitakasu#799): the five
descriptor length fields (n, m, n1, n2, n3) come off the wire unclamped
and are used as strncpy lengths against 64-byte sta_t fields. A declared
length above 63 writes past the 64-byte destination field; everything
from byte 31 onward is zero-padding rather than attacker content, since
the 32-byte staging buffers' read loops never capture more than 31 real
bytes. At 209 and up (n1, via rectype) the write reaches sta.pos, the
base station coordinates, zeroing low-order mantissa bytes and silently
corrupting the adopted base position.

Clamp each length to the staging buffers' capacity immediately before the
strncpy block. Benign streams decode byte-identically; malicious streams
(declared lengths 211, 255, and an out-of-bounds variant) land at the true
coordinate instead of a corrupted one; UBSan's array-bounds finding is gone.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant