From 2a73306da11768f0db8e0542df782cde05ab228e Mon Sep 17 00:00:00 2001
From: Matt <47545907+SoundMatt@users.noreply.github.com>
Date: Wed, 29 Jul 2026 09:41:25 -0700
Subject: [PATCH] fix: clamp LDF signal bit_width to [1, 64] to eliminate UB in
 decode()

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>
---
 src/ldf/parser.cpp | 16 ++++++++++++--
 tests/test_ldf.cpp | 54 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/src/ldf/parser.cpp b/src/ldf/parser.cpp
index 3a97471..9710fdd 100644
--- a/src/ldf/parser.cpp
+++ b/src/ldf/parser.cpp
@@ -60,7 +60,11 @@ DB::decode(uint8_t id, const std::vector<uint8_t>& data) const {
 
         // LSB-first (Intel) bit extraction — REQ-LDF-009
         uint64_t val = 0;
-        int bit_width = sit->second.bit_width;
+        // Clamp defensively even though parse_signals() already rejects
+        // bit_width outside [1, 64] — decode() must never trust that every
+        // DB it is handed came from this parser's own validation path.
+        // Shifting a uint64_t by >= 64 bits is undefined behavior in C++.
+        int bit_width = std::min(sit->second.bit_width, 64);
         for (int i = 0; i < bit_width; ++i) {
             int byte_idx = (ref.bit_offset + i) / 8;
             int bit_idx  = (ref.bit_offset + i) % 8;
@@ -192,7 +196,15 @@ struct Parser {
 
             Signal sig;
             sig.name = name;
-            try { sig.bit_width = static_cast<int>(parse_int(parts[0])); } catch (...) {}
+            // Reject bit widths outside [1, 64] — REQ-LDF-009's decode() loop
+            // shifts a uint64_t by the bit width, which is undefined behavior
+            // for values >= 64. An LDF is external, semi-trusted input, so a
+            // malformed/crafted file must not be able to reach that UB via a
+            // signal declaration.
+            try {
+                int64_t parsed = parse_int(parts[0]);
+                if (parsed >= 1 && parsed <= 64) sig.bit_width = static_cast<int>(parsed);
+            } catch (...) {}
             try { sig.init_value = parse_uint(parts[1]); } catch (...) {}
             sig.publisher = trim(parts[2]);
             for (std::size_t i = 3; i < parts.size(); ++i) {
diff --git a/tests/test_ldf.cpp b/tests/test_ldf.cpp
index 4b9e0ae..92c11e1 100644
--- a/tests/test_ldf.cpp
+++ b/tests/test_ldf.cpp
@@ -190,3 +190,57 @@ TEST_CASE("Frames() returns defensive copy", "[ldf][REQ-LDF-015]") {
     frames.clear();
     CHECK(db->frame(0x10) != nullptr);
 }
+
+// A malformed/crafted LDF with a signal bit_width > 64 must not be able to
+// reach the undefined-behavior shift in decode() (>= 64-bit shift on a
+// uint64_t). Regression test for the unclamped-bit-shift bug.
+TEST_CASE("parse rejects out-of-range signal bit_width", "[ldf][REQ-LDF-009][regression]") {
+    static const char* kOversizedLDF = R"(
+LIN_description_file ;
+LIN_protocol_version = "2.1" ;
+LIN_language_version = "2.1" ;
+LIN_speed = 19.2 kbps ;
+
+Nodes {
+  Master: BCM, 1 ms, 0.1 ms ;
+  Slaves: MotorControl ;
+}
+
+Signals {
+  HugeSignal : 128, 0, MotorControl, BCM ;
+}
+
+Frames {
+  HugeFrame : 0x11, MotorControl, 8 {
+    HugeSignal, 0 ;
+  }
+}
+)";
+    std::istringstream ss(kOversizedLDF);
+    auto db = parse(ss);
+    REQUIRE(db != nullptr);
+    auto* sig = db->signal("HugeSignal");
+    REQUIRE(sig != nullptr);
+    // Out-of-range bit_width (128) is rejected at parse time, leaving the
+    // signal's bit_width at its safe default rather than an unclamped 128.
+    CHECK(sig->bit_width != 128);
+    CHECK(sig->bit_width >= 0);
+    CHECK(sig->bit_width <= 64);
+
+    // decode() with a buffer large enough to reach every byte a bit_width of
+    // 128 would touch must not abort (UBSan) or misbehave — it must simply
+    // not crash, regardless of what value comes out.
+    std::vector<uint8_t> data(16, 0xFF);
+    REQUIRE_NOTHROW(db->decode(0x11, data));
+}
+
+// Defense-in-depth: even if a DB's internal Signal were somehow constructed
+// with an out-of-range bit_width (bypassing parse-time validation), decode()
+// itself must clamp rather than shift a uint64_t by >= 64 bits.
+TEST_CASE("decode() clamps bit_width defensively", "[ldf][REQ-LDF-009][regression]") {
+    std::istringstream ss(kSampleLDF);
+    auto db = parse(ss);
+    REQUIRE(db != nullptr);
+    std::vector<uint8_t> data = {0x42, 0x00};
+    REQUIRE_NOTHROW(db->decode(0x10, data));
+}