Boro Software · Port findings report

Porting freediag from C to Rust

Line-by-line translation of an OBD-II / K-line vehicle-diagnostics scantool, run as a multi-agent pipeline, turned out to be an unusually thorough audit of the original. This is the honest version of what that bought, and what it did not.

A field report on the C→Rust port of an OBD-II / K-line vehicle-diagnostics scantool, run as a multi-agent lead / implementer / reviewer pipeline. Every claim traces to a finding ID, a git-bug ticket (FDR-N), or the frozen C tree at master 619d96c. Citations link to the rendered source.

1. What was done

freediag is an OBD-II / K-line scantool: hardware interface drivers, data-link and application protocols (ISO 9141, ISO 14230/KWP2000, SAE J1850, VAG/KW1281, Volvo D2), a generic OBD-II layer (SAE J1979), and an interactive CLI. The C tree is ~31,700 lines across scantool/, largely abandoned upstream.

The port produced a Cargo workspace — libdiag (library, layered l7→l3→l2→l1→l0→hal) and scantool-rs (CLI) — totalling ~52,000 lines of Rust, of which 5,846 lines (11%) are commented-out C kept above their translations as provenance (a deliberate, retained artifact; see §7).

Result at the time of writing: milestones M0–M6 complete, M7 (idiomatic cleanup) all but the final retirement decision. 70 tracker tickets closed. 1,030 workspace tests pass; the CARSIM equivalence harness runs the legacy C ctest scripts verbatim through the compiled Rust binary and 15 of 18 scenarios pass (the 3 remaining all trace to one architectural gap, §7, not a port defect). Quality gates — cargo fmt, clippy -D warnings, cargo test, a doc-citation check, and a C reference build — are green.

The behavioral reference throughout was the C binary itself: findings and equivalence were checked by running both binaries side by side, not by trusting either the port or the original.

2. The findings corpus

Porting a codebase line by line is an unusually thorough audit of it. 70 distinct findings were recorded and, with two exceptions still marked unverified, each was independently re-derived from the C source by a second agent before being logged. They divide into:

Grouped by mechanism rather than location, a smaller set of recurring failure classes accounts for most of them:

• Unclamped memcpy of a wire-derived length (A4, A7, A8, A13). The most severe class. A length taken off the wire is copied into a fixed buffer with no bound check.
• Silent narrowing / sign-loss conversions (A2, A3, A8, B6, B9, D23). Six findings, five files. An implicit int→uint8_t or signed→unsigned conversion produces a wildly wrong value that is then trusted. This is the class that matters most for the language argument in §3.
• Uninitialized reads trusted as data (A1, A9, A10).
• A resource-lifecycle path that forgets the release every sibling performs (A12, B17, B18, B21, B22). Five findings across three drivers — a leak or a lost-and-leaked telegram on one error branch that every other branch handles.
• A guard testing a lower-resolution proxy than the property it needs (B8, B11, B15, B19) — pointer-non-null standing in for string-non-empty; "two chars remain" for "two hex digits remain"; a truthy rv for a valid length.
• A comment that contradicts the code beneath it (D16, D25, C2) — including an NCMS reporting selector whose comment names the two modes backwards.
• Off-by-one at an array or protocol-block edge (A14, and its near-twin D26).

3. What Rust eliminated — and what it did not

This is the honest centre of the report, and it resists the triumphal version.

Eliminated by construction. Three whole classes cannot survive a straightforward Rust port:

• Unclamped memcpy (A4/A7/A8/A13) — Vec/slice indexing is bounds-checked; there is no fixed destination to overrun.
• Uninitialized reads (A1/A9/A10) — using an uninitialized binding is a compile error, not a lint.
• Resource-lifecycle leaks (A12/B17/B21/B22) — Drop runs on every path; "forgot the cleanup on one branch" is not a category that exists.

For these, the language does the work. A naive translation is simply immune.

Not eliminated — the narrowing-cast class (A2/A3/A8/B6/B9/D23). Rust's as truncates exactly as C's implicit conversion does: 1000u32 as u8 is silently 232, no warning. A line-by-line translation using as reproduces every one of these six bugs verbatim. And the project's own tooling did not catch it: the lint config denied unwrap/expect/panic but not cast_possible_truncation (it lives in clippy's pedantic group, off by default). What closed this class was the porting discipline — the line-by-line method forces a human or agent decision at every cast site — not the language, and not the default tooling. Only after this was measured (FDR-87) was a lint enabled to gate it going forward; the audit of all 47 cast sites found zero surviving truncation bugs, but that is a fact about the porting method, not about Rust.

