Skip to content

ORC-2192: [C++] Reject invalid string length in StringDirectColumnReader#2673

Closed
dongjoon-hyun wants to merge 1 commit into
apache:mainfrom
dongjoon-hyun:ORC-2192
Closed

ORC-2192: [C++] Reject invalid string length in StringDirectColumnReader#2673
dongjoon-hyun wants to merge 1 commit into
apache:mainfrom
dongjoon-hyun:ORC-2192

Conversation

@dongjoon-hyun

@dongjoon-hyun dongjoon-hyun commented Jul 7, 2026

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

This PR adds validation to StringDirectColumnReader::computeSize in the C++ reader so a negative or overflowing string length throws ParseError before any blob is allocated. Both callers (next and skip) are covered, and since buildReader routes all direct-encoded string-family types to this reader, the single guard protects BINARY/CHAR/STRING/VARCHAR/GEOMETRY/GEOGRAPHY.

Why are the changes needed?

The LENGTH stream is decoded as unsigned RLE, so a malformed file can encode a varint >= 2^63 that becomes a negative int64_t. Cast to size_t, this triggers a huge blob.resize() and an out-of-bounds pointer walk. This restores parity with the Java reader, which already guards this path in BytesColumnVectorUtil.commonReadByteArrays.

if (totalLength < 0) {
StringBuilder sb = new StringBuilder("totalLength:" + totalLength
+ " is a negative number.");
if (batchSize > 1) {
sb.append(" The current batch size is ");
sb.append(batchSize);
sb.append(", you can reduce the value by '");
sb.append(OrcConf.ROW_BATCH_SIZE.getAttribute());
sb.append("'.");
}
throw new IOException(sb.toString());
}

How was this patch tested?

Added TestColumnReader.testStringDirectNegativeLength, which feeds a crafted LENGTH stream decoding to INT64_MIN and asserts ParseError is thrown.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Fable 5

@dongjoon-hyun dongjoon-hyun changed the title ORC-2192: Reject invalid string length in StringDirectColumnReader ORC-2192: [C++] Reject invalid string length in StringDirectColumnReader Jul 7, 2026
dongjoon-hyun added a commit that referenced this pull request Jul 7, 2026
### What changes were proposed in this pull request?

This PR adds validation to `StringDirectColumnReader::computeSize` in the C++ reader so a negative or overflowing string length throws `ParseError` before any blob is allocated. Both callers (`next` and `skip`) are covered, and since `buildReader` routes all direct-encoded string-family types to this reader, the single guard protects `BINARY`/`CHAR`/`STRING`/`VARCHAR`/`GEOMETRY`/`GEOGRAPHY`.

### Why are the changes needed?

The LENGTH stream is decoded as unsigned RLE, so a malformed file can encode a varint `>= 2^63` that becomes a negative `int64_t`. Cast to `size_t`, this triggers a huge `blob.resize()` and an out-of-bounds pointer walk. This restores parity with the Java reader, which already guards this path in `BytesColumnVectorUtil.commonReadByteArrays`.

https://github.com/apache/orc/blob/1ce1786b49c8af0ae21e2db4ef0e6526c5824f2a/java/core/src/java/org/apache/orc/impl/TreeReaderFactory.java#L2087-L2098

### How was this patch tested?

Added `TestColumnReader.testStringDirectNegativeLength`, which feeds a crafted LENGTH stream decoding to `INT64_MIN` and asserts `ParseError` is thrown.

### Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Fable 5

Closes #2673 from dongjoon-hyun/ORC-2192.

Authored-by: Dongjoon Hyun <dongjoon@apache.org>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
(cherry picked from commit 3b3f227)
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
@dongjoon-hyun dongjoon-hyun added this to the 2.3.1 milestone Jul 7, 2026
@dongjoon-hyun dongjoon-hyun deleted the ORC-2192 branch July 7, 2026 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant