From b43e2f2640438dca4afb782758ed9ef3370a5432 Mon Sep 17 00:00:00 2001
From: masumi ryugo <280057467+masumi-ryugo@users.noreply.github.com>
Date: Mon, 4 May 2026 09:36:54 +0900
Subject: [PATCH] fix(arrow-avro): cap shift in VLQDecoder::long to avoid
 overflow panic

`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 the 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

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).

`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.

Switch 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.

Includes a regression test in `reader::vlq::tests` driving an
overlong continuation run + terminator and asserting that the
subsequent call decodes a fresh varint cleanly (state reset).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 arrow-avro/src/reader/vlq.rs | 35 +++++++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/arrow-avro/src/reader/vlq.rs b/arrow-avro/src/reader/vlq.rs
index 26bf656159c0..faeb80e9de8a 100644
--- a/arrow-avro/src/reader/vlq.rs
+++ b/arrow-avro/src/reader/vlq.rs
@@ -32,8 +32,17 @@ impl VLQDecoder {
     pub fn long(&mut self, buf: &mut &[u8]) -> Option<i64> {
         while let Some(byte) = buf.first().copied() {
             *buf = &buf[1..];
-            self.in_progress |= ((byte & 0x7F) as u64) << self.shift;
-            self.shift += 7;
+            // A valid varint that fits in `u64` is at most 10 bytes, so
+            // the shift is at most 63. Use `checked_shl` to silently drop
+            // contributions past bit 63 instead of panicking with
+            // "attempt to shift left with overflow", and saturate the
+            // shift counter so a stream of continuation bytes can't wrap
+            // it 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);
             if byte & 0x80 == 0 {
                 let val = self.in_progress;
                 self.in_progress = 0;
@@ -163,4 +172,26 @@ mod tests {
             varint_test(rand::random());
         }
     }
+
+    /// Regression test for an overlong zig-zag varint that previously
+    /// panicked inside `((byte & 0x7F) as u64) << self.shift` with
+    /// "attempt to shift left with overflow" once `self.shift` reached
+    /// 64. Found by the `arrow-avro/fuzz/avro_reader` cargo-fuzz harness
+    /// being prototyped in apache/arrow-rs#5332. The 14-byte continuation
+    /// run plus terminator below drives `self.shift` well past 63;
+    /// after this patch the call returns a (truncated) value instead of
+    /// panicking.
+    #[test]
+    fn test_long_overlong_varint_does_not_panic() {
+        let mut decoder = VLQDecoder::default();
+        // Twelve continuation bytes (all 0x7F-payloads with high bit set)
+        // followed by a clean terminator. Shift would otherwise reach 84.
+        let mut bytes: &[u8] = &[
+            0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00,
+        ];
+        let _ = decoder.long(&mut bytes);
+        // The next call must start a fresh varint, with state reset.
+        let mut next: &[u8] = &[0x02];
+        assert_eq!(decoder.long(&mut next), Some(1));
+    }
 }