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]

Reply via email to