From 70a9ce2ef14c9d2e1ef4973ab11d0bbcb1908b9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roman=20Arce=20Br=C3=A1n?= Date: Wed, 15 Jul 2026 08:03:05 -0600 Subject: [PATCH] Harden RLE integer decoders against buffer cursor overshoot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When decoding a malformed RLE integer stream, the read cursor bufferStart_ can be advanced past the buffer sentinel bufferEnd_. Two spots then turn that past-the-end cursor into an out-of-bounds heap read instead of a clean error: * RleDecoderV2::bufLength() returns bufferEnd_ - bufferStart_ as an unsigned uint64_t. Once bufferStart_ > bufferEnd_ this underflows to a value near 2^64, defeating the std::min clamps in the UnpackDefault fast loops (unrolledUnpack16/32/40/56/64) and causing them to read past the buffer. Return 0 when the cursor has overshot so the fast-loop bufLength()/width term can no longer inflate. * RleDecoderV2::readByte() and RleDecoderV1::readByte() gate their refill on strict equality (bufferStart_ == bufferEnd_). An already-overshot cursor fails that test and dereferences *bufferStart_ out of bounds. Use >= so an overshot cursor still triggers a refill. Both changes are behavior-preserving for valid inputs (bufferStart_ never exceeds bufferEnd_ there); they only bound the pathological overshoot path. Reported to security@orc.apache.org and, per the project security model, redirected here as a hardening improvement by Arnout Engelen (ASF Security). Authored-by: Roman Arce Brán (Cyfra Tech Solutions) Co-Authored-By: Claude Fable 5 --- c++/src/RLEv1.cc | 5 ++++- c++/src/RLEv2.hh | 6 +++++- c++/src/RleDecoderV2.cc | 5 ++++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/c++/src/RLEv1.cc b/c++/src/RLEv1.cc index 72c555e610..4a7ab6e278 100644 --- a/c++/src/RLEv1.cc +++ b/c++/src/RLEv1.cc @@ -140,7 +140,10 @@ namespace orc { signed char RleDecoderV1::readByte() { SCOPED_MINUS_STOPWATCH(metrics, DecodingLatencyUs); - if (bufferStart_ == bufferEnd_) { + // Use >= rather than == so that a cursor which has already overshot the + // buffer end (bufferStart_ > bufferEnd_) still triggers a refill instead of + // dereferencing out of bounds. + if (bufferStart_ >= bufferEnd_) { int bufferLength; const void* bufferPointer; if (!inputStream_->Next(&bufferPointer, &bufferLength)) { diff --git a/c++/src/RLEv2.hh b/c++/src/RLEv2.hh index c2ce5aa851..b7279ee39b 100644 --- a/c++/src/RLEv2.hh +++ b/c++/src/RLEv2.hh @@ -188,7 +188,11 @@ namespace orc { } uint64_t bufLength() { - return bufferEnd_ - bufferStart_; + // Guard against a cursor that has overshot the buffer end. If bufferStart_ + // has advanced past bufferEnd_, the raw pointer difference is negative and + // would underflow to a huge value when returned as unsigned, defeating the + // std::min clamps in the bit-unpacking fast loops (BpackingDefault.cc). + return bufferEnd_ > bufferStart_ ? static_cast(bufferEnd_ - bufferStart_) : 0; } void setBitsLeft(const uint32_t bits) { diff --git a/c++/src/RleDecoderV2.cc b/c++/src/RleDecoderV2.cc index f35dd4fe5d..d3192869a0 100644 --- a/c++/src/RleDecoderV2.cc +++ b/c++/src/RleDecoderV2.cc @@ -31,7 +31,10 @@ namespace orc { unsigned char RleDecoderV2::readByte() { SCOPED_MINUS_STOPWATCH(metrics, DecodingLatencyUs); - if (bufferStart_ == bufferEnd_) { + // Use >= rather than == so that a cursor which has already overshot the + // buffer end (bufferStart_ > bufferEnd_) still triggers a refill instead of + // dereferencing out of bounds. + if (bufferStart_ >= bufferEnd_) { int bufferLength; const void* bufferPointer; if (!inputStream_->Next(&bufferPointer, &bufferLength)) {