FrankChen021 commented on code in PR #19818:
URL: https://github.com/apache/druid/pull/19818#discussion_r3686802401
##########
processing/src/main/java/org/apache/druid/segment/data/ImmutableRTreeObjectStrategy.java:
##########
@@ -65,7 +65,7 @@ public ImmutableRTree fromByteBuffer(ByteBuffer buffer, int
numBytes)
{
// always create the duplicate buffer for creating the objects as original
buffer may have mutations somewhere else which can corrupt objects
ByteBuffer duplicateBuf = buffer.duplicate();
- duplicateBuf.limit(duplicateBuf.position() + numBytes);
+ duplicateBuf.limit(Math.addExact(duplicateBuf.position(), numBytes));
return new ImmutableRTree(duplicateBuf, bitmapFactory);
Review Comment:
Fixed in ae6d91a223: ImmutableRTreeObjectStrategy now validates numBytes
before changing the duplicate buffer limit. ObjectStrategyBoundsTest covers
malformed negative and oversized lengths.
##########
processing/src/main/java/org/apache/druid/segment/data/CompressionStrategy.java:
##########
@@ -241,9 +241,10 @@ public static class UncompressedDecompressor implements
Decompressor
public void decompress(ByteBuffer in, int numBytes, ByteBuffer out)
{
final ByteBuffer copyBuffer = in.duplicate();
- copyBuffer.limit(copyBuffer.position() + numBytes);
+ final int newPosition = Math.addExact(copyBuffer.position(), numBytes);
+ copyBuffer.limit(newPosition);
out.put(copyBuffer).flip();
- in.position(in.position() + numBytes);
+ in.position(newPosition);
Review Comment:
Fixed in ae6d91a223: uncompressed decompression now validates numBytes
against both input and output remaining bytes before either buffer is mutated.
CompressionStrategyTest covers negative and oversized lengths and verifies
positions remain unchanged.
##########
processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongStringComplexMetricSerde.java:
##########
@@ -156,7 +156,7 @@ public SerializablePairLongString fromByteBuffer(ByteBuffer
buffer, int numBytes
{
ByteBuffer readOnlyByteBuffer =
buffer.asReadOnlyBuffer().order(buffer.order());
- readOnlyByteBuffer.limit(buffer.position() + numBytes);
+ readOnlyByteBuffer.limit(Math.addExact(buffer.position(), numBytes));
Review Comment:
Fixed in ae6d91a223 and completed in db308e373b: the pair serde validates
numBytes before slicing, and the same validation plus regression coverage was
applied to the LongDouble, LongFloat, LongLong, and LongString sibling serdes.
##########
processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongStringComplexMetricSerde.java:
##########
@@ -156,7 +156,7 @@ public SerializablePairLongString fromByteBuffer(ByteBuffer
buffer, int numBytes
{
ByteBuffer readOnlyByteBuffer =
buffer.asReadOnlyBuffer().order(buffer.order());
- readOnlyByteBuffer.limit(buffer.position() + numBytes);
+ readOnlyByteBuffer.limit(Math.addExact(buffer.position(), numBytes));
Review Comment:
Fixed in ae6d91a223 and completed in db308e373b: the pair serde validates
numBytes before slicing, and the same validation plus regression coverage was
applied to the LongDouble, LongFloat, LongLong, and LongString sibling serdes.
##########
processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java:
##########
@@ -115,7 +115,7 @@ public Class<ByteBuffer> getClazz()
public ByteBuffer fromByteBuffer(final ByteBuffer buffer, final int
numBytes)
{
final ByteBuffer dup = buffer.asReadOnlyBuffer();
- dup.limit(buffer.position() + numBytes);
+ dup.limit(Math.addExact(buffer.position(), numBytes));
return dup;
Review Comment:
Fixed in ae6d91a223: UTF8_STRATEGY now rejects negative lengths and lengths
larger than buffer.remaining() before creating the read-only view.
ObjectStrategyBoundsTest covers both cases.
##########
processing/src/main/java/org/apache/druid/segment/data/CompressionStrategy.java:
##########
@@ -241,9 +241,10 @@ public static class UncompressedDecompressor implements
Decompressor
public void decompress(ByteBuffer in, int numBytes, ByteBuffer out)
{
final ByteBuffer copyBuffer = in.duplicate();
- copyBuffer.limit(copyBuffer.position() + numBytes);
+ final int newPosition = Math.addExact(copyBuffer.position(), numBytes);
+ copyBuffer.limit(newPosition);
out.put(copyBuffer).flip();
- in.position(in.position() + numBytes);
+ in.position(newPosition);
Review Comment:
Fixed in ae6d91a223: uncompressed decompression now validates numBytes
against both input and output remaining bytes before either buffer is mutated.
CompressionStrategyTest covers negative and oversized lengths and verifies
positions remain unchanged.
--
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]
