dongjoon-hyun commented on code in PR #59117:
URL: https://github.com/apache/spark/pull/59117#discussion_r4147784267
##########
sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/VectorizedRleValuesReader.java:
##########
@@ -994,6 +1014,17 @@ private boolean readNextGroup() {
}
case PACKED -> {
int numGroups = header >>> 1;
+ // Validate before allocating the buffer, so a corrupted header can
neither overflow
+ // `currentCount` nor make us allocate more than the remaining bytes
can fill.
+ if (numGroups > Integer.MAX_VALUE / 8) {
+ throw new ParquetDecodingException(
+ "Corrupted RLE data: bit-packed run of " + numGroups + " groups
is too long");
+ }
+ if ((long) numGroups * bitWidth > in.available()) {
Review Comment:
This bounds the run by the bytes that are left, but not by the number of
values in the page, so a bit-packed run whose bytes are all present can still
allocate far more than the page needs. For example, a dictionary id section
`[01][PACKED header for 2^26 groups][2^26 zero bytes]` in a page with
`valueCount = 1` passes both checks, allocates `int[2^29]` (2 GiB) and unpacks
about 537M ids to return one. 64 MiB of zeros compresses to about 65 KB with
GZIP, so a small file can do this in every task.
It isn't a regression, since parquet-java's
`RunLengthBitPackingHybridDecoder` also allocates `new int[numGroups * 8]`
without a bound. But `initFromPage` already receives `valueCount`, so keeping
it and adding something like
```java
if (numGroups > (valueCount + 7L) / 8) {
throw new ParquetDecodingException("Corrupted RLE data: bit-packed run of
" + numGroups +
" groups is longer than the " + valueCount + " values in the page");
}
```
would cap the buffer at the page's value count with one more comparison per
run. A run of a valid stream never has more than `ceil(valueCount / 8)` groups.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]