Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions src/ldf/parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
54 changes: 54 additions & 0 deletions tests/test_ldf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Loading