From 6574c4fd22fe9d9615041dc4f6fb80012ca00573 Mon Sep 17 00:00:00 2001 From: Matt Jones <47545907+SoundMatt@users.noreply.github.com> Date: Fri, 31 Jul 2026 06:17:26 -0700 Subject: [PATCH] =?UTF-8?q?fix:=20audit=20conformance=20pass=20=E2=80=94?= =?UTF-8?q?=20LDF=20frame=20validation,=20E2E=20masquerade=20detection?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gap-audit findings, most-severe first: - ldf.parseFrameHeader now rejects a frame whose ID falls outside 0x00-0x3F or whose length falls outside 0-8 bytes, matching LIN Specification Package 2.2A §2.3.1 (6-bit frame ID) and this package's own LINMaxDataLen constant. Previously a negative length (e.g. from `f: 0x10, MASTER, -4;`) reached DB.Encode's `make([]byte, f.Length)` and panicked with `runtime error: makeslice: len out of range` (confirmed by reproduction against the pre-fix code — see PR body), and an out-of-range ID (e.g. 300) was silently truncated via a bare uint8() cast, corrupting the frame table entry at the truncated ID (CWE-20/CWE-789). Fixing this exposed a second latent bug: a rejected frame's single `continue` didn't skip its body, so its own closing brace was mistaken for the Frames section's closing brace, silently dropping every frame after it — fixed by skipping to the matching closing brace before continuing. Signal-ref bit offsets are now rejected when negative (previously the parse error was discarded). - safety.Receiver.Unwrap now compares the wire-transmitted DataID/ SourceID against the receiver's configured Config and returns a new ErrIDMismatch on mismatch. Per the AUTOSAR E2E Protocol Specification, the DataID is included in the CRC computation specifically to provide masquerade protection, but Unwrap only ever recomputed the CRC over the transmitted bytes themselves — proving self-consistency, not that the frame belonged to the receiver's expected stream. The misleading `_ = dataID // validated implicitly via CRC` comment (which claimed a check that didn't happen) is removed. - virtual.Bus.Publish/PublishClassic now reject a non-nil, zero-length payload the same way they already reject an over-length one, so the in-process virtual bus can never broadcast a 0-data-byte frame that lin.ValidateFrame itself would consider malformed (LIN Specification Package 2.2A: the data field carries 1-8 bytes). PublishClassic also gained the LINMaxDataLen over-length guard Publish already had. - All GitHub Actions in .github/workflows/ are now pinned to immutable commit SHAs (with a `# vX` comment) instead of mutable tags, matching the repo's own SLSA/supply-chain evidence posture. Regression tests added for every fix (verified to fail against the pre-fix code, see PR body); go build/vet/test/test -race and the FuzzParse/FuzzProtectUnwrap/FuzzSendHeader short fuzz runs all pass. Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com> Signed-off-by: Matt Jones <47545907+SoundMatt@users.noreply.github.com> --- .github/workflows/ci.yml | 36 +++++----- .github/workflows/dco.yml | 2 +- .github/workflows/docker.yml | 12 ++-- .github/workflows/release.yml | 4 +- CHANGELOG.md | 32 +++++++++ ldf/parser.go | 26 ++++++- ldf/parser_test.go | 131 ++++++++++++++++++++++++++++++++++ safety/e2e.go | 18 ++++- safety/e2e_test.go | 55 ++++++++++++++ virtual/bus.go | 9 +++ virtual/bus_test.go | 35 +++++++++ 11 files changed, 329 insertions(+), 31 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f3e2e55..3dee859 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,9 +21,9 @@ jobs: go: ["1.25", "1.26"] steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 - - uses: actions/setup-go@v6 + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 with: go-version: ${{ matrix.go }} @@ -36,7 +36,7 @@ jobs: - name: Upload coverage if: matrix.os == 'ubuntu-latest' && matrix.go == '1.25' - uses: actions/upload-artifact@v6 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6 with: name: coverage path: coverage.out @@ -48,9 +48,9 @@ jobs: name: Coverage floor (library packages >= 85%) runs-on: ubuntu-latest steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 - - uses: actions/setup-go@v6 + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 with: go-version: "1.25" @@ -79,9 +79,9 @@ jobs: name: Benchmark smoke (ubuntu-latest) runs-on: ubuntu-latest steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 - - uses: actions/setup-go@v6 + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 with: go-version: "1.25" @@ -103,9 +103,9 @@ jobs: name: Fuzz (short, ubuntu-latest) runs-on: ubuntu-latest steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 - - uses: actions/setup-go@v6 + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 with: go-version: "1.25" @@ -129,9 +129,9 @@ jobs: name: RELAY conform (--strict) runs-on: ubuntu-latest steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 - - uses: actions/setup-go@v6 + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 with: go-version: "1.25" @@ -151,9 +151,9 @@ jobs: name: RELAY interop (LIN) runs-on: ubuntu-latest steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 - - uses: actions/setup-go@v6 + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 with: go-version: "1.25" @@ -177,9 +177,9 @@ jobs: name: go-FuSa full lifecycle (v0.48.0) runs-on: ubuntu-latest steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 - - uses: actions/setup-go@v6 + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 with: go-version: "1.25" @@ -260,7 +260,7 @@ jobs: - name: Upload safety evidence if: always() - uses: actions/upload-artifact@v6 + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6 with: name: gofusa-safety-evidence path: gofusa-audit-pack.zip @@ -275,9 +275,9 @@ jobs: name: Standards compliance (ISO / IEC / DO / UNECE / SLSA) runs-on: ubuntu-latest steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 - - uses: actions/setup-go@v6 + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 with: go-version: "1.25" diff --git a/.github/workflows/dco.yml b/.github/workflows/dco.yml index 3a3edf8..ce3fa00 100644 --- a/.github/workflows/dco.yml +++ b/.github/workflows/dco.yml @@ -9,7 +9,7 @@ jobs: name: Developer Certificate of Origin runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 with: fetch-depth: 0 diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index e747a22..4f3eeb9 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -25,16 +25,16 @@ jobs: target: [go-lin, quickstart, lintool] steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 - name: Set up QEMU (multi-arch) - uses: docker/setup-qemu-action@v3 + uses: docker/setup-qemu-action@c7c53464625b32c7a7e944ae62b3e17d2b600130 # v3 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v3 + uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3 - name: Log in to GHCR - uses: docker/login-action@v3 + uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 # v3 with: registry: ${{ env.REGISTRY }} username: ${{ github.actor }} @@ -54,7 +54,7 @@ jobs: - name: Extract metadata id: meta - uses: docker/metadata-action@v5 + uses: docker/metadata-action@c299e40c65443455700f0fdfc63efafe5b349051 # v5 with: images: ${{ steps.image.outputs.name }} tags: | @@ -64,7 +64,7 @@ jobs: type=sha,prefix=sha-,format=short - name: Build and push - uses: docker/build-push-action@v6 + uses: docker/build-push-action@10e90e3645eae34f1e60eeb005ba3a3d33f178e8 # v6 with: context: . file: docker/Dockerfile diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a380f8b..2dc5245 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -16,12 +16,12 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 with: fetch-depth: 0 ref: main - - uses: actions/setup-go@v6 + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 with: go-version: "1.25" diff --git a/CHANGELOG.md b/CHANGELOG.md index 4904ae9..33b49df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,38 @@ canonical list. Dates are release dates (UTC-7, matching tag creation). ## [Unreleased] +- fix: `ldf.parseFrameHeader` now rejects (rather than silently corrupting) + a frame whose ID is outside 0x00–0x3F or whose declared length is + outside 0–8 bytes — previously a negative length (e.g. from + `f: 0x10, MASTER, -4;`) reached `DB.Encode`'s `make([]byte, f.Length)` + and panicked (`runtime error: makeslice: len out of range`, CWE-789 DoS + on untrusted LDF input), and an out-of-range ID (e.g. `300`) was + silently truncated via a bare `uint8()` cast, corrupting whatever frame + already lived at the truncated ID. A frame rejected this way is now + properly skipped rather than swallowing every subsequent frame in the + same `Frames` section (the previous single-`continue` mistook the + rejected frame's own closing brace for the section's closing brace). + Signal-ref bit offsets with a negative value (also previously discarded + the parse error) are now rejected the same way rather than relying on + incidental Go shift/comparison semantics to avoid a panic (#76) +- fix(safety): `safety.Receiver.Unwrap` now compares the wire-transmitted + `DataID`/`SourceID` against the receiver's configured `Config` and + returns a new `ErrIDMismatch` on mismatch — previously the CRC check + alone was (incorrectly, per the code's own now-corrected comment) + treated as sufficient masquerade protection, so a frame protected under + a different `DataID`/`SourceID` than the receiver's was accepted + without error (#76) +- fix(virtual): `Bus.Publish`/`Bus.PublishClassic` now reject a non-nil, + zero-length payload the same way they already reject an over-length + one, so the virtual bus can never broadcast a 0-data-byte frame that + `lin.ValidateFrame` itself would consider malformed (LIN Specification + Package 2.2A: the data field carries 1–8 bytes); `PublishClassic` also + gained the `LINMaxDataLen` over-length guard `Publish` already had (#76) +- chore(ci): pinned all third-party and first-party GitHub Actions in + `.github/workflows/` to immutable commit SHAs (with a `# vX` comment for + readability) instead of mutable version tags, matching the repo's own + SLSA/supply-chain evidence posture (#76) + ## [1.5.0] — 2026-07-30 - chore: bump `github.com/SoundMatt/RELAY` v1.11.0 → `github.com/SoundMatt/RELAY/v2` diff --git a/ldf/parser.go b/ldf/parser.go index 711b03a..cbad841 100644 --- a/ldf/parser.go +++ b/ldf/parser.go @@ -441,6 +441,15 @@ func (p *ldfParser) parseFrames(db *DB) error { p.next() fr, err := parseFrameHeader(line) if err != nil { + // Skip past this frame's body (up to and including its + // closing brace) so a single invalid frame header does not + // swallow subsequently well-formed frames or get mistaken + // for the closing brace of the whole Frames section. + for p.pos < len(p.lines) { + if p.next() == "}" { + break + } + } continue } // parse signal refs @@ -456,7 +465,14 @@ func (p *ldfParser) parseFrames(db *DB) error { parts := strings.SplitN(inner, ",", 2) if len(parts) == 2 { sigName := strings.TrimSpace(parts[0]) - offset, _ := parseInt(strings.TrimSpace(parts[1])) + offset, err := parseInt(strings.TrimSpace(parts[1])) + if err != nil || offset < 0 { + // Malformed or negative bit offset: skip this signal + // ref rather than let extractBits/packBits receive a + // negative offset (would rely on incidental Go shift/ + // comparison semantics to avoid a panic). + continue + } fr.Signals = append(fr.Signals, SignalRef{Name: sigName, BitOffset: int(offset)}) } } @@ -486,8 +502,14 @@ func parseFrameHeader(line string) (*Frame, error) { if err != nil { return nil, fmt.Errorf("ldf: invalid frame ID in %q: %w", line, err) } + if id < 0 || id > int64(lin.LINMaxID) { + return nil, fmt.Errorf("ldf: frame ID %d out of range 0-%d in %q", id, lin.LINMaxID, line) + } publisher := strings.TrimSpace(parts[1]) - length, _ := parseInt(strings.TrimSpace(parts[2])) + length, err := parseInt(strings.TrimSpace(parts[2])) + if err != nil || length < 0 || length > int64(lin.LINMaxDataLen) { + return nil, fmt.Errorf("ldf: frame length out of range 0-%d in %q", lin.LINMaxDataLen, line) + } return &Frame{ Name: name, ID: uint8(id), diff --git a/ldf/parser_test.go b/ldf/parser_test.go index 10e45a1..018d766 100644 --- a/ldf/parser_test.go +++ b/ldf/parser_test.go @@ -339,6 +339,131 @@ func TestFrames_defensiveCopy(t *testing.T) { } } +// ── go-LIN-01: frame ID/length range validation ────────────────────────────── +// +// Regression coverage for a bug where parseFrameHeader discarded the +// length-parse error and never range-checked ID or length. A frame with a +// negative length made DB.Encode panic via make([]byte, f.Length); an +// out-of-range ID (e.g. 300) was silently truncated to a valid-looking 6-bit +// ID via uint8(id), corrupting the frame table under the wrong key. + +//fusa:test REQ-LDF-005 +//fusa:test REQ-SEC-001 + +func TestParse_rejectsNegativeFrameLength(t *testing.T) { + const input = ` +Frames { + BadFrame: 0x10, MasterNode, -4 { + EngineSpeed, 0; + } +} +` + db, err := ldf.Parse(strings.NewReader(input)) + if err != nil { + t.Fatalf("Parse: %v", err) + } + if f := db.Frame(0x10); f != nil { + t.Fatalf("Frame(0x10) = %+v, want nil (frame with negative length must be rejected, not stored)", f) + } + // The original bug manifested here: make([]byte, f.Length) with a + // negative Length panics. Encode on the rejected ID must simply report + // "unknown frame" (nil), never panic. + if out := db.Encode(0x10, nil); out != nil { + t.Fatalf("Encode(0x10) = % X, want nil for a rejected frame", out) + } +} + +func TestParse_rejectsOutOfRangeFrameID(t *testing.T) { + const input = ` +Frames { + BadFrame: 300, MasterNode, 4 { + EngineSpeed, 0; + } +} +` + db, err := ldf.Parse(strings.NewReader(input)) + if err != nil { + t.Fatalf("Parse: %v", err) + } + // 300 truncated via a bare uint8() cast wraps to 300-256=44 (0x2C). The + // fix must reject the frame outright, not store it under the truncated + // ID, silently corrupting whatever legitimate frame lives at 0x2C. + if f := db.Frame(0x2C); f != nil { + t.Fatalf("Frame(0x2C) = %+v, want nil (out-of-range ID must not be silently truncated into the table)", f) + } + if len(db.Frames()) != 0 { + t.Fatalf("Frames() = %d entries, want 0", len(db.Frames())) + } +} + +// A frame rejected for an out-of-range ID/length must not swallow +// subsequently well-formed frames in the same Frames section — the parser +// must skip past the bad frame's body, not mistake its closing brace for the +// Frames section's own closing brace. +func TestParse_badFrameDoesNotSwallowLaterFrames(t *testing.T) { + const input = ` +Frames { + BadFrame: 300, MasterNode, 4 { + EngineSpeed, 0; + } + GoodFrame: 0x20, SlaveB, 2 { + WindowPos, 0; + } +} +` + db, err := ldf.Parse(strings.NewReader(input)) + if err != nil { + t.Fatalf("Parse: %v", err) + } + f := db.Frame(0x20) + if f == nil { + t.Fatal("Frame(0x20) = nil, want GoodFrame to still be parsed after a preceding invalid frame") + } + if f.Name != "GoodFrame" { + t.Errorf("frame Name = %q, want GoodFrame", f.Name) + } +} + +// ── go-LIN-A3: negative signal-ref bit offset ──────────────────────────────── +// +// Regression coverage for a bug where the bit offset in a frame's signal +// reference was parsed with the error discarded, allowing a negative +// BitOffset to reach extractBits/packBits and rely on incidental Go +// shift/comparison semantics to avoid a panic rather than being rejected by +// design. + +//fusa:test REQ-LDF-006 + +func TestParse_rejectsNegativeSignalBitOffset(t *testing.T) { + const input = ` +Signals { + EngineSpeed: 16, 0x0000, MasterNode, SlaveA; +} +Frames { + EngineFrame: 0x10, MasterNode, 4 { + EngineSpeed, -8; + } +} +` + db, err := ldf.Parse(strings.NewReader(input)) + if err != nil { + t.Fatalf("Parse: %v", err) + } + f := db.Frame(0x10) + if f == nil { + t.Fatal("Frame(0x10) = nil, want the frame itself to still be parsed") + } + for _, ref := range f.Signals { + if ref.Name == "EngineSpeed" { + t.Fatalf("signal ref %+v: negative BitOffset must be rejected, not stored", ref) + } + } + // Encode/Decode must remain panic-safe even though the signal ref was + // dropped (it simply won't appear in the packed/decoded output). + _ = db.Encode(0x10, map[string]uint64{"EngineSpeed": 1}) + _ = db.Decode(0x10, []byte{0, 0, 0, 0}) +} + func FuzzParse(f *testing.F) { f.Add(sampleLDF) f.Add("") @@ -354,6 +479,12 @@ func FuzzParse(f *testing.F) { _ = db.Signals() for id := uint8(0); id <= 0x3F; id++ { _ = db.Frame(id) + // Regression for go-LIN-01: a structurally-valid-but-malicious + // LDF (e.g. a negative or oversize frame length) must never make + // DB.Encode panic (makeslice with a negative/huge length) or + // over-allocate. Every frame Parse actually accepted must be + // safe to Encode. + _ = db.Encode(id, nil) } }) } diff --git a/safety/e2e.go b/safety/e2e.go index 076ca6b..f58dfeb 100644 --- a/safety/e2e.go +++ b/safety/e2e.go @@ -69,6 +69,9 @@ const ( ErrSequenceGap // ErrHeaderTooShort means the payload is shorter than the 10-byte header. ErrHeaderTooShort + // ErrIDMismatch means the transmitted DataID/SourceID did not match the + // Receiver's configured identity (masquerade / mis-addressing detection). + ErrIDMismatch ) // E2EError is returned when an E2E safety check fails. @@ -191,8 +194,19 @@ func (r *Receiver) Unwrap(data []byte) ([]byte, error) { } } - _ = dataID // validated implicitly via CRC - _ = sourceID // validated implicitly via CRC + // Masquerade / mis-addressing detection: the transmitted DataID and + // SourceID MUST match this Receiver's configured identity. The CRC only + // proves the header is self-consistent; it does not bind the frame to the + // expected logical stream, so this check is what actually protects against + // a frame from a different DataID/SourceID being accepted. + if dataID != r.cfg.DataID || sourceID != r.cfg.SourceID { + return nil, &E2EError{ + Kind: ErrIDMismatch, + Counter: seq, + Message: fmt.Sprintf("identity mismatch: wire DataID=0x%04X SourceID=0x%04X, want DataID=0x%04X SourceID=0x%04X", + dataID, sourceID, r.cfg.DataID, r.cfg.SourceID), + } + } r.mu.Lock() defer r.mu.Unlock() diff --git a/safety/e2e_test.go b/safety/e2e_test.go index 79452f4..e374f1e 100644 --- a/safety/e2e_test.go +++ b/safety/e2e_test.go @@ -210,6 +210,61 @@ func TestUnwrap_sequenceGap(t *testing.T) { } } +// ── go-LIN-02: masquerade / mis-addressing detection (ErrIDMismatch) ───────── +// +// Regression coverage for a bug where Receiver.Unwrap never compared the +// wire-transmitted DataID/SourceID against its own configured Config: the +// CRC only proved the header was self-consistent, not that it belonged to +// the expected logical stream, so a frame protected under a different +// DataID/SourceID than the receiver's was silently accepted. + +//fusa:test REQ-SAFETY-001 +//fusa:test REQ-SAFETY-002 +//fusa:test REQ-SEC-002 + +func TestUnwrap_rejectsDataIDMismatch(t *testing.T) { + // Protected under a different DataID than the receiver expects, but + // otherwise perfectly well-formed (correct CRC, correct sequence). + wrongDataID := safety.Config{DataID: cfg.DataID + 1, SourceID: cfg.SourceID} + p := safety.NewProtector(wrongDataID) + r := safety.NewReceiver(cfg) + + protected := p.Protect([]byte{0x01, 0x02}) + _, err := r.Unwrap(protected) + if err == nil { + t.Fatal("expected E2E error for DataID masquerade, got nil") + } + var e2e *safety.E2EError + if !errors.As(err, &e2e) { + t.Fatalf("expected *E2EError, got %T", err) + } + if e2e.Kind != safety.ErrIDMismatch { + t.Errorf("ErrorKind = %v, want ErrIDMismatch", e2e.Kind) + } +} + +func TestUnwrap_rejectsSourceIDMismatch(t *testing.T) { + // Protected under a different SourceID than the receiver expects — same + // masquerade class, different field, both of which the CRC alone does + // not bind to the receiver's expected identity. + wrongSourceID := safety.Config{DataID: cfg.DataID, SourceID: cfg.SourceID + 1} + p := safety.NewProtector(wrongSourceID) + r := safety.NewReceiver(cfg) + + protected := p.Protect([]byte{0x01, 0x02}) + _, err := r.Unwrap(protected) + if err == nil { + t.Fatal("expected E2E error for SourceID masquerade, got nil") + } + var e2e *safety.E2EError + if !errors.As(err, &e2e) { + t.Fatalf("expected *E2EError, got %T", err) + } + if e2e.Kind != safety.ErrIDMismatch { + t.Errorf("ErrorKind = %v, want ErrIDMismatch", e2e.Kind) + } +} + // ── REQ-SAFETY-010: Unwrap returns original payload ────────────────────────── //fusa:test REQ-SAFETY-010 diff --git a/virtual/bus.go b/virtual/bus.go index b9f8264..f361cad 100644 --- a/virtual/bus.go +++ b/virtual/bus.go @@ -100,6 +100,9 @@ func (b *Bus) Publish(id uint8, data []byte) error { if id > lin.LINMaxID { return fmt.Errorf("lin/virtual: frame ID 0x%02X exceeds maximum 0x%02X", id, lin.LINMaxID) } + if data != nil && len(data) == 0 { + return fmt.Errorf("lin/virtual: payload must not be empty: %w", lin.ErrInvalidFrame) + } if len(data) > lin.LINMaxDataLen { return fmt.Errorf("lin/virtual: payload length %d exceeds maximum %d: %w", len(data), lin.LINMaxDataLen, lin.ErrPayloadTooLarge) } @@ -124,6 +127,12 @@ func (b *Bus) PublishClassic(id uint8, data []byte) error { if id > lin.LINMaxID { return fmt.Errorf("lin/virtual: frame ID 0x%02X exceeds maximum 0x%02X", id, lin.LINMaxID) } + if data != nil && len(data) == 0 { + return fmt.Errorf("lin/virtual: payload must not be empty: %w", lin.ErrInvalidFrame) + } + if len(data) > lin.LINMaxDataLen { + return fmt.Errorf("lin/virtual: payload length %d exceeds maximum %d: %w", len(data), lin.LINMaxDataLen, lin.ErrPayloadTooLarge) + } b.mu.Lock() defer b.mu.Unlock() if b.closed { diff --git a/virtual/bus_test.go b/virtual/bus_test.go index 6ded7b0..c366b5a 100644 --- a/virtual/bus_test.go +++ b/virtual/bus_test.go @@ -92,6 +92,41 @@ func TestPublish_rejectsHighID(t *testing.T) { } } +// TestPublish_rejectsEmptyPayload is the regression test for go-LIN-A2: the +// virtual bus must never broadcast a 0-data-byte frame (LIN Specification +// Package 2.2A: the data field carries 1-8 bytes), matching what +// lin.ValidateFrame itself already rejects. Publish(id, nil) removes a +// registration and must remain accepted; Publish(id, []byte{}) — non-nil, +// zero-length — must be rejected the same way over-length data already is. +func TestPublish_rejectsEmptyPayload(t *testing.T) { + b, err := virtual.New() + if err != nil { + t.Fatal(err) + } + defer b.Close() + if err := b.Publish(0x10, []byte{}); err == nil { + t.Error("expected error for non-nil empty payload") + } + // nil must still be accepted (it removes any existing registration). + if err := b.Publish(0x10, nil); err != nil { + t.Errorf("Publish(id, nil) should still succeed, got %v", err) + } +} + +func TestPublishClassic_rejectsEmptyPayload(t *testing.T) { + b, err := virtual.New() + if err != nil { + t.Fatal(err) + } + defer b.Close() + if err := b.PublishClassic(0x10, []byte{}); err == nil { + t.Error("expected error for non-nil empty payload") + } + if err := b.PublishClassic(0x10, nil); err != nil { + t.Errorf("PublishClassic(id, nil) should still succeed, got %v", err) + } +} + func TestPublish_afterClose(t *testing.T) { b, err := virtual.New() if err != nil {