Skip to content

fix: clamp LDF signal bit_width to [1, 64] to eliminate UB in decode() - #28

Open
SoundMatt wants to merge 1 commit into
mainfrom
fix/ldf-decode-bitshift-clamp
Open

fix: clamp LDF signal bit_width to [1, 64] to eliminate UB in decode()#28
SoundMatt wants to merge 1 commit into
mainfrom
fix/ldf-decode-bitshift-clamp

Conversation

@SoundMatt

Copy link
Copy Markdown
Owner

Problem

Signal::bit_width was an unconstrained int parsed directly from LDF
text with no upper-bound check. DB::decode() then looped
val |= (uint64_t)1 << i up to bit_width, which is undefined behavior
(and a UBSan abort under this repo's own sanitizers/fusa-asil-b
gates) for any bit_width >= 64.

An LDF is an external, semi-trusted config file, so a crafted or
malformed one with an out-of-range signal width could reach this UB via
the public decode() API given a data buffer longer than 8 bytes — a
straightforward DoS via untrusted input.

Fix

  • parse_signals() now rejects bit_width values outside [1, 64] at
    parse time, leaving the signal's bit_width at its safe zero default
    rather than an unclamped out-of-range value.
  • decode() itself also clamps the loop bound to 64 as defense in
    depth, in case a DB is ever populated with an out-of-range Signal
    by some path other than parse().

Testing

  • Added a regression test with a 128-bit signal confirming parse-time
    rejection and that decode() does not throw/abort even with a data
    buffer sized to reach every byte the old unclamped width would have
    touched.
  • Added a defense-in-depth regression test at the decode() layer.
  • Full build + ctest (178/178 passing).
  • ASan+UBSan build + ctest (178/178 passing, no aborts).

Closes #18

Signal::bit_width was an unconstrained int parsed straight from LDF text
with no upper bound. DB::decode() then looped `1ULL << i` up to
bit_width, which is undefined behavior (and a UBSan abort) for any
bit_width >= 64. A crafted or malformed .ldf file with an out-of-range
signal width could reach this UB via the public decode() API given a
data buffer longer than 8 bytes.

parse_signals() now rejects bit_width values outside [1, 64] at parse
time (leaving the signal at its safe zero default), and decode() itself
clamps the loop bound to 64 as defense in depth, in case a DB is ever
constructed with an out-of-range Signal by some other path.

Closes #18

Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com>
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.

lin::ldf::DB::decode() has an unclamped bit-shift (UB, aborts under UBSan) for LDF signals with bit_width > 64

1 participant