fix(arrow-avro): cap shift in VLQDecoder::long to avoid overflow panic

[masumi-ryugo]

What

VLQDecoder::long accumulates a stateful zig-zag varint across multiple calls and shifts the next 7-bit payload by the running shift count. The shift wasn't bounded, so an overlong varint that keeps emitting continuation bytes drives self.shift past 63 and panics inside << self.shift:

thread '<unnamed>' panicked at arrow-avro/src/reader/vlq.rs:35:33:
attempt to shift left with overflow

Repro

The cargo-fuzz avro_reader harness being prototyped for #5332 reproduces this in single-digit minutes from an empty corpus with a 19-byte input — the Avro file magic followed by 13 0xFF continuation bytes and a terminator:

0000  4f 62 6a 01 ff ff ff ff  ff ff ff ff ff ff ff ff  |Obj.............|
0010  4f 06 b0 6a b3                                     |O..j.|

Reachable from arrow_avro::reader::ReaderBuilder::new().build(buf_reader) on attacker-controlled bytes — i.e. any service that consumes Avro through the public API will crash on this input.

read_varint, read_varint_array, read_varint_slow, and skip_varint* in the same file already cap at 10 bytes / shift 63 and so are not affected; only the streaming VLQDecoder::long path was missing the bound.

Fix

Switch the shift to checked_shl(self.shift).unwrap_or(0) so contributions past bit 63 are silently dropped (matching what the eventual u64 would have to do anyway), and saturating_add(7) so a truly malicious continuation run can't wrap self.shift past u32::MAX either. The terminator byte (high bit clear) still ends the varint, so a malformed varint that never terminates either runs out of input (returns None) or yields a truncated u64 from the trailing terminator.

self.in_progress |= ((byte & 0x7F) as u64).checked_shl(self.shift).unwrap_or(0);
self.shift = self.shift.saturating_add(7);

Tests

Adds reader::vlq::tests::test_long_overlong_varint_does_not_panic driving 12 continuation bytes plus a terminator (which would otherwise reach shift = 84), and asserts the subsequent call decodes a fresh varint cleanly so the decoder's state was reset on the malformed run. The existing test_varint continues to pass.

Alternatives considered

• Track byte count and bail at 10: matches what the stateless read_varint_* paths do. Would need an extra count: u8 field on VLQDecoder and a way to surface "malformed" to the caller. Bigger API change for the same observable behavior under valid input. Happy to switch if you prefer it.
• Saturate to u64::MAX: same panic-free property, but masks the malformed-input signal. The checked_shl form drops the contribution silently while still letting the terminator byte end the varint, which I think is the smallest-touch fix that keeps the public API unchanged.

xref #5332

[alamb]

Thank you for this @masumi-ryugo

[alamb]

I wonder if it would be better to return an error here on invalid input rather than silently ignoring it 🤔

@mzabaluev or @jecsand838 I wonder if you have an opinion

[github-actions]

Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.