[PR] fix: validate numBytes in CompressedVSizeColumnarIntsSupplier (druid)

["dfengliu (via GitHub)" <gi...@apache.org>]

dfengliu opened a new pull request, #20174:
URL: https://github.com/apache/druid/pull/20174

   ### Description
   
   `CompressedVSizeColumnarIntsSupplier.fromByteBuffer()` trusts the `numBytes` byte in the serialized column header. Writers only ever emit values 1..4 (`VSizeColumnarInts.getNumBytesForMax()`), but the read path never verifies it: the byte directly drives the shift/mask constants (`bigEndianShift`, `littleEndianMask`) and the per-value absolute reads (`buffer.getInt(bufferIndex * numBytes)`).
   
   A segment carrying any other value reads back silently corrupted data. Using a serialized supplier with the `numBytes` byte tampered to 100, `get(0)` returns 50462976 instead of 0 and `get(4)` returns 117835012 instead of 4, with no error raised. With `numBytes=0` every value reads as 0. This reader backs every dictionary-encoded column, multi-value columns, and nested variant columns, so malformed storage can silently poison query results. JVM bounds checks contain reads to the decompressed buffer, so this is an integrity/robustness issue rather than a memory-safety one.
   
   #### Fixed the bug ...
   
   Validate `numBytes` in the `CompressedVSizeColumnarIntsSupplier` constructor with `Preconditions.checkArgument` (range `[1, Integer.BYTES]`), so malformed headers fail fast with a descriptive message on both the read (`fromByteBuffer`) and construction (`fromList`) paths, before any shift/mask constants or offsets are computed.
   
   Added `testInvalidNumBytesRejected` to `CompressedVSizeColumnarIntsSupplierTest`: it serializes a real supplier, tampers the `numBytes` header byte with 0, 5, 100 and -1, and asserts rejection. I verified the test fails without the fix and passes with it; the control round-trip still reads all values correctly.
   
   #### Release note
   
   - Fix a potential silent data corruption when reading a column whose serialized header carries an invalid `numBytes` value. Druid now rejects such columns with a descriptive `IllegalArgumentException` instead of returning wrong values.
   
   <hr>
   
   ##### Key changed/added classes in this PR
    * `CompressedVSizeColumnarIntsSupplier`
    * `CompressedVSizeColumnarIntsSupplierTest`
   
   <hr>
   
   This PR has:
   
   - [x] been self-reviewed.
   - [ ] added documentation for new or modified features or behaviors.
   - [x] a release note entry in the PR description.
   - [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
   - [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
   - [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
   - [x] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
   - [ ] added integration tests.
   - [ ] been tested in a test Druid cluster.


-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

Re: [PR] fix: validate numBytes in CompressedVSizeColumnarIntsSupplier (druid)

["FrankChen021 (via GitHub)" <gi...@apache.org>]

FrankChen021 merged PR #20174:
URL: https://github.com/apache/druid/pull/20174


-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org