fix(pdf417): bound bitstream parser control operands by the length descriptor

[ginit64]

Summary

The PDF417 bitstream parser uses two different bounds. Most loops stop at codewords[0], the Symbol Length Descriptor, which is correct — everything after the declared data codewords is error correction data. But several control sequences index the codeword array directly, with no check at all.

pdf_417_scanning_decoder::decodeCodewords calls correctErrors before verifyCodewordCount and before parsing, so a malformed symbol whose error correction is valid reaches the parser intact and can drive those unchecked transitions. verifyCodewordCount also rewrites a zero descriptor to len - numECCodewords, so the error correction tail inside the slice is expected and normal.

Two consequences, both reachable from image input:

1. Panics. Out-of-bounds indexing, usize subtraction underflow, and an allocation sized from the underflowed value.
2. Silent payload corruption. When the array happens to be long enough, a truncated control sequence takes its operand from the error correction tail and the decoded text or ECI reflects error correction codewords.

Measured baseline on v0.9.2

Harness calls decoded_bit_stream_parser::decode (or decodeMacroBlock) per vector under catch_unwind, default-features = false, features = ["decoders", "pdf417", "encoding_rs"].

rxing 0.9.2 baseline, profile debug
empty slice                                    [] -> PANIC: index out of bounds: the len is 0 but the index is 0
zero length descriptor                         [0, 477, 477] -> PANIC: attempt to subtract with overflow
descriptor past array                          [9, 477] -> PANIC: index out of bounds: the len is 2 but the index is 2
macro file id at boundary, latch in ec tail    [7, 928, 111, 100, 100, 200, 300, 923] -> PANIC: attempt to subtract with overflow
macro optional field latch at boundary         [8, 928, 111, 100, 100, 200, 300, 923] -> PANIC: index out of bounds: the len is 8 but the index is 8
macro optional field identifier in ec tail     [8, 928, 111, 100, 100, 200, 300, 923, 0, 1000] -> PANIC: attempt to subtract with overflow
mode shift 913 at final data codeword          [3, 477, 913] -> PANIC: index out of bounds: the len is 3 but the index is 3
mode shift 913 operand in ec tail              [3, 477, 913, 1000, 1000] -> Ok(text "P\u{3e8}", default metadata)
outer mode shift 913 at boundary               [4, 901, 100, 913] -> PANIC: index out of bounds: the len is 4 but the index is 4
outer mode shift 913 operand in ec tail        [4, 901, 100, 913, 1000] -> Ok(text "d\u{3e8}", default metadata)
charset eci 927 at final data codeword         [3, 477, 927] -> PANIC: index out of bounds: the len is 3 but the index is 3
charset eci 927 operand in ec tail             [3, 477, 927, 1000, 1000] -> PANIC: attempt to subtract with overflow
byte compaction leading eci at boundary        [3, 901, 927] -> PANIC: index out of bounds: the len is 3 but the index is 3
byte compaction inner eci at boundary          [5, 901, 100, 101, 927] -> PANIC: index out of bounds: the len is 5 but the index is 5
byte compaction inner eci operand in ec tail   [5, 901, 100, 101, 927, 1000] -> Ok(text "de", default metadata)
outer charset eci at boundary                  [4, 902, 100, 927] -> PANIC: index out of bounds: the len is 4 but the index is 4
outer charset eci operand in ec tail           [4, 902, 100, 927, 1000] -> Ok(text "00", default metadata)
general purpose eci without operands           [4, 901, 100, 926, 1000, 1000] -> Ok(text "d", default metadata)
user defined eci without operand               [4, 901, 100, 925, 1000] -> Ok(text "d", default metadata)
panicking cases: 13/19

Release profile is the same 13 panics, with the underflow surfacing where the value is consumed:

zero length descriptor                         [0, 477, 477] -> PANIC: capacity overflow
macro file id at boundary, latch in ec tail    [7, 928, 111, 100, 100, 200, 300, 923] -> PANIC: slice index starts at 8 but ends at 7
macro optional field identifier in ec tail     [8, 928, 111, 100, 100, 200, 300, 923, 0, 1000] -> PANIC: capacity overflow
charset eci 927 operand in ec tail             [3, 477, 927, 1000, 1000] -> PANIC: capacity overflow

