Message::encode can emit a frame Message::decode rejects, when the header's declared length disagrees with the payload

[JustinKovacich]

Problem

Message::encode can emit a frame that Message::decode rejects.

Message::decode takes exactly header.payload_length bytes and hands them to
Payload::decode, which is not required to consume all of them. The decoded
Message keeps the header verbatim, including the declared length. encode
then writes that stale length beside a payload of its real size:

fn encode(&self, writer: &mut impl embedded_io::Write) -> Result<usize, MessageError> {
    let written = self.header.encode(writer)?;      // declared length, from the wire
    Ok(written + self.payload.encode(writer)?)      // the payload's actual size
}

Note that encoded_size() already disagrees with the header it writes — it
returns Header::SIZE + payload.encoded_size(), not Header::SIZE + header.payload_length.

Reproduction

A NACK frame whose header declares 5 body bytes and carries 1:

let framed: [u8; 13] = [0x02, 0xFD, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05, 0x03, 0, 0, 0, 0];
let (msg, _rest) = Message::decode(&framed).unwrap();   // accepted
assert_eq!(msg.header.payload_length, 5);               // preserved
// re-encode: 9 bytes, header still claims 5
let size = msg.encoded_size().unwrap();                 // 9
let mut buf = vec![0u8; size];
{ let mut w: &mut [u8] = &mut buf; msg.encode(&mut w).unwrap(); }
Message::decode(&buf).unwrap();  // Err(Incomplete { needed: 5, available: 1 })

Also reachable through the identification requests, where the decode is lossy
by design. 00 ff 00 01 00 00 00 01 00 decodes to the unit variant
VehicleIdentificationRequest, discarding the declared body byte; re-encoding
gives a header claiming 1 byte with no body.

Found by the ported fuzz_roundtrip target, in under a second.

Impact

Anything that decodes a frame and re-emits it — a proxy, a replay tool, a
logging fake, a test harness that echoes what it received — converts a
malformed-but-accepted frame into a corrupt one on the wire. The crate's own
MessageCodec Encoder is on that path.

Two behaviors contribute

1. decode is lenient: it accepts a declared length the payload does not
   consume. ISO 13400-2 has an entity answer an invalid payload length with
   NACK 0x04.
2. encode trusts the stored length rather than deriving it from the
   payload.

Options

Derive the length in encode is the small, safe fix: build the header from
payload.encoded_size() so an inconsistent frame can never be emitted,
regardless of how lenient decode is. No input that is accepted today would
start being rejected. It does mean decode(encode(m)).header.payload_length
can differ from m.header.payload_length for a frame that arrived with a bogus
length — which is the point.

Make decode strict is the standards-correct complement, but it is a
redesign rather than a fix: Payload::decode would have to report unconsumed
bytes, and the identification requests deliberately discard their EID/VIN body
(see ARCHITECTURE.md §7.6). Under a strict decode, a 0x0002 request
carrying its 6 EID bytes would be rejected outright rather than declined.

Related: ARCHITECTURE.md §4.2 asks whether Incomplete is classified
correctly, which is adjacent to this.

Meanwhile

fuzz/fuzz_targets/fuzz_roundtrip.rs skips inputs whose declared length
disagrees with the payload's real size, so it keeps hunting genuine
asymmetries without asserting a property the crate currently violates. That
skip should come out with the fix.

[JustinKovacich]

Fixed in #17 (stacked: #11 → #12 → #13 → #16 → #17).

Message::encode now derives the header's payload_length from payload.encoded_size() instead of writing self.header.payload_length verbatim, so an encoded frame is always self-consistent. No input that is accepted today starts being rejected, and a well-formed frame is byte-identical — all 11 golden vectors pass unchanged.

The fuzz_roundtrip skip is removed and the target now asserts idempotence (encode → decode → encode gives identical bytes) in place of whole-Message equality, which would have asserted the bug back into existence. 21,033,263 executions clean.

The decode half stays open by design and is recorded in ARCHITECTURE.md §7.6: decode still accepts a header whose payload_length disagrees with what the payload occupies. Making it strict is a redesign rather than a fix — Payload::decode would have to report unconsumed bytes, and the identification requests deliberately discard their EID/VIN body, so a 0x0002 request carrying its six EID bytes would start being rejected outright rather than declined.