FrankChen021 commented on code in PR #20447:
URL: https://github.com/apache/druid/pull/20447#discussion_r4144491574
##########
processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java:
##########
@@ -564,6 +575,14 @@ protected T copyBufferAndGet(ByteBuffer valueBuffer, int
startOffset, int endOff
{
ByteBuffer copyValueBuffer = valueBuffer.asReadOnlyBuffer();
int size = endOffset - startOffset;
+ if (startOffset < Integer.BYTES || endOffset < startOffset || endOffset >
copyValueBuffer.limit()) {
Review Comment:
[P2] Validate relative header offsets before adding the V1 base
**Finding:** This check validates only the absolute range passed to it, but
V1 adds valuesOffset to each header-derived offset before calling
copyBufferAndGet. A malformed two-entry table such as offsets [0, 10] therefore
lets a direct get(1) pass with startOffset[valuesOffset + 4] and
endOffset[valuesOffset + 10], even though the prior end offset is below the
mandatory four-byte value marker; the method then decodes bytes outside the
declared element instead of rejecting the header. The same intermediate-offset
gap exists in the shared V2 path, and the current tests only access index 0 for
the negative-offset case, so malformed headers can still produce the silent
misreads this PR is meant to eliminate.
**Suggestion:** Validate each header-relative end offset before converting
it to an absolute position (or carry the value-region base into the validator),
and add direct later-index tests for too-small or negative preceding offsets in
both normal and single-threaded access paths.
--
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]