The six non-panicking rows are the corruption case. [3, 477, 913, 1000, 1000] declares three data codewords, so 1000 is error correction data — yet U+03E8 appears in the decoded text. Decoding [3, 477, 913] alone panics, so that character comes only from the tail.

Defects

• decode: codewords[0] on an empty slice; textCompaction(codewords, 1, ..) computes codewords[0] - 1 for the buffer length, so a zero descriptor underflows and the result sizes a vec!.
• decode: MODE_SHIFT_TO_BYTE_COMPACTION_MODE (913) and ECI_CHARSET (927) read codewords[codeIndex] unconditionally.
• decode: ECI_GENERAL_PURPOSE (926) and ECI_USER_DEFINED (925) advance past operands without checking they exist.
• decodeMacroBlock: if codewords[codeIndex] == BEGIN_MACRO_PDF417_OPTIONAL_FIELD after the file-ID loop has no bound. This is the read testStandardSample3 works around with "Final dummy ECC codeword required to avoid ArrayIndexOutOfBounds". If that codeword is 923, optionalFieldsStart is set past codeIndex and codeIndex - optionalFieldsStart underflows.
• decodeMacroBlock: the optional-field latch reads its field identifier unconditionally, then calls textCompaction(codewords, codeIndex + 1, ..), which underflows the buffer length when the identifier came from the tail.
• decodeMacroBlock: optionalFieldsLength -= 1 is unchecked.
• textCompaction: both buffer lengths use codewords[0] as usize - codeIndex; the 913 and 927 arms read operands unconditionally, and the 927 arm reallocates from the same unchecked subtraction after advancing.
• byteCompaction: the leading-ECI loop and the inner code == ECI_CHARSET branch read operands unconditionally.

Fix

Express every bound as the declared data-codeword count:

• dataCodewordCount validates the descriptor once per entry point; absent, zero, or larger than the slice is Exceptions::FORMAT. verifyCodewordCount already guarantees 1..=len for the scanning-decoder path, so this only constrains direct callers.
• operandAt reads a control operand only from inside the declared range.
• skipOperands proves skipped operands exist before advancing.
• textCompactionCapacity uses checked_sub, so no buffer length is ever derived from an underflow.

The optional-field check after the file-ID loop is now bounded, and the optional-field copy uses checked subtraction.

Nothing else changes: no valid-symbol semantics, no error-correction algorithms, no character-set behaviour, no detector behaviour, no symbology support. The now-redundant codeIndex < codewords.len() in the file-ID loop is dropped because dataCodewordCount <= codewords.len() already holds, and keeping two different bounds side by side is what caused this bug class.

Tests

cargo test --release: 629 lib tests + every blackbox suite pass, 0 failed. Annex H standard samples, ECI suites, permutation suites, testBinaryData, and the PDF417 blackbox suites are unchanged. cargo clippy --lib --all-targets reports the same 89 warnings as the unpatched tag.

testStandardSample1/Sample2 already carry 1000, 1000, 1000 past their declared boundary with the comment "we should never reach these". That is now asserted rather than assumed.

Three existing tests were #[should_panic], which cannot distinguish a returned error from a crash — exactly the property at issue — so they now assert Err(Exceptions::FORMAT):

• testSampleWithBadSequenceIndexMacro
• testSampleWithNoFileIdMacro
• testSampleWithNoDataNoMacro

New tests:

• testUnusableSymbolLengthDescriptor
• testMacroFileIdEndingAtDataBoundary — also shows the dummy codeword testStandardSample3 needs is no longer required
• testTruncatedMacroOptionalField
• testMacroOptionalFieldPayloadEndingAtDataBoundary
• testTruncatedModeShiftToByteCompaction
• testTruncatedEciControlSequence
• testValidSamplesIgnoreTheErrorCorrectionTail

Each also asserts the general property via assertErrorCorrectionTailIsNotPayload: decoding a vector together with its error correction tail must equal decoding &codewords[..codewords[0]] alone. That fails for any read past the boundary, not only for the reads a specific assertion anticipated.

Notes

Based exactly on v0.9.2 so it can be cherry-picked onto main; happy to rebase or split the test-only changes if you prefer.

catch_unwind is not a viable mitigation for downstream users on wasm32 or with panic = "abort", which is why this is proposed as a parser fix rather than a caller-side guard.

[hschimke]

I will attempt to review the PR this week.