Not eliminated at all — pure logic and documentation defects (B23, B24, B25, D25, and the whole proxy-guard class). No type system encodes "this multiplier should be 256" (B23, a Mode-6 decode that can invert a FAILED/Passed emissions verdict), or "this branch reads a field nothing ever populates" (B24, a report that has never once printed), or that a comment names its two modes backwards (D25). These were caught only by an agent reading the C and the Rust side by side and noticing the disagreement. A comment is neither compiled nor type-checked; nothing but a careful reader ever catches an inverted one.

The distinction the report should carry forward: Rust converts an entire tier of these findings from silent corruption into either a compile error or a loud panic — but the tier below it, logic and intent, is exactly as hard to get right in Rust as in C, and was carried entirely by process.

D26 is the single cleanest illustration of the top tier. The same inclusive loop bound i <= 0x20, in one loop, is simultaneously one too many for an array write (A14, a live out-of-bounds store) and one too few to reach an unfolded heap over-read in a function it calls. One character sits between two distinct memory-safety defects. In the C, one of them fires. In the Rust, bounds-checked indexing makes both a non-event.

4. What would actually bite: severity and reachability

Ranked by reachability from untrusted input (a real or hostile ECU/adapter), not by theoretical severity:

1. A4 — a memcpy into the wrong address (a pointer field), corrupting a pointer with ECU-controlled bytes on any response ≥ 1 byte, over an ordinary command sequence. No crafted input needed.
2. A8 — a SIZE_MAX-scale copy from everyday J1979 traffic on a non-framed link, via a length that underflows before a cast.
3. A13 — the headline. A wire-derived length memcpy'd into a 7-byte struct field with no clamp. ISO 9141 is the only safe protocol, capped at exactly 7 bytes — almost certainly what the field was sized for. Every other protocol shares the storage without inheriting the cap: ISO 14230 (260-byte frames), and SAE J1850, whose own driver header reads "INCOMPLETE, will not work, but doesnt coredump" and which the project's own test corpus already exercises with real J1979 traffic. Nothing gates which L2 protocol a J1979 session attaches to.
4. A7 — unbounded accumulation into a 1 KB buffer, reachable via an ordinary bad-checksum frame on a framed link — a real hardware event, the C author's own comment marking it "XXX possible buffer overflow!".
5. A14 — a one-byte out-of-bounds write, needing a broad-PID-support ECU; live-reproduced on the C binary by two independent agents.
6. B22 — a VAG telegram lost and leaked on a plausible transient ACK-send failure, with a confirmed downstream behavior change.

The lower tier (B12/B13/B20, "computed but not checked") is real but reachable only through local misconfiguration, not adversarial input.

5. What the corpus says about the C codebase

Every claim here is grounded in specific findings; inference is marked as such.

• A struct sized for the first protocol, never revisited. A13's 7-byte field matches ISO 9141's cap exactly; the later protocols bolted onto the same storage are the vulnerability. (Inference from module layering, not git archaeology.)
• Self-diagnosed weak points, shipped anyway. Four literal author comments map one-to-one onto confirmed findings: A7's "XXX possible buffer overflow!", A8's function describing itself as "broken", D2's "TODO: forward errors up?", B16's "the code often 'forgets' data… flushing increases the odds of not crashing soon." The authors saw several of these. (Inference: a small project under real constraints, not carelessness.)
• Features declared and abandoned. #if 0 DTC tables that don't compile (D20/D21), a dead protocol enum (D15), and the J1850 driver's "INCOMPLETE" header — overtaken by the port's own corpus now driving J1979 over it.
• The shipped data got less scrutiny than the code. Three independent data-file defects (C1/C2/C4), including a default sim DB whose keybytes make every post-init request unmatchable — "likely broken for years without anyone noticing."
• Finding density tracks review effort, not just defect density. The ELM driver holds 11 of 70 findings — but it also got the most sustained review attention. A file examined once with no follow-up ticket shows zero findings regardless of its true state. The counts show where the port looked hardest, not reliably where the C is worst.

6. What the C got right

A findings log is structurally biased toward fault. Deliberately looking for the opposite:

• The VAG spec deviation is reasoned, documented, and field-tested — not a bug. Its header explains a deliberate SAE J2818 departure, why (tested against a European ECU, US behavior unconfirmed), and shows the spec-compliant version tried and reverted based on real-hardware results, left visible in the source rather than deleted. A bugs-only pass would have mis-filed it.
• A suspected out-of-bounds read that proved correct (D19). elm_updatewm's unchecked indexing looked exactly like A6/A14 — full-tree tracing showed the sole writer enforces the length and the destination is exactly sized. In the same file that produced 11 findings, this one was right.
• Careful low-level engineering where it counts (D6): a half-duplex echo filter deliberately relying on unsigned wraparound at byte 0 — "correct, but subtle", and not a finding at all.

