FrankChen021 commented on code in PR #20315:
URL: https://github.com/apache/druid/pull/20315#discussion_r3979690063
##########
processing/src/main/java/org/apache/druid/segment/data/FixedIndexed.java:
##########
@@ -63,6 +63,14 @@ public static <T> Supplier<FixedIndexed<T>> read(ByteBuffer
bb, TypeStrategy<T>
final boolean isSorted = (flags & IS_SORTED_MASK) == IS_SORTED_MASK ? true
: false;
Preconditions.checkState(!(hasNull && !isSorted), "cannot have null values
if not sorted");
final int size = buffer.getInt() + (hasNull ? 1 : 0);
+ Preconditions.checkArgument(size >= 0, "size[%s] must be non-negative",
size);
+ final int valuesCount = hasNull ? size - 1 : size;
Review Comment:
[P2] Reject a negative valuesCount after null adjustment
When the null flag is set, the header count is incremented before this line.
A crafted header with `hasNull` and a serialized count of `-1` therefore
produces `size == 0` and `valuesCount == -1`; the following upper-bound check
still passes because the byte count is negative. The final position update then
moves `bb` backwards by `width` (for the width-4 dictionary this lands in the
header), so callers that parse subsequent sections read them from the wrong
offset, while other callers can construct a zero-sized dictionary despite the
null flag. Reject a negative raw count or require `valuesCount >= 0` before the
multiplication, and cover this null-flag case with a test.
--
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]
