From 32a78ac89f79049aa00410bcad52a4342f53356a Mon Sep 17 00:00:00 2001 From: Frank Chen Date: Thu, 30 Jul 2026 23:40:36 +0800 Subject: [PATCH 1/5] Fix CodeQL arithmetic and bounds warnings --- .../data/input/aliyun/OssInputSourceTest.java | 4 ++-- .../druid/storage/aliyun/OssTaskLogsTest.java | 3 ++- .../ByteBufferCompressedBigDecimal.java | 14 +++++++++---- ...ressedBigDecimalAggregatorFactoryBase.java | 4 ++-- .../ddsketch/DDSketchObjectStrategy.java | 16 +++++++++++---- .../MomentSketchObjectStrategy.java | 9 ++++++++- .../histogram/SpectatorHistogramTest.java | 14 ++++++------- .../TDigestSketchObjectStrategy.java | 11 ++++++++-- .../GoogleCloudStorageInputSourceTest.java | 4 ++-- .../histogram/ApproximateHistogram.java | 6 +++--- .../ApproximateHistogramFoldingSerde.java | 9 ++++++++- .../histogram/FixedBucketsHistogram.java | 19 ++++++++++++++++-- .../histogram/FixedBucketsHistogramSerde.java | 11 ++++++++-- .../druid/data/input/MapPopulatorTest.java | 2 +- .../druid/storage/s3/S3TaskLogsTest.java | 3 ++- .../output/RetryableS3OutputStreamTest.java | 6 +++--- .../aggregation/variance/VarianceSerde.java | 9 ++++++++- ...kableStreamSupervisorStateManagerTest.java | 2 +- .../apache/druid/msq/exec/ControllerImpl.java | 8 ++++---- .../ClusterByStatisticsCollectorImpl.java | 8 ++++---- .../QuantilesSketchKeyCollector.java | 2 +- .../apache/druid/crypto/CryptoService.java | 6 ++++-- .../druid/extendedset/intset/ConciseSet.java | 4 ++++ .../frame/key/FrameComparisonWidgetImpl.java | 4 ++-- .../druid/frame/read/FrameReaderUtils.java | 2 +- .../columnar/ComplexFrameColumnReader.java | 17 ++++++++++++---- .../columnar/StringFrameColumnReader.java | 16 ++++++++++----- .../common/granularity/PeriodGranularity.java | 6 +++--- .../util/common/io/smoosh/FileSmoosher.java | 10 ++++++---- .../io/AppendableByteArrayInputStream.java | 7 ++++--- .../math/expr/BinaryMathOperatorExpr.java | 12 +++++++++++ ...zablePairLongDoubleComplexMetricSerde.java | 2 +- ...izablePairLongFloatComplexMetricSerde.java | 2 +- ...lizablePairLongLongComplexMetricSerde.java | 2 +- ...zablePairLongStringComplexMetricSerde.java | 2 +- .../hyperloglog/HyperUniquesSerde.java | 2 +- .../druid/query/cache/CacheKeyBuilder.java | 2 +- .../groupby/orderby/DefaultLimitSpec.java | 10 ++++++---- .../rowsandcols/ArrayListRowsAndColumns.java | 11 +++++++--- .../rowsandcols/LimitedRowsAndColumns.java | 6 +++--- .../rowsandcols/RearrangedRowsAndColumns.java | 6 +++--- .../druid/segment/StringDimensionIndexer.java | 2 +- .../segment/data/CompressedBlockReader.java | 16 ++++++++------- .../data/CompressedColumnarIntsSupplier.java | 4 +++- .../segment/data/CompressionStrategy.java | 5 +++-- .../druid/segment/data/FixedIndexed.java | 14 +++++++++---- .../druid/segment/data/GenericIndexed.java | 2 +- .../data/ImmutableRTreeObjectStrategy.java | 2 +- .../segment/data/SafeWritableMemory.java | 5 +++-- .../druid/segment/data/VSizeLongSerde.java | 2 +- .../nested/NestedPathArrayElement.java | 20 +++++++++---------- .../druid/segment/nested/VariantColumn.java | 4 ++-- .../org/apache/druid/timeline/SegmentId.java | 2 ++ ...apOperationAgainstConsecutiveRunsTest.java | 3 ++- .../util/common/CompressionUtilsTest.java | 12 +++++------ ...alizablePairLongStringBufferStoreTest.java | 3 --- .../scan/ScanQueryRunnerFactoryTest.java | 4 ++-- .../segment/data/SafeWritableMemoryTest.java | 20 +++++++++---------- ...dVSizeColumnarMultiIntsSerializerTest.java | 5 +++-- .../serde/HyperUniquesSerdeForTest.java | 3 +++ .../segment/loading/StorageLocationTest.java | 2 +- 61 files changed, 274 insertions(+), 149 deletions(-) diff --git a/extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/data/input/aliyun/OssInputSourceTest.java b/extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/data/input/aliyun/OssInputSourceTest.java index 12ad9fc8bf9b..917ef5ddc116 100644 --- a/extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/data/input/aliyun/OssInputSourceTest.java +++ b/extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/data/input/aliyun/OssInputSourceTest.java @@ -551,7 +551,7 @@ public void testReader() throws IOException Assert.assertEquals("world", nextRow.getDimension("dim2").get(0)); } - Assert.assertEquals(2 * CONTENT.length, inputStats.getProcessedBytes()); + Assert.assertEquals(2L * CONTENT.length, inputStats.getProcessedBytes()); EasyMock.verify(OSSCLIENT); } @@ -598,7 +598,7 @@ public void testCompressedReader() throws IOException Assert.assertEquals("world", nextRow.getDimension("dim2").get(0)); } - Assert.assertEquals(2 * CONTENT.length, inputStats.getProcessedBytes()); + Assert.assertEquals(2L * CONTENT.length, inputStats.getProcessedBytes()); EasyMock.verify(OSSCLIENT); } diff --git a/extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/storage/aliyun/OssTaskLogsTest.java b/extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/storage/aliyun/OssTaskLogsTest.java index 16b09866ec4b..92c8424832e4 100644 --- a/extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/storage/aliyun/OssTaskLogsTest.java +++ b/extensions-contrib/aliyun-oss-extensions/src/test/java/org/apache/druid/storage/aliyun/OssTaskLogsTest.java @@ -347,7 +347,8 @@ public void test_taskLog_fetch_withNegativeRange() throws IOException EasyMock.replay(ossClient); OssTaskLogs ossTaskLogs = getOssTaskLogs(); - Optional inputStreamOptional = ossTaskLogs.streamTaskLog(KEY_1, -1 * (LOG_CONTENTS.length() - 1)); + final Optional inputStreamOptional = + ossTaskLogs.streamTaskLog(KEY_1, 1L - LOG_CONTENTS.length()); String taskLogs = new BufferedReader( new InputStreamReader(inputStreamOptional.get(), StandardCharsets.UTF_8)) .lines() diff --git a/extensions-contrib/compressed-bigdecimal/src/main/java/org/apache/druid/compressedbigdecimal/ByteBufferCompressedBigDecimal.java b/extensions-contrib/compressed-bigdecimal/src/main/java/org/apache/druid/compressedbigdecimal/ByteBufferCompressedBigDecimal.java index 1cc0f8153fa0..e49445b22b59 100644 --- a/extensions-contrib/compressed-bigdecimal/src/main/java/org/apache/druid/compressedbigdecimal/ByteBufferCompressedBigDecimal.java +++ b/extensions-contrib/compressed-bigdecimal/src/main/java/org/apache/druid/compressedbigdecimal/ByteBufferCompressedBigDecimal.java @@ -119,9 +119,9 @@ public int getArraySize() * @return the entry */ @Override - public int getArrayEntry(int idx) + public int getArrayEntry(final int idx) { - return buf.getInt(position + idx * Integer.BYTES); + return buf.getInt(bufferPosition(idx)); } /** @@ -131,9 +131,15 @@ public int getArrayEntry(int idx) * @param val value to set */ @Override - protected void setArrayEntry(int idx, int val) + protected void setArrayEntry(final int idx, final int val) { - buf.putInt(position + idx * Integer.BYTES, val); + buf.putInt(bufferPosition(idx), val); + } + + private int bufferPosition(final int idx) + { + Preconditions.checkElementIndex(idx, size); + return Math.addExact(position, Math.multiplyExact(idx, Integer.BYTES)); } @Override diff --git a/extensions-contrib/compressed-bigdecimal/src/main/java/org/apache/druid/compressedbigdecimal/aggregator/CompressedBigDecimalAggregatorFactoryBase.java b/extensions-contrib/compressed-bigdecimal/src/main/java/org/apache/druid/compressedbigdecimal/aggregator/CompressedBigDecimalAggregatorFactoryBase.java index cb444316c1b0..8bcac1b48ed4 100644 --- a/extensions-contrib/compressed-bigdecimal/src/main/java/org/apache/druid/compressedbigdecimal/aggregator/CompressedBigDecimalAggregatorFactoryBase.java +++ b/extensions-contrib/compressed-bigdecimal/src/main/java/org/apache/druid/compressedbigdecimal/aggregator/CompressedBigDecimalAggregatorFactoryBase.java @@ -86,9 +86,9 @@ protected CompressedBigDecimalAggregatorFactoryBase( this.scale = scale == null ? DEFAULT_SCALE : scale; this.strictNumberParsing = strictNumberParsing == null ? DEFAULT_STRICT_NUMBER_PARSING : strictNumberParsing; - byte[] fieldNameBytes = StringUtils.toUtf8(fieldName); + final byte[] fieldNameBytes = StringUtils.toUtf8(fieldName); - cacheKey = ByteBuffer.allocate(1 + fieldNameBytes.length + 2 * Integer.BYTES + 1) + cacheKey = ByteBuffer.allocate(Math.addExact(fieldNameBytes.length, 2 * Integer.BYTES + 2)) .put(cacheTypeId) .put(fieldNameBytes) .putInt(this.size) diff --git a/extensions-contrib/ddsketch/src/main/java/org/apache/druid/query/aggregation/ddsketch/DDSketchObjectStrategy.java b/extensions-contrib/ddsketch/src/main/java/org/apache/druid/query/aggregation/ddsketch/DDSketchObjectStrategy.java index 219e23f817b5..0f5607ea909a 100644 --- a/extensions-contrib/ddsketch/src/main/java/org/apache/druid/query/aggregation/ddsketch/DDSketchObjectStrategy.java +++ b/extensions-contrib/ddsketch/src/main/java/org/apache/druid/query/aggregation/ddsketch/DDSketchObjectStrategy.java @@ -22,6 +22,7 @@ import com.datadoghq.sketch.ddsketch.DDSketch; import com.datadoghq.sketch.ddsketch.DDSketchProtoBinding; import com.datadoghq.sketch.ddsketch.store.CollapsingLowestDenseStore; +import com.google.common.base.Preconditions; import com.google.protobuf.InvalidProtocolBufferException; import org.apache.druid.segment.data.ObjectStrategy; @@ -44,11 +45,18 @@ public DDSketch fromByteBuffer(ByteBuffer buffer, int numBytes) if (numBytes == 0) { return null; } - ByteBuffer readOnlyBuffer = buffer.asReadOnlyBuffer(); - readOnlyBuffer.limit(buffer.position() + numBytes); + Preconditions.checkArgument( + numBytes >= 0 && numBytes <= buffer.remaining(), + "numBytes[%s] exceeds buffer remaining[%s]", + numBytes, + buffer.remaining() + ); + final ByteBuffer readOnlyBuffer = buffer.asReadOnlyBuffer(); + readOnlyBuffer.limit(Math.addExact(buffer.position(), numBytes)); try { - com.datadoghq.sketch.ddsketch.proto.DDSketch proto = com.datadoghq.sketch.ddsketch.proto.DDSketch.parseFrom(readOnlyBuffer); - DDSketch recovered = DDSketchProtoBinding.fromProto(() -> new CollapsingLowestDenseStore(1000), proto); + final com.datadoghq.sketch.ddsketch.proto.DDSketch proto = + com.datadoghq.sketch.ddsketch.proto.DDSketch.parseFrom(readOnlyBuffer); + final DDSketch recovered = DDSketchProtoBinding.fromProto(() -> new CollapsingLowestDenseStore(1000), proto); return recovered; } catch (InvalidProtocolBufferException e) { diff --git a/extensions-contrib/momentsketch/src/main/java/org/apache/druid/query/aggregation/momentsketch/MomentSketchObjectStrategy.java b/extensions-contrib/momentsketch/src/main/java/org/apache/druid/query/aggregation/momentsketch/MomentSketchObjectStrategy.java index 77fa9dc0d4e5..bf1f8f827940 100644 --- a/extensions-contrib/momentsketch/src/main/java/org/apache/druid/query/aggregation/momentsketch/MomentSketchObjectStrategy.java +++ b/extensions-contrib/momentsketch/src/main/java/org/apache/druid/query/aggregation/momentsketch/MomentSketchObjectStrategy.java @@ -19,6 +19,7 @@ package org.apache.druid.query.aggregation.momentsketch; +import com.google.common.base.Preconditions; import org.apache.druid.query.aggregation.momentsketch.aggregator.MomentSketchAggregatorFactory; import org.apache.druid.segment.data.ObjectStrategy; @@ -41,7 +42,13 @@ public MomentSketchWrapper fromByteBuffer(ByteBuffer buffer, int numBytes) if (numBytes == 0) { return null; } - buffer.limit(buffer.position() + numBytes); + Preconditions.checkArgument( + numBytes > 0 && numBytes <= buffer.remaining(), + "numBytes[%s] exceeds buffer remaining[%s]", + numBytes, + buffer.remaining() + ); + buffer.limit(Math.addExact(buffer.position(), numBytes)); return MomentSketchWrapper.fromBytes(buffer); } diff --git a/extensions-contrib/spectator-histogram/src/test/java/org/apache/druid/spectator/histogram/SpectatorHistogramTest.java b/extensions-contrib/spectator-histogram/src/test/java/org/apache/druid/spectator/histogram/SpectatorHistogramTest.java index ac0d59189dbd..4aadc06a8d99 100644 --- a/extensions-contrib/spectator-histogram/src/test/java/org/apache/druid/spectator/histogram/SpectatorHistogramTest.java +++ b/extensions-contrib/spectator-histogram/src/test/java/org/apache/druid/spectator/histogram/SpectatorHistogramTest.java @@ -57,7 +57,7 @@ public void testToBytesSmallValues() byte[] bytes = histogram.toBytes(); int keySize = Short.BYTES; int valSize = 0; - Assert.assertEquals("Should compact small values within key bytes", 5 * (keySize + valSize), bytes.length); + Assert.assertEquals("Should compact small values within key bytes", 5L * (keySize + valSize), bytes.length); SpectatorHistogram deserialized = SpectatorHistogram.deserialize(bytes); Assert.assertEquals(1L, deserialized.get(PercentileBuckets.indexOf(10))); @@ -86,7 +86,7 @@ public void testToBytesSmallishValues() byte[] bytes = histogram.toBytes(); int keySize = Short.BYTES; int valSize = Byte.BYTES; - Assert.assertEquals("Should compact small values to a byte", 5 * (keySize + valSize), bytes.length); + Assert.assertEquals("Should compact small values to a byte", 5L * (keySize + valSize), bytes.length); SpectatorHistogram deserialized = SpectatorHistogram.deserialize(bytes); Assert.assertEquals(64L, deserialized.get(PercentileBuckets.indexOf(10))); @@ -115,7 +115,7 @@ public void testToBytesMedValues() byte[] bytes = histogram.toBytes(); int keySize = Short.BYTES; int valSize = Short.BYTES; - Assert.assertEquals("Should compact medium values to short", 5 * (keySize + valSize), bytes.length); + Assert.assertEquals("Should compact medium values to short", 5L * (keySize + valSize), bytes.length); SpectatorHistogram deserialized = SpectatorHistogram.deserialize(bytes); Assert.assertEquals(512L, deserialized.get(PercentileBuckets.indexOf(10))); @@ -144,7 +144,7 @@ public void testToBytesLargerValues() byte[] bytes = histogram.toBytes(); int keySize = Short.BYTES; int valSize = Integer.BYTES; - Assert.assertEquals("Should compact larger values to integer", 5 * (keySize + valSize), bytes.length); + Assert.assertEquals("Should compact larger values to integer", 5L * (keySize + valSize), bytes.length); SpectatorHistogram deserialized = SpectatorHistogram.deserialize(bytes); Assert.assertEquals(100000L, deserialized.get(PercentileBuckets.indexOf(10))); @@ -173,7 +173,7 @@ public void testToBytesBiggestValues() byte[] bytes = histogram.toBytes(); int keySize = Short.BYTES; int valSize = Long.BYTES; - Assert.assertEquals("Should not compact larger values", 5 * (keySize + valSize), bytes.length); + Assert.assertEquals("Should not compact larger values", 5L * (keySize + valSize), bytes.length); SpectatorHistogram deserialized = SpectatorHistogram.deserialize(bytes); Assert.assertEquals(10000000000L, deserialized.get(PercentileBuckets.indexOf(10))); @@ -201,7 +201,7 @@ public void testToBytesMixedValues() byte[] bytes = histogram.toBytes(); int keySize = Short.BYTES; - Assert.assertEquals("Should not compact larger values", (5 * keySize) + 0 + 2 + 4 + 8 + 8, bytes.length); + Assert.assertEquals("Should not compact larger values", (5L * keySize) + 0 + 2 + 4 + 8 + 8, bytes.length); SpectatorHistogram deserialized = SpectatorHistogram.deserialize(bytes); Assert.assertEquals(1L, deserialized.get(PercentileBuckets.indexOf(10))); @@ -232,7 +232,7 @@ public void testToBytesBoundaryValues() byte[] bytes = histogram.toBytes(); int keySize = Short.BYTES; - Assert.assertEquals("Should compact", (8 * keySize) + 0 + 1 + 1 + 2 + 2 + 4 + 4 + 8, bytes.length); + Assert.assertEquals("Should compact", (8L * keySize) + 0 + 1 + 1 + 2 + 2 + 4 + 4 + 8, bytes.length); SpectatorHistogram deserialized = SpectatorHistogram.deserialize(bytes); Assert.assertEquals(63L, deserialized.get(6)); diff --git a/extensions-contrib/tdigestsketch/src/main/java/org/apache/druid/query/aggregation/tdigestsketch/TDigestSketchObjectStrategy.java b/extensions-contrib/tdigestsketch/src/main/java/org/apache/druid/query/aggregation/tdigestsketch/TDigestSketchObjectStrategy.java index 1a660d90aff4..8200ecae6f51 100644 --- a/extensions-contrib/tdigestsketch/src/main/java/org/apache/druid/query/aggregation/tdigestsketch/TDigestSketchObjectStrategy.java +++ b/extensions-contrib/tdigestsketch/src/main/java/org/apache/druid/query/aggregation/tdigestsketch/TDigestSketchObjectStrategy.java @@ -19,6 +19,7 @@ package org.apache.druid.query.aggregation.tdigestsketch; +import com.google.common.base.Preconditions; import com.tdunning.math.stats.MergingDigest; import org.apache.druid.segment.data.ObjectStrategy; @@ -41,8 +42,14 @@ public MergingDigest fromByteBuffer(ByteBuffer buffer, int numBytes) if (numBytes == 0) { return null; } - ByteBuffer readOnlyBuffer = buffer.asReadOnlyBuffer(); - readOnlyBuffer.limit(buffer.position() + numBytes); + Preconditions.checkArgument( + numBytes > 0 && numBytes <= buffer.remaining(), + "numBytes[%s] exceeds buffer remaining[%s]", + numBytes, + buffer.remaining() + ); + final ByteBuffer readOnlyBuffer = buffer.asReadOnlyBuffer(); + readOnlyBuffer.limit(Math.addExact(buffer.position(), numBytes)); return MergingDigest.fromBytes(readOnlyBuffer); } diff --git a/extensions-core/google-extensions/src/test/java/org/apache/druid/data/input/google/GoogleCloudStorageInputSourceTest.java b/extensions-core/google-extensions/src/test/java/org/apache/druid/data/input/google/GoogleCloudStorageInputSourceTest.java index 77f0b9436fb5..7c9cabaa1015 100644 --- a/extensions-core/google-extensions/src/test/java/org/apache/druid/data/input/google/GoogleCloudStorageInputSourceTest.java +++ b/extensions-core/google-extensions/src/test/java/org/apache/druid/data/input/google/GoogleCloudStorageInputSourceTest.java @@ -419,7 +419,7 @@ public void testReader() throws IOException Assert.assertEquals("hello", nextRow.getDimension("dim1").get(0)); Assert.assertEquals("world", nextRow.getDimension("dim2").get(0)); } - Assert.assertEquals(2 * CONTENT.length, inputStats.getProcessedBytes()); + Assert.assertEquals(2L * CONTENT.length, inputStats.getProcessedBytes()); } @Test @@ -466,7 +466,7 @@ public void testCompressedReader() throws IOException Assert.assertEquals("hello", nextRow.getDimension("dim1").get(0)); Assert.assertEquals("world", nextRow.getDimension("dim2").get(0)); } - Assert.assertEquals(2 * CONTENT.length, inputStats.getProcessedBytes()); + Assert.assertEquals(2L * CONTENT.length, inputStats.getProcessedBytes()); } @Test diff --git a/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java b/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java index 11236b8d25fe..8cc563e9ecd5 100644 --- a/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java +++ b/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java @@ -1261,7 +1261,7 @@ public void toBytesCompact(ByteBuffer buf) for (int i = 0; i < binCount; ++i) { // repeat each value bins[i] times for approximate bins if ((bins[i] & APPROX_FLAG_BIT) != 0) { - for (int k = 0; k < (bins[i] & COUNT_BITS); ++k) { + for (long k = 0; k < (bins[i] & COUNT_BITS); ++k) { buf.putFloat(positions[i]); } } @@ -1277,7 +1277,7 @@ public void toBytesCompact(ByteBuffer buf) for (int i = 0; i < binCount; ++i) { // repeat each value bins[i] times for exact bins if ((bins[i] & APPROX_FLAG_BIT) == 0) { - for (int k = 0; k < (bins[i] & COUNT_BITS); ++k) { + for (long k = 0; k < (bins[i] & COUNT_BITS); ++k) { buf.putFloat(positions[i]); } } @@ -1371,7 +1371,7 @@ public static ApproximateHistogram fromBytesCompact(ByteBuffer buf) } return histogram; } else { - byte approxCount = (byte) (-1 * count); + final int approxCount = -count; Map approx = new HashMap<>(); diff --git a/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogramFoldingSerde.java b/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogramFoldingSerde.java index 31784f0d744a..7a5565f2f9b4 100644 --- a/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogramFoldingSerde.java +++ b/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogramFoldingSerde.java @@ -19,6 +19,7 @@ package org.apache.druid.query.aggregation.histogram; +import com.google.common.base.Preconditions; import it.unimi.dsi.fastutil.bytes.ByteArrays; import org.apache.druid.data.input.InputRow; import org.apache.druid.data.input.Rows; @@ -93,7 +94,13 @@ public Class getClazz() @Override public ApproximateHistogram fromByteBuffer(ByteBuffer buffer, int numBytes) { - buffer.limit(buffer.position() + numBytes); + Preconditions.checkArgument( + numBytes >= 0 && numBytes <= buffer.remaining(), + "numBytes[%s] exceeds buffer remaining[%s]", + numBytes, + buffer.remaining() + ); + buffer.limit(Math.addExact(buffer.position(), numBytes)); return ApproximateHistogram.fromBytes(buffer); } diff --git a/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogram.java b/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogram.java index 457891499292..a18d2964a4d3 100644 --- a/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogram.java +++ b/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogram.java @@ -993,7 +993,7 @@ protected static FixedBucketsHistogram fromByteBufferFullNoSerdeHeader(ByteBuffe double lowerLimit = buf.getDouble(); double upperLimit = buf.getDouble(); int numBuckets = buf.getInt(); - OutlierHandlingMode outlierHandlingMode = OutlierHandlingMode.values()[buf.get()]; + final OutlierHandlingMode outlierHandlingMode = readOutlierHandlingMode(buf); long count = buf.getLong(); long lowerOutlierCount = buf.getLong(); @@ -1034,7 +1034,7 @@ private static FixedBucketsHistogram fromBytesSparse(ByteBuffer buf) double lowerLimit = buf.getDouble(); double upperLimit = buf.getDouble(); int numBuckets = buf.getInt(); - OutlierHandlingMode outlierHandlingMode = OutlierHandlingMode.values()[buf.get()]; + final OutlierHandlingMode outlierHandlingMode = readOutlierHandlingMode(buf); long count = buf.getLong(); long lowerOutlierCount = buf.getLong(); @@ -1067,6 +1067,21 @@ private static FixedBucketsHistogram fromBytesSparse(ByteBuffer buf) ); } + private static OutlierHandlingMode readOutlierHandlingMode(final ByteBuffer buf) + { + final byte mode = buf.get(); + switch (mode) { + case 0: + return OutlierHandlingMode.IGNORE; + case 1: + return OutlierHandlingMode.OVERFLOW; + case 2: + return OutlierHandlingMode.CLIP; + default: + throw new ISE("Invalid outlier handling mode [%s].", mode); + } + } + /** * Compute the size in bytes of a full-encoding serialized histogram, without the serialization header * diff --git a/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogramSerde.java b/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogramSerde.java index 0a63041828e9..d2068db435d2 100644 --- a/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogramSerde.java +++ b/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/FixedBucketsHistogramSerde.java @@ -19,6 +19,7 @@ package org.apache.druid.query.aggregation.histogram; +import com.google.common.base.Preconditions; import com.google.common.collect.Ordering; import org.apache.druid.data.input.InputRow; import org.apache.druid.data.input.Rows; @@ -134,8 +135,14 @@ public Class getClazz() @Override public FixedBucketsHistogram fromByteBuffer(ByteBuffer buffer, int numBytes) { - buffer.limit(buffer.position() + numBytes); - FixedBucketsHistogram fbh = FixedBucketsHistogram.fromByteBuffer(buffer); + Preconditions.checkArgument( + numBytes >= 0 && numBytes <= buffer.remaining(), + "numBytes[%s] exceeds buffer remaining[%s]", + numBytes, + buffer.remaining() + ); + buffer.limit(Math.addExact(buffer.position(), numBytes)); + final FixedBucketsHistogram fbh = FixedBucketsHistogram.fromByteBuffer(buffer); return fbh; } diff --git a/extensions-core/lookups-cached-global/src/test/java/org/apache/druid/data/input/MapPopulatorTest.java b/extensions-core/lookups-cached-global/src/test/java/org/apache/druid/data/input/MapPopulatorTest.java index 99aa8ac1c801..c6ca71801918 100644 --- a/extensions-core/lookups-cached-global/src/test/java/org/apache/druid/data/input/MapPopulatorTest.java +++ b/extensions-core/lookups-cached-global/src/test/java/org/apache/druid/data/input/MapPopulatorTest.java @@ -39,7 +39,7 @@ public class MapPopulatorTest public void test_getByteLengthOfObject_string_stringLength() { String o = "string"; - Assert.assertEquals((o.length() * Character.BYTES) + 40, MapPopulator.getByteLengthOfObject(o)); + Assert.assertEquals(((long) o.length() * Character.BYTES) + 40, MapPopulator.getByteLengthOfObject(o)); } @Test diff --git a/extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3TaskLogsTest.java b/extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3TaskLogsTest.java index 2516b62ba4c8..9adcdd1198f0 100644 --- a/extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3TaskLogsTest.java +++ b/extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3TaskLogsTest.java @@ -511,7 +511,8 @@ public void test_taskLog_fetch_withNegativeRange() throws IOException S3TaskLogs s3TaskLogs = getS3TaskLogs(); - Optional inputStreamOptional = s3TaskLogs.streamTaskLog(KEY_1, -1 * (LOG_CONTENTS.length() - 1)); + final Optional inputStreamOptional = + s3TaskLogs.streamTaskLog(KEY_1, 1L - LOG_CONTENTS.length()); String taskLogs = new BufferedReader( new InputStreamReader(inputStreamOptional.get(), StandardCharsets.UTF_8)) .lines() diff --git a/extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/output/RetryableS3OutputStreamTest.java b/extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/output/RetryableS3OutputStreamTest.java index 170401acd6e4..8db8c780ee56 100644 --- a/extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/output/RetryableS3OutputStreamTest.java +++ b/extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/output/RetryableS3OutputStreamTest.java @@ -126,7 +126,7 @@ public void testWriteAndHappy() throws IOException } // each chunk is 10 bytes, so there should be 10 chunks. Assert.assertEquals(10, s3.partRequests.size()); - s3.assertCompleted(chunkSize, Integer.BYTES * 25); + s3.assertCompleted(chunkSize, Integer.BYTES * 25L); } @Test @@ -144,7 +144,7 @@ public void testWriteSizeLargerThanConfiguredMaxChunkSizeShouldSucceed() throws } // each chunk 10 bytes, so there should be 2 chunks. Assert.assertEquals(2, s3.partRequests.size()); - s3.assertCompleted(chunkSize, Integer.BYTES * 3); + s3.assertCompleted(chunkSize, Integer.BYTES * 3L); } @Test @@ -195,7 +195,7 @@ public void testSuccessToUploadAfterRetry() throws IOException } // each chunk is 10 bytes, so there should be 10 chunks. Assert.assertEquals(10, s3.partRequests.size()); - s3.assertCompleted(chunkSize, Integer.BYTES * 25); + s3.assertCompleted(chunkSize, Integer.BYTES * 25L); } @Test diff --git a/extensions-core/stats/src/main/java/org/apache/druid/query/aggregation/variance/VarianceSerde.java b/extensions-core/stats/src/main/java/org/apache/druid/query/aggregation/variance/VarianceSerde.java index 5b6832278002..99b7416bb937 100644 --- a/extensions-core/stats/src/main/java/org/apache/druid/query/aggregation/variance/VarianceSerde.java +++ b/extensions-core/stats/src/main/java/org/apache/druid/query/aggregation/variance/VarianceSerde.java @@ -19,6 +19,7 @@ package org.apache.druid.query.aggregation.variance; +import com.google.common.base.Preconditions; import com.google.common.collect.Ordering; import org.apache.druid.data.input.InputRow; import org.apache.druid.segment.data.ObjectStrategy; @@ -90,7 +91,13 @@ public Class getClazz() @Override public VarianceAggregatorCollector fromByteBuffer(ByteBuffer buffer, int numBytes) { - buffer.limit(buffer.position() + numBytes); + Preconditions.checkArgument( + numBytes >= 0 && numBytes <= buffer.remaining(), + "numBytes[%s] exceeds buffer remaining[%s]", + numBytes, + buffer.remaining() + ); + buffer.limit(Math.addExact(buffer.position(), numBytes)); return VarianceAggregatorCollector.from(buffer); } diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorStateManagerTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorStateManagerTest.java index 6ffe9dcdfc5d..91f4e5ee8abf 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorStateManagerTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorStateManagerTest.java @@ -235,7 +235,7 @@ public void testTransientUnhealthiness() stateManager.markRunFinished(); // clean run Assert.assertEquals(BasicState.RUNNING, stateManager.getSupervisorState()); Assert.assertEquals(BasicState.RUNNING, stateManager.getSupervisorState().getBasicState()); - Assert.assertEquals(j * (config.getUnhealthinessThreshold() - 1), stateManager.getExceptionEvents().size()); + Assert.assertEquals((long) j * (config.getUnhealthinessThreshold() - 1), stateManager.getExceptionEvents().size()); } } diff --git a/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java b/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java index 17a729f23484..789fae0d1327 100644 --- a/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java +++ b/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java @@ -976,15 +976,15 @@ public void workerWarning(List errorReports) { // This check safeguards that the controller doesn't run out of memory. Workers apply their own limiting to // protect their own memory, and to conserve worker -> controller bandwidth. - long numReportsToAddCheck = Math.min( + final int numReportsToAddCheck = Math.min( errorReports.size(), - Limits.MAX_WORKERS * Limits.MAX_VERBOSE_WARNINGS - workerWarnings.size() + Math.toIntExact(Limits.MAX_WORKERS * Limits.MAX_VERBOSE_WARNINGS - workerWarnings.size()) ); if (numReportsToAddCheck > 0) { synchronized (workerWarnings) { - long numReportsToAdd = Math.min( + final int numReportsToAdd = Math.min( errorReports.size(), - Limits.MAX_WORKERS * Limits.MAX_VERBOSE_WARNINGS - workerWarnings.size() + Math.toIntExact(Limits.MAX_WORKERS * Limits.MAX_VERBOSE_WARNINGS - workerWarnings.size()) ); for (int i = 0; i < numReportsToAdd; ++i) { workerWarnings.add(errorReports.get(i)); diff --git a/multi-stage-query/src/main/java/org/apache/druid/msq/statistics/ClusterByStatisticsCollectorImpl.java b/multi-stage-query/src/main/java/org/apache/druid/msq/statistics/ClusterByStatisticsCollectorImpl.java index e915270e7d7b..51a167796114 100644 --- a/multi-stage-query/src/main/java/org/apache/druid/msq/statistics/ClusterByStatisticsCollectorImpl.java +++ b/multi-stage-query/src/main/java/org/apache/druid/msq/statistics/ClusterByStatisticsCollectorImpl.java @@ -474,7 +474,7 @@ private void assertRetainedByteCountsAreTrackedCorrectly() private static class BucketHolder { private final KeyCollector keyCollector; - private double retainedBytes; + private long retainedBytes; public BucketHolder(final KeyCollector keyCollector) { @@ -482,10 +482,10 @@ public BucketHolder(final KeyCollector keyCollector) this.retainedBytes = keyCollector.estimatedRetainedBytes(); } - public double updateRetainedBytes() + public long updateRetainedBytes() { - final double newRetainedBytes = keyCollector.estimatedRetainedBytes(); - final double difference = newRetainedBytes - retainedBytes; + final long newRetainedBytes = keyCollector.estimatedRetainedBytes(); + final long difference = newRetainedBytes - retainedBytes; retainedBytes = newRetainedBytes; return difference; } diff --git a/multi-stage-query/src/main/java/org/apache/druid/msq/statistics/QuantilesSketchKeyCollector.java b/multi-stage-query/src/main/java/org/apache/druid/msq/statistics/QuantilesSketchKeyCollector.java index 6f1f1db0de75..a351e945b9f2 100644 --- a/multi-stage-query/src/main/java/org/apache/druid/msq/statistics/QuantilesSketchKeyCollector.java +++ b/multi-stage-query/src/main/java/org/apache/druid/msq/statistics/QuantilesSketchKeyCollector.java @@ -66,7 +66,7 @@ public void add(RowKey key, long weight) double estimatedTotalSketchSizeInBytes = averageKeyLength * sketch.getN(); // The key is added "weight" times to the sketch, we can update the total weight directly. estimatedTotalSketchSizeInBytes += key.estimatedObjectSizeBytes() * weight; - for (int i = 0; i < weight; i++) { + for (long i = 0; i < weight; i++) { // Add the same key multiple times to make it "heavier". sketch.update(key.array()); } diff --git a/processing/src/main/java/org/apache/druid/crypto/CryptoService.java b/processing/src/main/java/org/apache/druid/crypto/CryptoService.java index 9e63c0adfa94..a762fba228c7 100644 --- a/processing/src/main/java/org/apache/druid/crypto/CryptoService.java +++ b/processing/src/main/java/org/apache/druid/crypto/CryptoService.java @@ -188,8 +188,10 @@ public byte[] getCipher() public byte[] toByteAray() { - int headerLength = 12; - ByteBuffer bb = ByteBuffer.allocate(salt.length + iv.length + cipher.length + headerLength); + final int headerLength = 12; + final int encryptedDataLength = + Math.addExact(Math.addExact(Math.addExact(salt.length, iv.length), cipher.length), headerLength); + final ByteBuffer bb = ByteBuffer.allocate(encryptedDataLength); bb.putInt(salt.length) .putInt(iv.length) .putInt(cipher.length) diff --git a/processing/src/main/java/org/apache/druid/extendedset/intset/ConciseSet.java b/processing/src/main/java/org/apache/druid/extendedset/intset/ConciseSet.java index 5bce5616a7b5..23b0d33f999d 100755 --- a/processing/src/main/java/org/apache/druid/extendedset/intset/ConciseSet.java +++ b/processing/src/main/java/org/apache/druid/extendedset/intset/ConciseSet.java @@ -1179,11 +1179,15 @@ public int size() } else { if (isZeroSequence(w)) { if (!isSequenceWithNoBits(w)) { + // Cardinality is bounded by MAX_ALLOWED_INTEGER + 1, so this increment cannot overflow. + // codeql[java/tainted-arithmetic] size++; } } else { size += maxLiteralLengthMultiplication(getSequenceCount(w) + 1); if (!isSequenceWithNoBits(w)) { + // This branch follows addition of at least one literal and therefore cannot underflow. + // codeql[java/tainted-arithmetic] size--; } } diff --git a/processing/src/main/java/org/apache/druid/frame/key/FrameComparisonWidgetImpl.java b/processing/src/main/java/org/apache/druid/frame/key/FrameComparisonWidgetImpl.java index 56afed0f57e9..345f841147d2 100644 --- a/processing/src/main/java/org/apache/druid/frame/key/FrameComparisonWidgetImpl.java +++ b/processing/src/main/java/org/apache/druid/frame/key/FrameComparisonWidgetImpl.java @@ -208,8 +208,8 @@ public RowKey readKey(int row) final int keyEndInRow = dataRegion.getInt(rowPosition + (long) (keyFieldCount - 1) * Integer.BYTES); - final long keyLength = keyEndInRow - firstFieldPosition; - final byte[] keyBytes = new byte[Ints.checkedCast(keyFieldPointersEndInRow + keyEndInRow - firstFieldPosition)]; + final int keyLength = Ints.checkedCast((long) keyEndInRow - firstFieldPosition); + final byte[] keyBytes = new byte[Math.addExact(keyFieldPointersEndInRow, keyLength)]; // Length of the portion of the header which isn't included in the rowKey final int headerSizeAdjustment = (signature.size() - keyFieldCount) * Integer.BYTES; diff --git a/processing/src/main/java/org/apache/druid/frame/read/FrameReaderUtils.java b/processing/src/main/java/org/apache/druid/frame/read/FrameReaderUtils.java index 4dc032978363..e3a1d024a4b5 100644 --- a/processing/src/main/java/org/apache/druid/frame/read/FrameReaderUtils.java +++ b/processing/src/main/java/org/apache/druid/frame/read/FrameReaderUtils.java @@ -60,7 +60,7 @@ public static ByteBuffer readByteBuffer(final Memory memory, final long dataStar if (memory.hasByteBuffer()) { // Avoid data copy final ByteBuffer byteBuffer = memory.getByteBuffer().duplicate(); - byteBuffer.limit(Ints.checkedCast(memory.getRegionOffset(dataStart + dataLength))); + byteBuffer.limit(Ints.checkedCast(memory.getRegionOffset(Math.addExact(dataStart, dataLength)))); byteBuffer.position(Ints.checkedCast(memory.getRegionOffset(dataStart))); return byteBuffer; } else { diff --git a/processing/src/main/java/org/apache/druid/frame/read/columnar/ComplexFrameColumnReader.java b/processing/src/main/java/org/apache/druid/frame/read/columnar/ComplexFrameColumnReader.java index fc30d7c996f6..766f7d89fb57 100644 --- a/processing/src/main/java/org/apache/druid/frame/read/columnar/ComplexFrameColumnReader.java +++ b/processing/src/main/java/org/apache/druid/frame/read/columnar/ComplexFrameColumnReader.java @@ -236,22 +236,31 @@ protected Comparator getComparator() private Object getObjectForPhysicalRow(final int physicalRow) { final long endOffset = - startOfDataSection + memory.getInt(startOfOffsetSection + (long) Integer.BYTES * physicalRow); + Math.addExact( + startOfDataSection, + memory.getInt(Math.addExact(startOfOffsetSection, (long) Integer.BYTES * physicalRow)) + ); final long startOffset; if (physicalRow == 0) { startOffset = startOfDataSection; } else { startOffset = - startOfDataSection + memory.getInt(startOfOffsetSection + (long) Integer.BYTES * (physicalRow - 1)); + Math.addExact( + startOfDataSection, + memory.getInt( + Math.addExact(startOfOffsetSection, (long) Integer.BYTES * Math.subtractExact(physicalRow, 1)) + ) + ); } if (memory.getByte(startOffset) == ComplexFrameMaker.NULL_MARKER) { return null; } else { - final int payloadLength = Ints.checkedCast(endOffset - startOffset - Byte.BYTES); + final int payloadLength = + Ints.checkedCast(Math.subtractExact(Math.subtractExact(endOffset, startOffset), Byte.BYTES)); final byte[] complexBytes = new byte[payloadLength]; - memory.getByteArray(startOffset + Byte.BYTES, complexBytes, 0, payloadLength); + memory.getByteArray(Math.addExact(startOffset, Byte.BYTES), complexBytes, 0, payloadLength); return serde.fromBytes(complexBytes, 0, complexBytes.length); } } diff --git a/processing/src/main/java/org/apache/druid/frame/read/columnar/StringFrameColumnReader.java b/processing/src/main/java/org/apache/druid/frame/read/columnar/StringFrameColumnReader.java index d1216b5c2714..34ee5a5958e3 100644 --- a/processing/src/main/java/org/apache/druid/frame/read/columnar/StringFrameColumnReader.java +++ b/processing/src/main/java/org/apache/druid/frame/read/columnar/StringFrameColumnReader.java @@ -523,18 +523,24 @@ private ByteBuffer getStringUtf8(final int index) { final long dataStart; final long dataEnd = - startOfStringDataSection + - memory.getInt(startOfStringLengthSection + (long) Integer.BYTES * index); + Math.addExact( + startOfStringDataSection, + memory.getInt(Math.addExact(startOfStringLengthSection, (long) Integer.BYTES * index)) + ); if (index == 0) { dataStart = startOfStringDataSection; } else { dataStart = - startOfStringDataSection + - memory.getInt(startOfStringLengthSection + (long) Integer.BYTES * (index - 1)); + Math.addExact( + startOfStringDataSection, + memory.getInt( + Math.addExact(startOfStringLengthSection, (long) Integer.BYTES * Math.subtractExact(index, 1)) + ) + ); } - final int dataLength = Ints.checkedCast(dataEnd - dataStart); + final int dataLength = Ints.checkedCast(Math.subtractExact(dataEnd, dataStart)); if (dataLength == 1 && memory.getByte(dataStart) == FrameWriterUtils.NULL_STRING_MARKER) { return null; diff --git a/processing/src/main/java/org/apache/druid/java/util/common/granularity/PeriodGranularity.java b/processing/src/main/java/org/apache/druid/java/util/common/granularity/PeriodGranularity.java index 909e80f72c95..df8730555695 100644 --- a/processing/src/main/java/org/apache/druid/java/util/common/granularity/PeriodGranularity.java +++ b/processing/src/main/java/org/apache/druid/java/util/common/granularity/PeriodGranularity.java @@ -595,11 +595,11 @@ private long truncateMillisPeriod(final long t) // which may not always be the case, e.g if there are daylight saving changes. if (chronology.days().isPrecise() && chronology.hours().isPrecise()) { final long millis = period.toStandardDuration().getMillis(); - long offset = t % millis - origin % millis; + long offset = Math.subtractExact(t % millis, origin % millis); if (offset < 0) { - offset += millis; + offset = Math.addExact(offset, millis); } - return t - offset; + return Math.subtractExact(t, offset); } else { throw new UnsupportedOperationException( "Period cannot be converted to milliseconds as some fields mays vary in length with chronology " + chronology diff --git a/processing/src/main/java/org/apache/druid/java/util/common/io/smoosh/FileSmoosher.java b/processing/src/main/java/org/apache/druid/java/util/common/io/smoosh/FileSmoosher.java index f6166e65648c..48c9b2ff8782 100644 --- a/processing/src/main/java/org/apache/druid/java/util/common/io/smoosh/FileSmoosher.java +++ b/processing/src/main/java/org/apache/druid/java/util/common/io/smoosh/FileSmoosher.java @@ -356,9 +356,10 @@ public int addToOffset(long numBytesWritten) if (numBytesWritten > bytesLeft()) { throw new ISE("Wrote more bytes[%,d] than available[%,d]. Don't do that.", numBytesWritten, bytesLeft()); } - currOffset += numBytesWritten; + final int numBytesWrittenInt = Ints.checkedCast(numBytesWritten); + currOffset = Math.addExact(currOffset, numBytesWrittenInt); - return Ints.checkedCast(numBytesWritten); + return numBytesWrittenInt; } @Override @@ -489,9 +490,10 @@ public int addToOffset(long numBytesWritten) if (numBytesWritten > bytesLeft()) { throw new ISE("Wrote more bytes[%,d] than available[%,d]. Don't do that.", numBytesWritten, bytesLeft()); } - currOffset += numBytesWritten; + final int numBytesWrittenInt = Ints.checkedCast(numBytesWritten); + currOffset = Math.addExact(currOffset, numBytesWrittenInt); - return Ints.checkedCast(numBytesWritten); + return numBytesWrittenInt; } @Override diff --git a/processing/src/main/java/org/apache/druid/java/util/http/client/io/AppendableByteArrayInputStream.java b/processing/src/main/java/org/apache/druid/java/util/http/client/io/AppendableByteArrayInputStream.java index 6789faf56a76..72cfee058285 100644 --- a/processing/src/main/java/org/apache/druid/java/util/http/client/io/AppendableByteArrayInputStream.java +++ b/processing/src/main/java/org/apache/druid/java/util/http/client/io/AppendableByteArrayInputStream.java @@ -19,6 +19,7 @@ package org.apache.druid.java.util.http.client.io; +import com.google.common.primitives.Ints; import org.apache.druid.java.util.common.logger.Logger; import java.io.IOException; @@ -137,7 +138,7 @@ private long scanThroughBytesAndDoSomething(long numToScan, Doer doer) throws IO break; } try { - available -= numPulled; + available = Math.subtractExact(available, Ints.checkedCast(numPulled)); numPulled = 0; singleByteReaderDoer.wait(); } @@ -168,12 +169,12 @@ private long scanThroughBytesAndDoSomething(long numToScan, Doer doer) throws IO final long numToPullFromCurr = Math.min(curr.length - currIndex, numToScan - numScanned); doer.doSomethingWithByteArray((int) numToPullFromCurr); numScanned += numToPullFromCurr; - currIndex += numToPullFromCurr; + currIndex = Math.addExact(currIndex, Ints.checkedCast(numToPullFromCurr)); numPulled += numToPullFromCurr; } synchronized (singleByteReaderDoer) { - available -= numPulled; + available = Math.subtractExact(available, Ints.checkedCast(numPulled)); } return numScanned; diff --git a/processing/src/main/java/org/apache/druid/math/expr/BinaryMathOperatorExpr.java b/processing/src/main/java/org/apache/druid/math/expr/BinaryMathOperatorExpr.java index bffc0435335b..bf483f0ab5c4 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/BinaryMathOperatorExpr.java +++ b/processing/src/main/java/org/apache/druid/math/expr/BinaryMathOperatorExpr.java @@ -59,12 +59,16 @@ protected ExprEval evalString(@Nullable String left, @Nullable String right) @Override protected long evalLong(long left, long right) { + // Druid expression arithmetic follows Java's wrapping long semantics. + // codeql[java/tainted-arithmetic] return left + right; } @Override protected double evalDouble(double left, double right) { + // Druid expression arithmetic follows IEEE 754 semantics, including infinity. + // codeql[java/tainted-arithmetic] return left + right; } @@ -105,12 +109,16 @@ protected BinaryOpExprBase copy(Expr left, Expr right) @Override protected long evalLong(long left, long right) { + // Druid expression arithmetic follows Java's wrapping long semantics. + // codeql[java/tainted-arithmetic] return left - right; } @Override protected double evalDouble(double left, double right) { + // Druid expression arithmetic follows IEEE 754 semantics, including infinity. + // codeql[java/tainted-arithmetic] return left - right; } @@ -144,12 +152,16 @@ protected BinaryOpExprBase copy(Expr left, Expr right) @Override protected long evalLong(long left, long right) { + // Druid expression arithmetic follows Java's wrapping long semantics. + // codeql[java/tainted-arithmetic] return left * right; } @Override protected double evalDouble(double left, double right) { + // Druid expression arithmetic follows IEEE 754 semantics, including infinity. + // codeql[java/tainted-arithmetic] return left * right; } diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongDoubleComplexMetricSerde.java b/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongDoubleComplexMetricSerde.java index 692639e87c0c..61f3327ade6a 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongDoubleComplexMetricSerde.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongDoubleComplexMetricSerde.java @@ -103,7 +103,7 @@ public SerializablePairLongDouble fromByteBuffer(ByteBuffer buffer, int numBytes { ByteBuffer readOnlyByteBuffer = buffer.asReadOnlyBuffer().order(buffer.order()); - readOnlyByteBuffer.limit(buffer.position() + numBytes); + readOnlyByteBuffer.limit(Math.addExact(buffer.position(), numBytes)); return SERDE.deserialize(readOnlyByteBuffer); } diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongFloatComplexMetricSerde.java b/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongFloatComplexMetricSerde.java index 9c5841be9354..1ac4493065df 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongFloatComplexMetricSerde.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongFloatComplexMetricSerde.java @@ -104,7 +104,7 @@ public SerializablePairLongFloat fromByteBuffer(ByteBuffer buffer, int numBytes) { ByteBuffer readOnlyByteBuffer = buffer.asReadOnlyBuffer().order(buffer.order()); - readOnlyByteBuffer.limit(buffer.position() + numBytes); + readOnlyByteBuffer.limit(Math.addExact(buffer.position(), numBytes)); return SERDE.deserialize(readOnlyByteBuffer); } diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongLongComplexMetricSerde.java b/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongLongComplexMetricSerde.java index 2be1d434c442..8faa1c99c6d2 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongLongComplexMetricSerde.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongLongComplexMetricSerde.java @@ -103,7 +103,7 @@ public SerializablePairLongLong fromByteBuffer(ByteBuffer buffer, int numBytes) { ByteBuffer readOnlyByteBuffer = buffer.asReadOnlyBuffer().order(buffer.order()); - readOnlyByteBuffer.limit(buffer.position() + numBytes); + readOnlyByteBuffer.limit(Math.addExact(buffer.position(), numBytes)); return SERDE.deserialize(readOnlyByteBuffer); } diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongStringComplexMetricSerde.java b/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongStringComplexMetricSerde.java index 4f3171d8f673..f5d2a4b013f8 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongStringComplexMetricSerde.java +++ b/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)); return SERDE.deserialize(readOnlyByteBuffer); } diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/hyperloglog/HyperUniquesSerde.java b/processing/src/main/java/org/apache/druid/query/aggregation/hyperloglog/HyperUniquesSerde.java index bd8c9093f93c..8904c6230e8a 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/hyperloglog/HyperUniquesSerde.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/hyperloglog/HyperUniquesSerde.java @@ -107,7 +107,7 @@ public HyperLogLogCollector fromByteBuffer(ByteBuffer buffer, int numBytes) // make a copy of buffer, because the given buffer is not duplicated in HyperLogLogCollector.makeCollector() and // stored in a field. final ByteBuffer readOnlyBuffer = buffer.asReadOnlyBuffer(); - readOnlyBuffer.limit(readOnlyBuffer.position() + numBytes); + readOnlyBuffer.limit(Math.addExact(readOnlyBuffer.position(), numBytes)); return HyperLogLogCollector.makeCollector(readOnlyBuffer); } diff --git a/processing/src/main/java/org/apache/druid/query/cache/CacheKeyBuilder.java b/processing/src/main/java/org/apache/druid/query/cache/CacheKeyBuilder.java index 4a0de49c5c55..ac43338c30ba 100644 --- a/processing/src/main/java/org/apache/druid/query/cache/CacheKeyBuilder.java +++ b/processing/src/main/java/org/apache/druid/query/cache/CacheKeyBuilder.java @@ -85,7 +85,7 @@ private static class Item int byteSize() { - return 1 + item.length; + return Math.addExact(1, item.length); } } diff --git a/processing/src/main/java/org/apache/druid/query/groupby/orderby/DefaultLimitSpec.java b/processing/src/main/java/org/apache/druid/query/groupby/orderby/DefaultLimitSpec.java index 0c04839c3d2c..789c54f4bcb7 100644 --- a/processing/src/main/java/org/apache/druid/query/groupby/orderby/DefaultLimitSpec.java +++ b/processing/src/main/java/org/apache/druid/query/groupby/orderby/DefaultLimitSpec.java @@ -318,11 +318,13 @@ public DefaultLimitSpec withOffsetToLimit() if (limit == Integer.MAX_VALUE) { // Unlimited stays unlimited. newLimit = Integer.MAX_VALUE; - } else if (limit > Integer.MAX_VALUE - offset) { - // Handle overflow as best we can. - throw new ISE("Cannot apply limit[%d] with offset[%d] due to overflow", limit, offset); } else { - newLimit = limit + offset; + try { + newLimit = Math.addExact(limit, offset); + } + catch (ArithmeticException e) { + throw new ISE("Cannot apply limit[%d] with offset[%d] due to overflow", limit, offset); + } } return new DefaultLimitSpec(columns, 0, newLimit); diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java index dc7a4a355644..775f79887c04 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/ArrayListRowsAndColumns.java @@ -115,8 +115,13 @@ private ArrayListRowsAndColumns( int endOffset ) { - if (endOffset - startOffset < 0) { - throw new ISE("endOffset[%,d] - startOffset[%,d] was somehow negative!?", endOffset, startOffset); + if (startOffset < 0 || endOffset < startOffset || endOffset > rows.size()) { + throw new ISE( + "Invalid row range startOffset[%,d], endOffset[%,d], rows.size()[%,d]", + startOffset, + endOffset, + rows.size() + ); } this.rows = rows; this.rowAdapter = rowAdapter; @@ -136,7 +141,7 @@ public Collection getColumnNames() @Override public int numRows() { - return endOffset - startOffset; + return Math.subtractExact(endOffset, startOffset); } @Override diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/LimitedRowsAndColumns.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/LimitedRowsAndColumns.java index 30d1c4d717a1..438ab8c16e42 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/LimitedRowsAndColumns.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/LimitedRowsAndColumns.java @@ -35,8 +35,8 @@ public class LimitedRowsAndColumns implements RowsAndColumns public LimitedRowsAndColumns(RowsAndColumns rac, int start, int end) { final int numRows = rac.numRows(); - if (numRows < end) { - throw new ISE("end[%d] is out of bounds, cannot be greater than numRows[%d]", end, numRows); + if (start < 0 || end < start || end > numRows) { + throw new ISE("Invalid row range start[%d], end[%d], numRows[%d]", start, end, numRows); } this.rac = rac; @@ -53,7 +53,7 @@ public Collection getColumnNames() @Override public int numRows() { - return end - start; + return Math.subtractExact(end, start); } @Override diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/RearrangedRowsAndColumns.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/RearrangedRowsAndColumns.java index f1793f8fd0e4..864463282d54 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/RearrangedRowsAndColumns.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/RearrangedRowsAndColumns.java @@ -67,8 +67,8 @@ public RearrangedRowsAndColumns( RowsAndColumns rac ) { - if (end - start < 0 || end > pointers.length) { - throw new IAE("end[%,d] - start[%,d] was invalid!? pointers.length[%,d]", end, start, pointers.length); + if (start < 0 || end < start || end > pointers.length) { + throw new IAE("Invalid row range start[%,d], end[%,d], pointers.length[%,d]", start, end, pointers.length); } this.pointers = pointers; this.start = start; @@ -85,7 +85,7 @@ public Collection getColumnNames() @Override public int numRows() { - return end - start; + return Math.subtractExact(end, start); } @Override diff --git a/processing/src/main/java/org/apache/druid/segment/StringDimensionIndexer.java b/processing/src/main/java/org/apache/druid/segment/StringDimensionIndexer.java index da8dccb4287a..b95a0b745622 100644 --- a/processing/src/main/java/org/apache/druid/segment/StringDimensionIndexer.java +++ b/processing/src/main/java/org/apache/druid/segment/StringDimensionIndexer.java @@ -208,7 +208,7 @@ public long estimateEncodedKeyComponentSize(int[] keys) // string length is being accounted for each time they are referenced, based on dimension handler interface, // even though they are stored just once. It may overestimate the size by a bit, but we wanted to leave // more buffer to be safe - long estimatedSize = keys.length * Integer.BYTES; + long estimatedSize = (long) keys.length * Integer.BYTES; String[] vals = dimLookup.getValues(keys); for (String val : vals) { diff --git a/processing/src/main/java/org/apache/druid/segment/data/CompressedBlockReader.java b/processing/src/main/java/org/apache/druid/segment/data/CompressedBlockReader.java index 951a3cca447f..30b98fe4ef2a 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/CompressedBlockReader.java +++ b/processing/src/main/java/org/apache/druid/segment/data/CompressedBlockReader.java @@ -74,18 +74,20 @@ public static Supplier fromByteBuffer( "Maximum block size must be less than " + CompressedPools.BUFFER_SIZE ); final int numBlocks = buffer.getInt(); - final int offsetsSize = numBlocks * Integer.BYTES; + Preconditions.checkState(numBlocks > 0, "Number of blocks must be positive"); + final int offsetsSize = Math.multiplyExact(numBlocks, Integer.BYTES); // buffer is at start of ending offsets final ByteBuffer offsets = buffer.asReadOnlyBuffer().order(compressionOrder); - offsets.limit(offsets.position() + offsetsSize); + offsets.limit(Math.addExact(offsets.position(), offsetsSize)); final IntBuffer offsetView = offsets.slice().order(compressionOrder).asIntBuffer(); - final int compressedSize = offsetView.get(numBlocks - 1); + final int compressedSize = offsetView.get(Math.subtractExact(numBlocks, 1)); + Preconditions.checkState(compressedSize >= 0, "Compressed size must be nonnegative"); // move to start of compressed data - buffer.position(buffer.position() + offsetsSize); + buffer.position(Math.addExact(buffer.position(), offsetsSize)); final ByteBuffer compressedData = buffer.asReadOnlyBuffer().order(compressionOrder); - compressedData.limit(compressedData.position() + compressedSize); - buffer.position(buffer.position() + compressedSize); + compressedData.limit(Math.addExact(compressedData.position(), compressedSize)); + buffer.position(Math.addExact(buffer.position(), compressedSize)); final ByteBuffer compressedDataView = compressedData.slice().order(compressionOrder); return () -> new CompressedBlockReader( @@ -149,7 +151,7 @@ public CompressedBlockReader( */ public long getSize() { - return endOffsetsBuffer.get(numBlocks - 1); + return endOffsetsBuffer.get(Math.subtractExact(numBlocks, 1)); } /** diff --git a/processing/src/main/java/org/apache/druid/segment/data/CompressedColumnarIntsSupplier.java b/processing/src/main/java/org/apache/druid/segment/data/CompressedColumnarIntsSupplier.java index 55da79dbff8c..2136b98420f0 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/CompressedColumnarIntsSupplier.java +++ b/processing/src/main/java/org/apache/druid/segment/data/CompressedColumnarIntsSupplier.java @@ -61,6 +61,8 @@ private CompressedColumnarIntsSupplier( CompressionStrategy compression ) { + Preconditions.checkArgument(totalSize >= 0, "totalSize must be nonnegative"); + Preconditions.checkArgument(sizePer > 0, "sizePer must be positive"); this.totalSize = totalSize; this.sizePer = sizePer; this.baseIntBuffers = baseIntBuffers; @@ -71,7 +73,7 @@ private CompressedColumnarIntsSupplier( public ColumnarInts get() { final int div = Integer.numberOfTrailingZeros(sizePer); - final int rem = sizePer - 1; + final int rem = Math.subtractExact(sizePer, 1); final boolean isPowerOf2 = sizePer == (1 << div); if (isPowerOf2) { return new CompressedColumnarInts() diff --git a/processing/src/main/java/org/apache/druid/segment/data/CompressionStrategy.java b/processing/src/main/java/org/apache/druid/segment/data/CompressionStrategy.java index 48946a913c3e..f0066606d059 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/CompressionStrategy.java +++ b/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); } } diff --git a/processing/src/main/java/org/apache/druid/segment/data/FixedIndexed.java b/processing/src/main/java/org/apache/druid/segment/data/FixedIndexed.java index 10691d9e9b00..186c575beabd 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/FixedIndexed.java +++ b/processing/src/main/java/org/apache/druid/segment/data/FixedIndexed.java @@ -55,6 +55,7 @@ public class FixedIndexed implements Indexed public static Supplier> read(ByteBuffer bb, TypeStrategy strategy, ByteOrder byteOrder, int width) { + Preconditions.checkArgument(width > 0, "FixedIndexed requires a fixed width value type"); final ByteBuffer buffer = bb.asReadOnlyBuffer().order(byteOrder); final byte version = buffer.get(); Preconditions.checkState(version == 0, "Unknown version [%s]", version); @@ -62,7 +63,9 @@ public static Supplier> read(ByteBuffer bb, TypeStrategy final boolean hasNull = (flags & TypeStrategies.IS_NULL_BYTE) == TypeStrategies.IS_NULL_BYTE ? true : false; 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); + final int storedSize = buffer.getInt(); + Preconditions.checkState(storedSize >= 0, "FixedIndexed size must be nonnegative"); + final int size = Math.addExact(storedSize, hasNull ? 1 : 0); final int valuesOffset = buffer.position(); final Supplier> fixedIndexed = () -> new FixedIndexed<>( bb, @@ -75,7 +78,7 @@ public static Supplier> read(ByteBuffer bb, TypeStrategy valuesOffset ); - bb.position(buffer.position() + (width * (hasNull ? size - 1 : size))); + bb.position(Math.addExact(buffer.position(), Math.multiplyExact(width, storedSize))); return fixedIndexed; } @@ -125,9 +128,12 @@ public T get(int index) if (index == 0) { return null; } - return typeStrategy.read(buffer, valuesOffset + ((index - 1) * width)); + return typeStrategy.read( + buffer, + Math.addExact(valuesOffset, Math.multiplyExact(Math.subtractExact(index, 1), width)) + ); } else { - return typeStrategy.read(buffer, valuesOffset + (index * width)); + return typeStrategy.read(buffer, Math.addExact(valuesOffset, Math.multiplyExact(index, width))); } } diff --git a/processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java b/processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java index 1225800c4b84..228602bcc127 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java +++ b/processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java @@ -115,7 +115,7 @@ public Class 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; } diff --git a/processing/src/main/java/org/apache/druid/segment/data/ImmutableRTreeObjectStrategy.java b/processing/src/main/java/org/apache/druid/segment/data/ImmutableRTreeObjectStrategy.java index 8015f79202af..8b51eea22a00 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/ImmutableRTreeObjectStrategy.java +++ b/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); } diff --git a/processing/src/main/java/org/apache/druid/segment/data/SafeWritableMemory.java b/processing/src/main/java/org/apache/druid/segment/data/SafeWritableMemory.java index 9006ac5cec9e..4ef8a42ab8b8 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/SafeWritableMemory.java +++ b/processing/src/main/java/org/apache/druid/segment/data/SafeWritableMemory.java @@ -201,8 +201,9 @@ public int compareTo( @Override public void copyTo(long srcOffsetBytes, WritableMemory destination, long dstOffsetBytes, long lengthBytes) { - int offset = Ints.checkedCast(srcOffsetBytes); - for (int i = 0; i < lengthBytes; i++) { + final int offset = Ints.checkedCast(srcOffsetBytes); + final int length = Ints.checkedCast(lengthBytes); + for (int i = 0; i < length; i++) { destination.putByte(dstOffsetBytes + i, buffer.get(offset + i)); } } diff --git a/processing/src/main/java/org/apache/druid/segment/data/VSizeLongSerde.java b/processing/src/main/java/org/apache/druid/segment/data/VSizeLongSerde.java index d400a13af2f0..93c76f280c44 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/VSizeLongSerde.java +++ b/processing/src/main/java/org/apache/druid/segment/data/VSizeLongSerde.java @@ -46,7 +46,7 @@ public static int getBitsForMax(long value) if (value < 0) { throw new IAE("maxValue[%s] must be positive", value); } - byte numBits = 0; + int numBits = 0; long maxValue = 1; for (int supportedSize : SUPPORTED_SIZES) { while (numBits < supportedSize && maxValue < Long.MAX_VALUE / 2) { diff --git a/processing/src/main/java/org/apache/druid/segment/nested/NestedPathArrayElement.java b/processing/src/main/java/org/apache/druid/segment/nested/NestedPathArrayElement.java index 7f1bd38b466f..fca3fcafb3fd 100644 --- a/processing/src/main/java/org/apache/druid/segment/nested/NestedPathArrayElement.java +++ b/processing/src/main/java/org/apache/druid/segment/nested/NestedPathArrayElement.java @@ -43,24 +43,24 @@ public Object find(@Nullable Object input) { // handle lists or arrays because who knows what might end up here, depending on how is created if (input instanceof List) { - List currentList = (List) input; + final List currentList = (List) input; final int currentSize = currentList.size(); if (index < 0) { - final int adjusted = currentSize + index; - if (adjusted >= 0) { - return currentList.get(adjusted); + final long adjusted = (long) currentSize + index; + if (adjusted >= 0 && adjusted < currentSize) { + return currentList.get((int) adjusted); } - } else if (currentList.size() > index) { + } else if (index < currentSize) { return currentList.get(index); } } else if (input instanceof Object[]) { - Object[] currentList = (Object[]) input; + final Object[] currentList = (Object[]) input; if (index < 0) { - final int adjusted = currentList.length + index; - if (adjusted >= 0) { - return currentList[adjusted]; + final long adjusted = (long) currentList.length + index; + if (adjusted >= 0 && adjusted < currentList.length) { + return currentList[(int) adjusted]; } - } else if (currentList.length > index) { + } else if (index < currentList.length) { return currentList[index]; } } diff --git a/processing/src/main/java/org/apache/druid/segment/nested/VariantColumn.java b/processing/src/main/java/org/apache/druid/segment/nested/VariantColumn.java index 05470edae961..5cdde69b3a09 100644 --- a/processing/src/main/java/org/apache/druid/segment/nested/VariantColumn.java +++ b/processing/src/main/java/org/apache/druid/segment/nested/VariantColumn.java @@ -140,8 +140,8 @@ public VariantColumn( this.logicalType = logicalType; } this.adjustLongId = stringDictionary.size(); - this.adjustDoubleId = adjustLongId + longDictionary.size(); - this.adjustArrayId = adjustDoubleId + doubleDictionary.size(); + this.adjustDoubleId = Math.addExact(adjustLongId, longDictionary.size()); + this.adjustArrayId = Math.addExact(adjustDoubleId, doubleDictionary.size()); } @Override diff --git a/processing/src/main/java/org/apache/druid/timeline/SegmentId.java b/processing/src/main/java/org/apache/druid/timeline/SegmentId.java index 72ca414d4df9..3a13050c45a9 100644 --- a/processing/src/main/java/org/apache/druid/timeline/SegmentId.java +++ b/processing/src/main/java/org/apache/druid/timeline/SegmentId.java @@ -286,6 +286,8 @@ private int computeHashCode() // added in the end of the chain, resulting hashCode of SegmentId could have worse distribution. int hashCode = partitionNum; // 1000003 is a constant used in Google AutoValue, provides a little better distribution than 31 + // Hash-code overflow is intentional and required for standard Java hash behavior. + // codeql[java/tainted-arithmetic] hashCode = hashCode * 1000003 + version.hashCode(); hashCode = hashCode * 1000003 + dataSource.hashCode(); diff --git a/processing/src/test/java/org/apache/druid/collections/bitmap/BitmapOperationAgainstConsecutiveRunsTest.java b/processing/src/test/java/org/apache/druid/collections/bitmap/BitmapOperationAgainstConsecutiveRunsTest.java index 67e60f5d7cb0..d1e26d5adc29 100644 --- a/processing/src/test/java/org/apache/druid/collections/bitmap/BitmapOperationAgainstConsecutiveRunsTest.java +++ b/processing/src/test/java/org/apache/druid/collections/bitmap/BitmapOperationAgainstConsecutiveRunsTest.java @@ -58,7 +58,8 @@ public static void prepareRandomRanges() throws Exception } } minIntersection = MIN_INTERSECT; - for (int k = BITMAP_LENGTH / 2; k < BITMAP_LENGTH / 2 + minIntersection; ++k) { + final int minimumIntersection = Math.toIntExact(minIntersection); + for (int k = BITMAP_LENGTH / 2; k < BITMAP_LENGTH / 2 + minimumIntersection; ++k) { c.add(k); r.add(k); expectedUnion.set(k); diff --git a/processing/src/test/java/org/apache/druid/java/util/common/CompressionUtilsTest.java b/processing/src/test/java/org/apache/druid/java/util/common/CompressionUtilsTest.java index 937c51458d90..760eb316c148 100644 --- a/processing/src/test/java/org/apache/druid/java/util/common/CompressionUtilsTest.java +++ b/processing/src/test/java/org/apache/druid/java/util/common/CompressionUtilsTest.java @@ -577,7 +577,7 @@ public void testZeroRemainingInputStream() throws IOException ) ); } - Assert.assertEquals(GZ_BYTES.length * 3, testFile.length()); + Assert.assertEquals(3L * GZ_BYTES.length, testFile.length()); try (InputStream inputStream = new ZeroRemainingInputStream(new FileInputStream(testFile))) { for (int i = 0; i < 3; ++i) { final byte[] bytes = new byte[GZ_BYTES.length]; @@ -611,17 +611,17 @@ public InputStream openStream() } }; - Assert.assertEquals((long) (EXPECTED.length * 3), CompressionUtils.gunzip(inputStreamFactory, testFile).size()); + Assert.assertEquals(3L * EXPECTED.length, CompressionUtils.gunzip(inputStreamFactory, testFile).size()); try (final InputStream inputStream = new FileInputStream(testFile)) { try (final ByteArrayOutputStream outputStream = new ByteArrayOutputStream(EXPECTED.length * 3)) { Assert.assertEquals( "Read terminated too soon", - EXPECTED.length * 3, + 3L * EXPECTED.length, ByteStreams.copy(inputStream, outputStream) ); final byte[] found = outputStream.toByteArray(); - Assert.assertEquals(EXPECTED.length * 3, found.length); + Assert.assertEquals(3L * EXPECTED.length, found.length); Assert.assertArrayEquals(EXPECTED, Arrays.copyOfRange(found, EXPECTED.length * 0, EXPECTED.length * 1)); Assert.assertArrayEquals(EXPECTED, Arrays.copyOfRange(found, EXPECTED.length * 1, EXPECTED.length * 2)); Assert.assertArrayEquals(EXPECTED, Arrays.copyOfRange(found, EXPECTED.length * 2, EXPECTED.length * 3)); @@ -640,7 +640,7 @@ public void testGunzipConcatenatedStream() throws IOException try (ByteArrayOutputStream bos = new ByteArrayOutputStream(EXPECTED.length * 3)) { Assert.assertEquals( - EXPECTED.length * 3, + 3L * EXPECTED.length, CompressionUtils.gunzip( new ZeroRemainingInputStream( new ByteArrayInputStream(tripleGzByteStream.toByteArray()) @@ -648,7 +648,7 @@ public void testGunzipConcatenatedStream() throws IOException ) ); final byte[] found = bos.toByteArray(); - Assert.assertEquals(EXPECTED.length * 3, found.length); + Assert.assertEquals(3L * EXPECTED.length, found.length); Assert.assertArrayEquals(EXPECTED, Arrays.copyOfRange(found, EXPECTED.length * 0, EXPECTED.length * 1)); Assert.assertArrayEquals(EXPECTED, Arrays.copyOfRange(found, EXPECTED.length * 1, EXPECTED.length * 2)); Assert.assertArrayEquals(EXPECTED, Arrays.copyOfRange(found, EXPECTED.length * 2, EXPECTED.length * 3)); diff --git a/processing/src/test/java/org/apache/druid/query/aggregation/SerializablePairLongStringBufferStoreTest.java b/processing/src/test/java/org/apache/druid/query/aggregation/SerializablePairLongStringBufferStoreTest.java index a48e719a8bd6..da421fa3adbb 100644 --- a/processing/src/test/java/org/apache/druid/query/aggregation/SerializablePairLongStringBufferStoreTest.java +++ b/processing/src/test/java/org/apache/druid/query/aggregation/SerializablePairLongStringBufferStoreTest.java @@ -213,7 +213,6 @@ public void testVariedSize() throws Exception int maxStringSize = 1024 * 1024; int minStringSize = 1024; List input = new ArrayList<>(rowCount); - int totalCount = 0; for (int i = 0; i < rowCount; i++) { long longValue = random.nextLong(); @@ -221,8 +220,6 @@ public void testVariedSize() throws Exception new SerializablePairLongString(longValue, RandomStringUtils.randomAlphabetic(minStringSize, maxStringSize)); input.add(value); - totalCount += longValue; - totalCount = Math.max(totalCount, 0); bufferStore.store(value); } diff --git a/processing/src/test/java/org/apache/druid/query/scan/ScanQueryRunnerFactoryTest.java b/processing/src/test/java/org/apache/druid/query/scan/ScanQueryRunnerFactoryTest.java index 794656198d00..9760e15d1cb5 100644 --- a/processing/src/test/java/org/apache/druid/query/scan/ScanQueryRunnerFactoryTest.java +++ b/processing/src/test/java/org/apache/druid/query/scan/ScanQueryRunnerFactoryTest.java @@ -265,7 +265,7 @@ private void validateSortedOutput(List output, List expec } // check total # of rows <= limit - Assert.assertTrue(output.size() <= query.getScanRowsLimit()); + Assert.assertTrue((long) output.size() <= query.getScanRowsLimit()); // check ordering is correct for (int i = 1; i < output.size(); i++) { @@ -279,7 +279,7 @@ private void validateSortedOutput(List output, List expec } // check the values are correct - for (int i = 0; i < query.getScanRowsLimit() && i < output.size(); i++) { + for (int i = 0; (long) i < query.getScanRowsLimit() && i < output.size(); i++) { Assert.assertEquals((long) expectedEventTimestamps.get(i), output.get(i).getFirstEventTimestamp(resultFormat)); } } diff --git a/processing/src/test/java/org/apache/druid/segment/data/SafeWritableMemoryTest.java b/processing/src/test/java/org/apache/druid/segment/data/SafeWritableMemoryTest.java index 786443f43ed6..b45c40e75d85 100644 --- a/processing/src/test/java/org/apache/druid/segment/data/SafeWritableMemoryTest.java +++ b/processing/src/test/java/org/apache/druid/segment/data/SafeWritableMemoryTest.java @@ -107,7 +107,7 @@ public void testPutAndGetArrays() memory.putDoubleArray(100L, double1, 0, 1); memory.putDoubleArray(100L + Double.BYTES, double1, 1, 3); memory.getDoubleArray(100L, double2, 0, 2); - memory.getDoubleArray(100L + (2 * Double.BYTES), double2, 2, 2); + memory.getDoubleArray(100L + (2L * Double.BYTES), double2, 2, 2); for (int i = 0; i < double1.length; i++) { Assert.assertEquals(double1[i], double2[i], 0.0); } @@ -117,7 +117,7 @@ public void testPutAndGetArrays() memory.putFloatArray(100L, float1, 0, 1); memory.putFloatArray(100L + Float.BYTES, float1, 1, 3); memory.getFloatArray(100L, float2, 0, 2); - memory.getFloatArray(100L + (2 * Float.BYTES), float2, 2, 2); + memory.getFloatArray(100L + (2L * Float.BYTES), float2, 2, 2); for (int i = 0; i < float1.length; i++) { Assert.assertEquals(float1[i], float2[i], 0.0); } @@ -127,7 +127,7 @@ public void testPutAndGetArrays() memory.putIntArray(100L, ints1, 0, 1); memory.putIntArray(100L + Integer.BYTES, ints1, 1, 3); memory.getIntArray(100L, ints2, 0, 2); - memory.getIntArray(100L + (2 * Integer.BYTES), ints2, 2, 2); + memory.getIntArray(100L + (2L * Integer.BYTES), ints2, 2, 2); Assert.assertArrayEquals(ints1, ints2); final long[] longs1 = new long[]{1L, -2L, 3L, -14L}; @@ -135,7 +135,7 @@ public void testPutAndGetArrays() memory.putLongArray(100L, longs1, 0, 1); memory.putLongArray(100L + Long.BYTES, longs1, 1, 3); memory.getLongArray(100L, longs2, 0, 2); - memory.getLongArray(100L + (2 * Long.BYTES), longs2, 2, 2); + memory.getLongArray(100L + (2L * Long.BYTES), longs2, 2, 2); Assert.assertArrayEquals(longs1, longs2); final short[] shorts1 = new short[]{1, -2, 3, -14}; @@ -143,7 +143,7 @@ public void testPutAndGetArrays() memory.putShortArray(100L, shorts1, 0, 1); memory.putShortArray(100L + Short.BYTES, shorts1, 1, 3); memory.getShortArray(100L, shorts2, 0, 2); - memory.getShortArray(100L + (2 * Short.BYTES), shorts2, 2, 2); + memory.getShortArray(100L + (2L * Short.BYTES), shorts2, 2, 2); Assert.assertArrayEquals(shorts1, shorts2); } @@ -153,15 +153,15 @@ public void testFill() final byte theByte = 0x01; final byte anotherByte = 0x02; final WritableMemory memory = getMemory(); - final int halfWay = (int) (memory.getCapacity() / 2); + final long halfWay = memory.getCapacity() / 2; memory.fill(theByte); - for (int i = 0; i < memory.getCapacity(); i++) { + for (long i = 0; i < memory.getCapacity(); i++) { Assert.assertEquals(theByte, memory.getByte(i)); } memory.fill(halfWay, memory.getCapacity() - halfWay, anotherByte); - for (int i = 0; i < memory.getCapacity(); i++) { + for (long i = 0; i < memory.getCapacity(); i++) { if (i < halfWay) { Assert.assertEquals(theByte, memory.getByte(i)); } else { @@ -170,7 +170,7 @@ public void testFill() } memory.clear(halfWay, memory.getCapacity() - halfWay); - for (int i = 0; i < memory.getCapacity(); i++) { + for (long i = 0; i < memory.getCapacity(); i++) { if (i < halfWay) { Assert.assertEquals(theByte, memory.getByte(i)); } else { @@ -184,7 +184,7 @@ public void testFill() Assert.assertEquals(anotherByte, memory.getByte(halfWay - 1)); memory.clear(); - for (int i = 0; i < memory.getCapacity(); i++) { + for (long i = 0; i < memory.getCapacity(); i++) { Assert.assertEquals(0, memory.getByte(i)); } } diff --git a/processing/src/test/java/org/apache/druid/segment/data/V3CompressedVSizeColumnarMultiIntsSerializerTest.java b/processing/src/test/java/org/apache/druid/segment/data/V3CompressedVSizeColumnarMultiIntsSerializerTest.java index b5def34eb18f..dfc5bcf441fb 100644 --- a/processing/src/test/java/org/apache/druid/segment/data/V3CompressedVSizeColumnarMultiIntsSerializerTest.java +++ b/processing/src/test/java/org/apache/druid/segment/data/V3CompressedVSizeColumnarMultiIntsSerializerTest.java @@ -466,9 +466,10 @@ private void generateV2SerializedSizeAndData( V3CompressedVSizeColumnarMultiIntsSupplier supplierFromByteBuffer = V3CompressedVSizeColumnarMultiIntsSupplier.fromByteBuffer(mapper.mapFile("test"), byteOrder, mapper); ColumnarMultiInts columnarMultiInts = supplierFromByteBuffer.get(); - Assert.assertEquals(columnarMultiInts.size(), numRows); + final int rowCount = Math.toIntExact(numRows); + Assert.assertEquals(columnarMultiInts.size(), rowCount); Random verifier = new Random(0); - for (int i = 0; i < numRows; ++i) { + for (int i = 0; i < rowCount; ++i) { IndexedInts subVals = columnarMultiInts.get(i); int[] expected = generateRow(verifier, maxValue, maxValuesPerRow); Assert.assertEquals(subVals.size(), expected.length); diff --git a/processing/src/test/java/org/apache/druid/segment/serde/HyperUniquesSerdeForTest.java b/processing/src/test/java/org/apache/druid/segment/serde/HyperUniquesSerdeForTest.java index 99f3c24a54a5..dfa9c283eb20 100644 --- a/processing/src/test/java/org/apache/druid/segment/serde/HyperUniquesSerdeForTest.java +++ b/processing/src/test/java/org/apache/druid/segment/serde/HyperUniquesSerdeForTest.java @@ -116,6 +116,9 @@ public Class getClazz() public HyperLogLogCollector fromByteBuffer(ByteBuffer buffer, int numBytes) { final ByteBuffer readOnlyBuffer = buffer.asReadOnlyBuffer(); + if (numBytes < 0 || numBytes > readOnlyBuffer.remaining()) { + throw new IllegalArgumentException("numBytes exceeds the available buffer contents"); + } readOnlyBuffer.limit(readOnlyBuffer.position() + numBytes); return HyperLogLogCollector.makeCollector(readOnlyBuffer); } diff --git a/server/src/test/java/org/apache/druid/segment/loading/StorageLocationTest.java b/server/src/test/java/org/apache/druid/segment/loading/StorageLocationTest.java index 433a5b414bde..3b05767f123b 100644 --- a/server/src/test/java/org/apache/druid/segment/loading/StorageLocationTest.java +++ b/server/src/test/java/org/apache/druid/segment/loading/StorageLocationTest.java @@ -586,7 +586,7 @@ public void testAdjustReservationWeakEntryShrinksHeldBytesWithMultipleHolds() th private void verifyLoc(long maxSize, StorageLocation loc) { Assertions.assertEquals(maxSize, loc.availableSizeBytes()); - for (int i = 0; i <= maxSize; ++i) { + for (long i = 0; i <= maxSize; ++i) { Assertions.assertTrue(loc.canHandle(makeSegmentEntry("2013/2014", i)).isSuccess(), String.valueOf(i)); } } From ae6d91a2238044fcc92bc16d7f95591d602ade7f Mon Sep 17 00:00:00 2001 From: Frank Chen Date: Fri, 31 Jul 2026 06:46:24 +0800 Subject: [PATCH 2/5] fix: validate serialized buffer lengths --- ...zablePairLongStringComplexMetricSerde.java | 4 ++ .../hyperloglog/HyperUniquesSerde.java | 4 ++ .../segment/data/CompressionStrategy.java | 9 ++++ .../druid/segment/data/GenericIndexed.java | 3 ++ .../data/ImmutableRTreeObjectStrategy.java | 4 ++ .../segment/data/CompressionStrategyTest.java | 20 +++++++ .../data/ObjectStrategyBoundsTest.java | 54 +++++++++++++++++++ 7 files changed, 98 insertions(+) create mode 100644 processing/src/test/java/org/apache/druid/segment/data/ObjectStrategyBoundsTest.java diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongStringComplexMetricSerde.java b/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongStringComplexMetricSerde.java index f5d2a4b013f8..65599326bacc 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongStringComplexMetricSerde.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongStringComplexMetricSerde.java @@ -22,6 +22,7 @@ import it.unimi.dsi.fastutil.Hash; import org.apache.druid.collections.SerializablePair; import org.apache.druid.data.input.InputRow; +import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.segment.GenericColumnSerializer; import org.apache.druid.segment.IndexSpec; @@ -154,6 +155,9 @@ public Class getClazz() @Override public SerializablePairLongString fromByteBuffer(ByteBuffer buffer, int numBytes) { + if (numBytes < 0 || numBytes > buffer.remaining()) { + throw new IAE("Invalid numBytes[%d] for buffer remaining[%d]", numBytes, buffer.remaining()); + } ByteBuffer readOnlyByteBuffer = buffer.asReadOnlyBuffer().order(buffer.order()); readOnlyByteBuffer.limit(Math.addExact(buffer.position(), numBytes)); diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/hyperloglog/HyperUniquesSerde.java b/processing/src/main/java/org/apache/druid/query/aggregation/hyperloglog/HyperUniquesSerde.java index 8904c6230e8a..acd02a5e101b 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/hyperloglog/HyperUniquesSerde.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/hyperloglog/HyperUniquesSerde.java @@ -22,6 +22,7 @@ import org.apache.druid.data.input.InputRow; import org.apache.druid.hll.HyperLogLogCollector; import org.apache.druid.hll.HyperLogLogHash; +import org.apache.druid.java.util.common.IAE; import org.apache.druid.segment.data.ObjectStrategy; import org.apache.druid.segment.serde.ComplexMetricExtractor; import org.apache.druid.segment.serde.ComplexMetricSerde; @@ -104,6 +105,9 @@ public Class getClazz() @Override public HyperLogLogCollector fromByteBuffer(ByteBuffer buffer, int numBytes) { + if (numBytes < 0 || numBytes > buffer.remaining()) { + throw new IAE("Invalid numBytes[%d] for buffer remaining[%d]", numBytes, buffer.remaining()); + } // make a copy of buffer, because the given buffer is not duplicated in HyperLogLogCollector.makeCollector() and // stored in a field. final ByteBuffer readOnlyBuffer = buffer.asReadOnlyBuffer(); diff --git a/processing/src/main/java/org/apache/druid/segment/data/CompressionStrategy.java b/processing/src/main/java/org/apache/druid/segment/data/CompressionStrategy.java index f0066606d059..7605355bb30e 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/CompressionStrategy.java +++ b/processing/src/main/java/org/apache/druid/segment/data/CompressionStrategy.java @@ -30,6 +30,7 @@ import org.apache.commons.lang3.ArrayUtils; import org.apache.druid.collections.ResourceHolder; import org.apache.druid.java.util.common.ByteBufferUtils; +import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.common.io.Closer; import org.apache.druid.java.util.common.logger.Logger; @@ -240,6 +241,14 @@ public static class UncompressedDecompressor implements Decompressor @Override public void decompress(ByteBuffer in, int numBytes, ByteBuffer out) { + if (numBytes < 0 || numBytes > in.remaining() || numBytes > out.remaining()) { + throw new IAE( + "Invalid numBytes[%d] for input remaining[%d] and output remaining[%d]", + numBytes, + in.remaining(), + out.remaining() + ); + } final ByteBuffer copyBuffer = in.duplicate(); final int newPosition = Math.addExact(copyBuffer.position(), numBytes); copyBuffer.limit(newPosition); diff --git a/processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java b/processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java index 228602bcc127..528c32bbd502 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java +++ b/processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java @@ -114,6 +114,9 @@ public Class getClazz() @Override public ByteBuffer fromByteBuffer(final ByteBuffer buffer, final int numBytes) { + if (numBytes < 0 || numBytes > buffer.remaining()) { + throw new IAE("Invalid numBytes[%d] for buffer remaining[%d]", numBytes, buffer.remaining()); + } final ByteBuffer dup = buffer.asReadOnlyBuffer(); dup.limit(Math.addExact(buffer.position(), numBytes)); return dup; diff --git a/processing/src/main/java/org/apache/druid/segment/data/ImmutableRTreeObjectStrategy.java b/processing/src/main/java/org/apache/druid/segment/data/ImmutableRTreeObjectStrategy.java index 8b51eea22a00..a7208d9280bd 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/ImmutableRTreeObjectStrategy.java +++ b/processing/src/main/java/org/apache/druid/segment/data/ImmutableRTreeObjectStrategy.java @@ -22,6 +22,7 @@ import com.google.common.collect.Ordering; import org.apache.druid.collections.bitmap.BitmapFactory; import org.apache.druid.collections.spatial.ImmutableRTree; +import org.apache.druid.java.util.common.IAE; import org.apache.druid.segment.writeout.WriteOutBytes; import java.io.IOException; @@ -63,6 +64,9 @@ public Class getClazz() @Override public ImmutableRTree fromByteBuffer(ByteBuffer buffer, int numBytes) { + if (numBytes < 0 || numBytes > buffer.remaining()) { + throw new IAE("Invalid numBytes[%d] for buffer remaining[%d]", numBytes, buffer.remaining()); + } // 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(Math.addExact(duplicateBuf.position(), numBytes)); diff --git a/processing/src/test/java/org/apache/druid/segment/data/CompressionStrategyTest.java b/processing/src/test/java/org/apache/druid/segment/data/CompressionStrategyTest.java index da6b781e339b..4f71e407aae2 100644 --- a/processing/src/test/java/org/apache/druid/segment/data/CompressionStrategyTest.java +++ b/processing/src/test/java/org/apache/druid/segment/data/CompressionStrategyTest.java @@ -91,6 +91,26 @@ public void testBasicOperations() throws IOException } } + @Test + public void testUncompressedDecompressorRejectsInvalidLengths() + { + final ByteBuffer input = ByteBuffer.allocate(4); + final ByteBuffer output = ByteBuffer.allocate(4); + input.position(1); + output.position(2); + + Assert.assertThrows( + IllegalArgumentException.class, + () -> CompressionStrategy.UNCOMPRESSED.getDecompressor().decompress(input, -1, output) + ); + Assert.assertThrows( + IllegalArgumentException.class, + () -> CompressionStrategy.UNCOMPRESSED.getDecompressor().decompress(input, 3, output) + ); + Assert.assertEquals(1, input.position()); + Assert.assertEquals(2, output.position()); + } + @Test(timeout = 60_000L) public void testConcurrency() throws Exception { diff --git a/processing/src/test/java/org/apache/druid/segment/data/ObjectStrategyBoundsTest.java b/processing/src/test/java/org/apache/druid/segment/data/ObjectStrategyBoundsTest.java new file mode 100644 index 000000000000..862dcea2f896 --- /dev/null +++ b/processing/src/test/java/org/apache/druid/segment/data/ObjectStrategyBoundsTest.java @@ -0,0 +1,54 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to You under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.segment.data; + +import org.apache.druid.collections.bitmap.ConciseBitmapFactory; +import org.apache.druid.java.util.common.IAE; +import org.apache.druid.query.aggregation.SerializablePairLongStringComplexMetricSerde; +import org.apache.druid.query.aggregation.hyperloglog.HyperUniquesSerde; +import org.junit.Assert; +import org.junit.Test; + +import java.nio.ByteBuffer; +import java.util.List; + +public class ObjectStrategyBoundsTest +{ + @Test + public void testRejectsInvalidSerializedLengths() + { + final List> strategies = List.of( + GenericIndexed.UTF8_STRATEGY, + new ImmutableRTreeObjectStrategy(new ConciseBitmapFactory()), + new HyperUniquesSerde().getObjectStrategy(), + new SerializablePairLongStringComplexMetricSerde().getObjectStrategy() + ); + + for (final ObjectStrategy strategy : strategies) { + final ByteBuffer buffer = ByteBuffer.allocate(4); + buffer.position(1); + + Assert.assertThrows(IAE.class, () -> strategy.fromByteBuffer(buffer, -1)); + Assert.assertThrows(IAE.class, () -> strategy.fromByteBuffer(buffer, 4)); + Assert.assertEquals(1, buffer.position()); + Assert.assertEquals(4, buffer.limit()); + } + } +} From db308e373b4329736697f0720a273e50d9790442 Mon Sep 17 00:00:00 2001 From: Frank Chen Date: Fri, 31 Jul 2026 06:51:37 +0800 Subject: [PATCH 3/5] fix: complete serialized buffer bounds validation --- .../SerializablePairLongDoubleComplexMetricSerde.java | 6 +++++- .../SerializablePairLongFloatComplexMetricSerde.java | 6 +++++- .../SerializablePairLongLongComplexMetricSerde.java | 6 +++++- .../SerializablePairLongStringComplexMetricSerde.java | 2 +- .../druid/segment/data/ImmutableRTreeObjectStrategy.java | 2 +- .../apache/druid/segment/data/ObjectStrategyBoundsTest.java | 6 ++++++ 6 files changed, 23 insertions(+), 5 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongDoubleComplexMetricSerde.java b/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongDoubleComplexMetricSerde.java index 61f3327ade6a..f540fb515665 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongDoubleComplexMetricSerde.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongDoubleComplexMetricSerde.java @@ -21,6 +21,7 @@ import it.unimi.dsi.fastutil.Hash; import org.apache.druid.collections.SerializablePair; +import org.apache.druid.java.util.common.IAE; import org.apache.druid.segment.GenericColumnSerializer; import org.apache.druid.segment.IndexSpec; import org.apache.druid.segment.column.ColumnBuilder; @@ -101,7 +102,10 @@ public Class getClazz() @Override public SerializablePairLongDouble fromByteBuffer(ByteBuffer buffer, int numBytes) { - ByteBuffer readOnlyByteBuffer = buffer.asReadOnlyBuffer().order(buffer.order()); + if (numBytes < 0 || numBytes > buffer.remaining()) { + throw new IAE("Invalid numBytes[%d] for buffer remaining[%d]", numBytes, buffer.remaining()); + } + final ByteBuffer readOnlyByteBuffer = buffer.asReadOnlyBuffer().order(buffer.order()); readOnlyByteBuffer.limit(Math.addExact(buffer.position(), numBytes)); diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongFloatComplexMetricSerde.java b/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongFloatComplexMetricSerde.java index 1ac4493065df..ad10d4646fc6 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongFloatComplexMetricSerde.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongFloatComplexMetricSerde.java @@ -21,6 +21,7 @@ import it.unimi.dsi.fastutil.Hash; import org.apache.druid.collections.SerializablePair; +import org.apache.druid.java.util.common.IAE; import org.apache.druid.segment.GenericColumnSerializer; import org.apache.druid.segment.IndexSpec; import org.apache.druid.segment.column.ColumnBuilder; @@ -102,7 +103,10 @@ public Class getClazz() @Override public SerializablePairLongFloat fromByteBuffer(ByteBuffer buffer, int numBytes) { - ByteBuffer readOnlyByteBuffer = buffer.asReadOnlyBuffer().order(buffer.order()); + if (numBytes < 0 || numBytes > buffer.remaining()) { + throw new IAE("Invalid numBytes[%d] for buffer remaining[%d]", numBytes, buffer.remaining()); + } + final ByteBuffer readOnlyByteBuffer = buffer.asReadOnlyBuffer().order(buffer.order()); readOnlyByteBuffer.limit(Math.addExact(buffer.position(), numBytes)); diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongLongComplexMetricSerde.java b/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongLongComplexMetricSerde.java index 8faa1c99c6d2..b971d19f4d28 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongLongComplexMetricSerde.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongLongComplexMetricSerde.java @@ -21,6 +21,7 @@ import it.unimi.dsi.fastutil.Hash; import org.apache.druid.collections.SerializablePair; +import org.apache.druid.java.util.common.IAE; import org.apache.druid.segment.GenericColumnSerializer; import org.apache.druid.segment.IndexSpec; import org.apache.druid.segment.column.ColumnBuilder; @@ -101,7 +102,10 @@ public Class getClazz() @Override public SerializablePairLongLong fromByteBuffer(ByteBuffer buffer, int numBytes) { - ByteBuffer readOnlyByteBuffer = buffer.asReadOnlyBuffer().order(buffer.order()); + if (numBytes < 0 || numBytes > buffer.remaining()) { + throw new IAE("Invalid numBytes[%d] for buffer remaining[%d]", numBytes, buffer.remaining()); + } + final ByteBuffer readOnlyByteBuffer = buffer.asReadOnlyBuffer().order(buffer.order()); readOnlyByteBuffer.limit(Math.addExact(buffer.position(), numBytes)); diff --git a/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongStringComplexMetricSerde.java b/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongStringComplexMetricSerde.java index 65599326bacc..fc9b2027d251 100644 --- a/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongStringComplexMetricSerde.java +++ b/processing/src/main/java/org/apache/druid/query/aggregation/SerializablePairLongStringComplexMetricSerde.java @@ -158,7 +158,7 @@ public SerializablePairLongString fromByteBuffer(ByteBuffer buffer, int numBytes if (numBytes < 0 || numBytes > buffer.remaining()) { throw new IAE("Invalid numBytes[%d] for buffer remaining[%d]", numBytes, buffer.remaining()); } - ByteBuffer readOnlyByteBuffer = buffer.asReadOnlyBuffer().order(buffer.order()); + final ByteBuffer readOnlyByteBuffer = buffer.asReadOnlyBuffer().order(buffer.order()); readOnlyByteBuffer.limit(Math.addExact(buffer.position(), numBytes)); diff --git a/processing/src/main/java/org/apache/druid/segment/data/ImmutableRTreeObjectStrategy.java b/processing/src/main/java/org/apache/druid/segment/data/ImmutableRTreeObjectStrategy.java index a7208d9280bd..55c002aea071 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/ImmutableRTreeObjectStrategy.java +++ b/processing/src/main/java/org/apache/druid/segment/data/ImmutableRTreeObjectStrategy.java @@ -68,7 +68,7 @@ public ImmutableRTree fromByteBuffer(ByteBuffer buffer, int numBytes) throw new IAE("Invalid numBytes[%d] for buffer remaining[%d]", numBytes, buffer.remaining()); } // 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(); + final ByteBuffer duplicateBuf = buffer.duplicate(); duplicateBuf.limit(Math.addExact(duplicateBuf.position(), numBytes)); return new ImmutableRTree(duplicateBuf, bitmapFactory); } diff --git a/processing/src/test/java/org/apache/druid/segment/data/ObjectStrategyBoundsTest.java b/processing/src/test/java/org/apache/druid/segment/data/ObjectStrategyBoundsTest.java index 862dcea2f896..3a48fe7a1ae4 100644 --- a/processing/src/test/java/org/apache/druid/segment/data/ObjectStrategyBoundsTest.java +++ b/processing/src/test/java/org/apache/druid/segment/data/ObjectStrategyBoundsTest.java @@ -21,6 +21,9 @@ import org.apache.druid.collections.bitmap.ConciseBitmapFactory; import org.apache.druid.java.util.common.IAE; +import org.apache.druid.query.aggregation.SerializablePairLongDoubleComplexMetricSerde; +import org.apache.druid.query.aggregation.SerializablePairLongFloatComplexMetricSerde; +import org.apache.druid.query.aggregation.SerializablePairLongLongComplexMetricSerde; import org.apache.druid.query.aggregation.SerializablePairLongStringComplexMetricSerde; import org.apache.druid.query.aggregation.hyperloglog.HyperUniquesSerde; import org.junit.Assert; @@ -38,6 +41,9 @@ public void testRejectsInvalidSerializedLengths() GenericIndexed.UTF8_STRATEGY, new ImmutableRTreeObjectStrategy(new ConciseBitmapFactory()), new HyperUniquesSerde().getObjectStrategy(), + new SerializablePairLongDoubleComplexMetricSerde().getObjectStrategy(), + new SerializablePairLongFloatComplexMetricSerde().getObjectStrategy(), + new SerializablePairLongLongComplexMetricSerde().getObjectStrategy(), new SerializablePairLongStringComplexMetricSerde().getObjectStrategy() ); From cec711581522d84a5a08c22ea5320fa3e2f577af Mon Sep 17 00:00:00 2001 From: Frank Chen Date: Fri, 31 Jul 2026 06:57:38 +0800 Subject: [PATCH 4/5] style: fix bounds test license header --- .../org/apache/druid/segment/data/ObjectStrategyBoundsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/processing/src/test/java/org/apache/druid/segment/data/ObjectStrategyBoundsTest.java b/processing/src/test/java/org/apache/druid/segment/data/ObjectStrategyBoundsTest.java index 3a48fe7a1ae4..43c9baae5a28 100644 --- a/processing/src/test/java/org/apache/druid/segment/data/ObjectStrategyBoundsTest.java +++ b/processing/src/test/java/org/apache/druid/segment/data/ObjectStrategyBoundsTest.java @@ -3,7 +3,7 @@ * or more contributor license agreements. See the NOTICE file * distributed with this work for additional information * regarding copyright ownership. The ASF licenses this file - * to You under the Apache License, Version 2.0 (the + * to you under the Apache License, Version 2.0 (the * "License"); you may not use this file except in compliance * with the License. You may obtain a copy of the License at * From 2fc3590fd8bf3b462541910ddac809b086302766 Mon Sep 17 00:00:00 2001 From: Frank Chen Date: Fri, 31 Jul 2026 07:18:19 +0800 Subject: [PATCH 5/5] fix: validate compressed block buffer bounds --- .../segment/data/CompressedBlockReader.java | 5 ++ .../data/CompressedBlockReaderTest.java | 56 +++++++++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 processing/src/test/java/org/apache/druid/segment/data/CompressedBlockReaderTest.java diff --git a/processing/src/main/java/org/apache/druid/segment/data/CompressedBlockReader.java b/processing/src/main/java/org/apache/druid/segment/data/CompressedBlockReader.java index 30b98fe4ef2a..7e3cd0f29d84 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/CompressedBlockReader.java +++ b/processing/src/main/java/org/apache/druid/segment/data/CompressedBlockReader.java @@ -76,12 +76,17 @@ public static Supplier fromByteBuffer( final int numBlocks = buffer.getInt(); Preconditions.checkState(numBlocks > 0, "Number of blocks must be positive"); final int offsetsSize = Math.multiplyExact(numBlocks, Integer.BYTES); + Preconditions.checkState(offsetsSize <= buffer.remaining(), "Offset table exceeds remaining buffer size"); // buffer is at start of ending offsets final ByteBuffer offsets = buffer.asReadOnlyBuffer().order(compressionOrder); offsets.limit(Math.addExact(offsets.position(), offsetsSize)); final IntBuffer offsetView = offsets.slice().order(compressionOrder).asIntBuffer(); final int compressedSize = offsetView.get(Math.subtractExact(numBlocks, 1)); Preconditions.checkState(compressedSize >= 0, "Compressed size must be nonnegative"); + Preconditions.checkState( + compressedSize <= Math.subtractExact(buffer.remaining(), offsetsSize), + "Compressed data exceeds remaining buffer size" + ); // move to start of compressed data buffer.position(Math.addExact(buffer.position(), offsetsSize)); diff --git a/processing/src/test/java/org/apache/druid/segment/data/CompressedBlockReaderTest.java b/processing/src/test/java/org/apache/druid/segment/data/CompressedBlockReaderTest.java new file mode 100644 index 000000000000..032321253754 --- /dev/null +++ b/processing/src/test/java/org/apache/druid/segment/data/CompressedBlockReaderTest.java @@ -0,0 +1,56 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.segment.data; + +import org.apache.druid.segment.CompressedPools; +import org.junit.Assert; +import org.junit.Test; + +import java.nio.ByteBuffer; +import java.nio.ByteOrder; + +public class CompressedBlockReaderTest +{ + @Test + public void testRejectsCompressedSizeBeyondBufferWithoutAdvancingPastOffsets() + { + final int headerSize = 2 * Byte.BYTES + 2 * Integer.BYTES; + final ByteBuffer buffer = ByteBuffer.allocate(headerSize + Integer.BYTES + Byte.BYTES) + .order(ByteOrder.BIG_ENDIAN); + buffer.put(CompressedBlockReader.VERSION); + buffer.put(CompressionStrategy.LZ4.getId()); + buffer.putInt(CompressedPools.BUFFER_SIZE); + buffer.putInt(1); + buffer.putInt(2); + buffer.put((byte) 0); + buffer.flip(); + + Assert.assertThrows( + IllegalStateException.class, + () -> CompressedBlockReader.fromByteBuffer( + buffer, + ByteOrder.BIG_ENDIAN, + ByteOrder.BIG_ENDIAN, + false + ) + ); + Assert.assertEquals(headerSize, buffer.position()); + } +}