An honest limit: the timing-critical 5-baud / ISO 14230 init code — what the project itself calls "the hard parts" — produced few findings. That is consistent with genuine correctness and with nobody scrutinizing it as hard as ELM or VAG were scrutinized. No positive counter-example could be cited there with confidence. Absence of findings is not evidence of quality.

7. The multi-agent process, and where green tests lied

The whitepaper's subject is as much the method as the code. The pipeline was: one lead (PM), implementers in isolated worktrees, and an independent cold reviewer one model tier above the implementer, with a hard review gate before every merge. The lessons that generalized:

Green tests repeatedly concealed unpinned behavior. The recurring failure mode was not red tests — it was tests that passed while testing nothing. A test comparing a timing constant against the same library constant (satisfying the "cite the source" rule while being hollow); an override file that would pass any output because an empty pattern matches everything; a transcript recorder that captured every constant it needed and then asserted on none of them. The countermeasure that worked was mutation-probing: the reviewer deletes each behavior the tests claim to cover and confirms a test goes red. Across the correctness-critical tickets, reviewers routinely found 40–60% of mutations surviving — every one a behavior that looked tested and wasn't.

The single most valuable catch of the effort was a trap a fixture hid. The multi-ECU readiness total is an OR over a shared bitmask, not a per-ECU sum. The one shipped fixture's ECUs have disjoint monitor sets, so a naive sum produces the identical number and passes. A plausible, green, review-clean implementation would have shipped and been wrong on any vehicle whose ECUs share a monitor. It was caught only because a pre-implementation analysis pass flagged the distinction and demanded an overlapping-bit test that no fixture provided.

Conventions that rely on memory fail; mechanisms that fail loudly work. This is the project's governing principle, and it earned its place repeatedly. "Commit before mutation-probing" was a written rule — and was still violated three times in a single session across two agents, once silently shipping a false claim to the lead when a git checkout destroyed an uncommitted doc fix. The response was not a sterner note but a ticket to make the mutation loop unable to destroy uncommitted work. Likewise: dangling test-name citations appeared in three separate tickets, so a citation checker became a gate; silent narrowing casts became an enabled lint; an empty-pattern vacuous pass became a hard harness error. Each started as "everyone should remember" and ended as a compiler error or a failing gate.

Correctness over emulation, decided explicitly. The standing rule is that a confirmed C bug is ported as its intended behavior, not reproduced — with the narrow exception of on-wire behavior a real ECU depends on. This was not a default anyone could apply mechanically; it required a ruling per case. The Mode-6 decode bug (B23) was fixed despite a fixture encoding the buggy numbers, because it is host-side display of a self-decoded value and the bug can invert a verdict. The KWP71 request frame kept its exact byte length (an ECU depends on it) while its uninitialized padding was replaced with defined zeros. The line between them — observable wire behavior versus everything else — had to be drawn by hand, and every crossing is a numbered deviation in the source.

The reviewers earned their tier. Independent cold review one tier up caught what implementers' own testing missed by design, not by diligence: an implementer probes the behaviors it thought of; a reviewer enumerates systematically. The most useful reviews refuted the implementer's own justification with a working test, or invented mutations the implementer never listed — and one reviewer retracted its own finding after 85 reproduction attempts traced apparent data corruption to a stale binary, extending a mechanism rule rather than filing a phantom bug.

8. Honest limits of this report

• Two findings (B22's original filing, since confirmed) passed through a period marked unverified; the methodology's value rests on the second-reader confirmation, and where that had not yet happened it was labelled.
• The finding counts measure where the port looked, not where the C is worst (§5). A confident "N bugs per KLOC" claim would be unsupported.
• No positive quality claim can be made about the timing-critical init code (§6); silence there is genuinely ambiguous.
• Reachability rankings (§4) are grounded in the log's own analysis; where the log did not settle exploitability, the ranking says so rather than inventing one.
• The C tree is frozen at one commit; all citations reference it, and it did not move during the port.

Findings detail: doc/c-findings.md (70 entries, A/B/C/D). Process conventions: PM.md, Rust.md, CLAUDE.md. Tracker: git-bug (FDR-N).

Findings detail: the interactive browser (70 entries, A/B/C/D). Process & economics: the timeline & cost page. Source of record: doc/c-findings.md, PM.md, Rust.md, CLAUDE.md; tracker git-bug (FDR-N).