Roman-Da-Ripper opened a new pull request, #2691:
URL: https://github.com/apache/orc/pull/2691

   ### What changes were proposed in this pull request?
   
   This is a small defense-in-depth hardening of the C++ RLE integer decoders 
so that a **malformed / truncated RLE stream can no longer walk the read cursor 
past the end of the internal stream buffer**.
   
   Two minimal changes:
   
   1. **`RleDecoderV2::bufLength()` (`c++/src/RLEv2.hh`)** now returns `0` when 
the cursor has overshot the buffer (`bufferStart_ > bufferEnd_`) instead of the 
raw pointer difference. The difference is returned as an unsigned `uint64_t`; 
once `bufferStart_ > bufferEnd_` it underflows to a value near 2^64, which 
defeats the `std::min` clamps in the `UnpackDefault` bit-unpacking fast loops 
(`unrolledUnpack16/32/40/56/64` in `BpackingDefault.cc`) and lets them read 
`valuesRemaining * width` bytes off the end of the heap buffer.
   
   2. **`RleDecoderV2::readByte()` (`c++/src/RleDecoderV2.cc`) and 
`RleDecoderV1::readByte()` (`c++/src/RLEv1.cc`)** now gate their buffer refill 
on `bufferStart_ >= bufferEnd_` instead of `bufferStart_ == bufferEnd_`. A 
cursor that has already overshot fails the strict-equality test and 
dereferences `*bufferStart_` out of bounds; `>=` makes an overshot cursor 
refill (and cleanly hit `ParseError` at EOF) instead.
   
   Both changes are behavior-neutral on valid data — for a well-formed stream 
`bufferStart_` never exceeds `bufferEnd_`, so `bufLength()` returns the same 
value and the refill guard fires at exactly the same point. They only bound the 
pathological overshoot path.
   
   ### Why are the changes needed?
   
   Reading a crafted/corrupt ORC file can currently drive the RLE integer 
decoders to read one or more bytes past the end of a heap stream buffer 
(AddressSanitizer: `heap-buffer-overflow READ of size 1`) via the public reader 
API (`orc::createReader` / `RowReader::next`). The effect is out-of-bounds 
**reads** / a crash of the reading process (no write / no code-execution 
demonstrated).
   
   The underlying cause is that the decode cursor `bufferStart_` is allowed to 
advance past the `bufferEnd_` sentinel when a run length / bit width taken from 
the file implies consuming more bytes than the current stream buffer holds; 
neither `bufLength()` nor the `readByte()` refill guard bounds the cursor once 
it has overshot.
   
   This was reported privately to `[email protected]`. Per the project's 
published security model (https://orc.apache.org/security/), a crash while 
reading an untrusted/malformed file is out of scope as a vulnerability, and 
Arnout Engelen (ASF Security) kindly invited us to submit the fix through the 
normal contribution channel as a hardening improvement — hence this PR.
   
   ### How was this patch tested?
   
   * Full existing C++ unit suite: **752/752 passed** (`orc-test`, 
`RelWithDebInfo`), including the 123 RLE / integer / byte-RLE / writer 
round-trip tests — no regressions.
   * Reproduced the original overreads against the **real file-based reader** 
(`readLocalFile` + `createReader` + `RowReader::next`) built with 
AddressSanitizer, on `main`:
     * **Before:** 31/31 malformed-file test cases produced 
`heap-buffer-overflow` in `RleDecoderV2::readByte`, `RleDecoderV1::readByte`, 
`UnpackDefault::unrolledUnpack32` and `unrolledUnpack64`.
     * **After:** 0/31 crash; every input now surfaces a handled exception 
(`ParseError` / decompression EOF) or decodes cleanly. No ASAN report.
   
   Representative before/after (`unrolledUnpack32` case):
   ```
   before: AddressSanitizer: heap-buffer-overflow READ of size 1
           #0 orc::UnpackDefault::unrolledUnpack32  BpackingDefault.cc:149
   after:  HANDLED-EXCEPTION: Buffer error in 
ZlibDecompressionStream::NextDecompress
   ```
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Code (Anthropic Claude Opus 4.8)
   
   ---
   
   Contributed by **Roman Arce Brán — Cyfra Tech Solutions** 
(`[email protected]`).
   
   Note on issue tracking: I don't have an Apache JIRA account to file the 
`ORC-xxxx` ticket that the PR template asks for, so this PR intentionally omits 
the `ORC-xxxx` prefix. Happy to add the reference (and reword the commit) once 
a JIRA is created/assigned — just let me know the number.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to