From 611438c980f39dab44845fb74bc4931c075a78c3 Mon Sep 17 00:00:00 2001 From: abdul rawoof Date: Thu, 23 Jul 2026 15:01:53 +0530 Subject: [PATCH 1/2] validate decimal byte length before writing in setBigEndian --- .../apache/arrow/vector/Decimal256Vector.java | 15 ++++++++++++-- .../apache/arrow/vector/DecimalVector.java | 15 ++++++++++++-- .../arrow/vector/TestDecimal256Vector.java | 19 ++++++++++++++++++ .../arrow/vector/TestDecimalVector.java | 20 +++++++++++++++++++ 4 files changed, 65 insertions(+), 4 deletions(-) diff --git a/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java b/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java index f9d7e5cb9e..85f11c05ec 100644 --- a/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java +++ b/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java @@ -208,6 +208,14 @@ public void setBigEndian(int index, byte[] value) { BitVectorHelper.setBit(validityBuffer, index); final int length = value.length; + // Reject an oversized value before any bytes are written. The little-endian path below + // copies all `length` bytes into the fixed TYPE_WIDTH slot with unchecked native writes, + // so validating after the copy would leave adjacent memory already corrupted. + if (length > TYPE_WIDTH) { + throw new IllegalArgumentException( + "Invalid decimal value length. Valid length in [1 - 32], got " + length); + } + // do the bound check. valueBuffer.checkBytes((long) index * TYPE_WIDTH, (long) (index + 1) * TYPE_WIDTH); @@ -243,8 +251,6 @@ public void setBigEndian(int index, byte[] value) { return; } } - throw new IllegalArgumentException( - "Invalid decimal value length. Valid length in [1 - 32], got " + length); } /** @@ -308,6 +314,11 @@ public void setBigEndianSafe(int index, long start, ArrowBuf buffer, int length) handleSafe(index); BitVectorHelper.setBit(validityBuffer, index); + if (length > TYPE_WIDTH) { + throw new IllegalArgumentException( + "Invalid decimal value length. Valid length in [1 - 32], got " + length); + } + // do the bound checks. buffer.checkBytes(start, start + length); valueBuffer.checkBytes((long) index * TYPE_WIDTH, (long) (index + 1) * TYPE_WIDTH); diff --git a/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java b/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java index 9bf1812cc6..b2034d706c 100644 --- a/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java +++ b/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java @@ -207,6 +207,14 @@ public void setBigEndian(int index, byte[] value) { BitVectorHelper.setBit(validityBuffer, index); final int length = value.length; + // Reject an oversized value before any bytes are written. The little-endian path below + // copies all `length` bytes into the fixed TYPE_WIDTH slot with unchecked native writes, + // so validating after the copy would leave adjacent memory already corrupted. + if (length > TYPE_WIDTH) { + throw new IllegalArgumentException( + "Invalid decimal value length. Valid length in [1 - 16], got " + length); + } + // do the bound check. valueBuffer.checkBytes((long) index * TYPE_WIDTH, (long) (index + 1) * TYPE_WIDTH); @@ -241,8 +249,6 @@ public void setBigEndian(int index, byte[] value) { return; } } - throw new IllegalArgumentException( - "Invalid decimal value length. Valid length in [1 - 16], got " + length); } /** @@ -306,6 +312,11 @@ public void setBigEndianSafe(int index, long start, ArrowBuf buffer, int length) handleSafe(index); BitVectorHelper.setBit(validityBuffer, index); + if (length > TYPE_WIDTH) { + throw new IllegalArgumentException( + "Invalid decimal value length. Valid length in [1 - 16], got " + length); + } + // do the bound checks. buffer.checkBytes(start, start + length); valueBuffer.checkBytes((long) index * TYPE_WIDTH, (long) (index + 1) * TYPE_WIDTH); diff --git a/vector/src/test/java/org/apache/arrow/vector/TestDecimal256Vector.java b/vector/src/test/java/org/apache/arrow/vector/TestDecimal256Vector.java index b995dc5d92..d0b2802507 100644 --- a/vector/src/test/java/org/apache/arrow/vector/TestDecimal256Vector.java +++ b/vector/src/test/java/org/apache/arrow/vector/TestDecimal256Vector.java @@ -283,6 +283,25 @@ public void decimalBE2NE() { } } + @Test + public void setBigEndianOversizedDoesNotCorruptNeighbor() { + try (Decimal256Vector decimalVector = + TestUtils.newVector( + Decimal256Vector.class, "decimal", new ArrowType.Decimal(60, 0, 256), allocator)) { + decimalVector.allocateNew(2); + + // A value whose low bytes are all non-zero, stored in the slot right after the target. + final BigInteger neighbor = new BigInteger("305419896"); // 0x12345678 + decimalVector.setBigEndian(1, neighbor.toByteArray()); + + // A value longer than the 32-byte decimal must be rejected before anything is written. + assertThrows( + IllegalArgumentException.class, () -> decimalVector.setBigEndian(0, new byte[40])); + + assertEquals(neighbor, decimalVector.getObject(1).unscaledValue()); + } + } + @Test public void setUsingArrowBufOfLEInts() { try (Decimal256Vector decimalVector = diff --git a/vector/src/test/java/org/apache/arrow/vector/TestDecimalVector.java b/vector/src/test/java/org/apache/arrow/vector/TestDecimalVector.java index 85c11e8f3d..0638905799 100644 --- a/vector/src/test/java/org/apache/arrow/vector/TestDecimalVector.java +++ b/vector/src/test/java/org/apache/arrow/vector/TestDecimalVector.java @@ -18,6 +18,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -279,6 +280,25 @@ public void decimalBE2NE() { } } + @Test + public void setBigEndianOversizedDoesNotCorruptNeighbor() { + try (DecimalVector decimalVector = + TestUtils.newVector( + DecimalVector.class, "decimal", new ArrowType.Decimal(38, 0, 128), allocator)) { + decimalVector.allocateNew(2); + + // A value whose low bytes are all non-zero, stored in the slot right after the target. + final BigInteger neighbor = new BigInteger("305419896"); // 0x12345678 + decimalVector.setBigEndian(1, neighbor.toByteArray()); + + // A value longer than the 16-byte decimal must be rejected before anything is written. + assertThrows( + IllegalArgumentException.class, () -> decimalVector.setBigEndian(0, new byte[24])); + + assertEquals(neighbor, decimalVector.getObject(1).unscaledValue()); + } + } + @Test public void setUsingArrowBufOfInts() { try (DecimalVector decimalVector = From 2ce28e9e45a2a15c7c2292c0423c6b6db352393d Mon Sep 17 00:00:00 2001 From: abdul rawoof Date: Thu, 8 Oct 2026 14:52:45 +0530 Subject: [PATCH 2/2] Validate length before mutating vector in setBigEndian/setBigEndianSafe Move the length check ahead of handleSafe/setBit so a rejected oversize call leaves the vector unchanged: the slot stays null and the vector is not grown. Add setBigEndianSafe tests and assert isNull after rejection. --- .../apache/arrow/vector/Decimal256Vector.java | 16 +++++++++------ .../apache/arrow/vector/DecimalVector.java | 16 +++++++++------ .../arrow/vector/TestDecimal256Vector.java | 20 +++++++++++++++++++ .../arrow/vector/TestDecimalVector.java | 20 +++++++++++++++++++ 4 files changed, 60 insertions(+), 12 deletions(-) diff --git a/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java b/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java index 85f11c05ec..4fd484c66b 100644 --- a/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java +++ b/vector/src/main/java/org/apache/arrow/vector/Decimal256Vector.java @@ -205,17 +205,19 @@ public void set(int index, ArrowBuf buffer) { * @param value array of bytes containing decimal in big endian byte order. */ public void setBigEndian(int index, byte[] value) { - BitVectorHelper.setBit(validityBuffer, index); final int length = value.length; - // Reject an oversized value before any bytes are written. The little-endian path below + // Reject an oversized value before touching the vector. The little-endian path below // copies all `length` bytes into the fixed TYPE_WIDTH slot with unchecked native writes, - // so validating after the copy would leave adjacent memory already corrupted. + // so validating after the copy would leave adjacent memory already corrupted. Checking + // before setBit also keeps the slot null when the call is rejected. if (length > TYPE_WIDTH) { throw new IllegalArgumentException( "Invalid decimal value length. Valid length in [1 - 32], got " + length); } + BitVectorHelper.setBit(validityBuffer, index); + // do the bound check. valueBuffer.checkBytes((long) index * TYPE_WIDTH, (long) (index + 1) * TYPE_WIDTH); @@ -311,14 +313,16 @@ public void setSafe(int index, long start, ArrowBuf buffer, int length) { * @param length length of the value in the buffer */ public void setBigEndianSafe(int index, long start, ArrowBuf buffer, int length) { - handleSafe(index); - BitVectorHelper.setBit(validityBuffer, index); - + // Validate before handleSafe/setBit so a rejected call has no side effects: it neither + // grows the vector nor marks the slot valid. if (length > TYPE_WIDTH) { throw new IllegalArgumentException( "Invalid decimal value length. Valid length in [1 - 32], got " + length); } + handleSafe(index); + BitVectorHelper.setBit(validityBuffer, index); + // do the bound checks. buffer.checkBytes(start, start + length); valueBuffer.checkBytes((long) index * TYPE_WIDTH, (long) (index + 1) * TYPE_WIDTH); diff --git a/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java b/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java index b2034d706c..7c4000d99a 100644 --- a/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java +++ b/vector/src/main/java/org/apache/arrow/vector/DecimalVector.java @@ -204,17 +204,19 @@ public void set(int index, ArrowBuf buffer) { * @param value array of bytes containing decimal in big endian byte order. */ public void setBigEndian(int index, byte[] value) { - BitVectorHelper.setBit(validityBuffer, index); final int length = value.length; - // Reject an oversized value before any bytes are written. The little-endian path below + // Reject an oversized value before touching the vector. The little-endian path below // copies all `length` bytes into the fixed TYPE_WIDTH slot with unchecked native writes, - // so validating after the copy would leave adjacent memory already corrupted. + // so validating after the copy would leave adjacent memory already corrupted. Checking + // before setBit also keeps the slot null when the call is rejected. if (length > TYPE_WIDTH) { throw new IllegalArgumentException( "Invalid decimal value length. Valid length in [1 - 16], got " + length); } + BitVectorHelper.setBit(validityBuffer, index); + // do the bound check. valueBuffer.checkBytes((long) index * TYPE_WIDTH, (long) (index + 1) * TYPE_WIDTH); @@ -309,14 +311,16 @@ public void setSafe(int index, long start, ArrowBuf buffer, int length) { * @param length length of the value in the buffer */ public void setBigEndianSafe(int index, long start, ArrowBuf buffer, int length) { - handleSafe(index); - BitVectorHelper.setBit(validityBuffer, index); - + // Validate before handleSafe/setBit so a rejected call has no side effects: it neither + // grows the vector nor marks the slot valid. if (length > TYPE_WIDTH) { throw new IllegalArgumentException( "Invalid decimal value length. Valid length in [1 - 16], got " + length); } + handleSafe(index); + BitVectorHelper.setBit(validityBuffer, index); + // do the bound checks. buffer.checkBytes(start, start + length); valueBuffer.checkBytes((long) index * TYPE_WIDTH, (long) (index + 1) * TYPE_WIDTH); diff --git a/vector/src/test/java/org/apache/arrow/vector/TestDecimal256Vector.java b/vector/src/test/java/org/apache/arrow/vector/TestDecimal256Vector.java index d0b2802507..42f58f36be 100644 --- a/vector/src/test/java/org/apache/arrow/vector/TestDecimal256Vector.java +++ b/vector/src/test/java/org/apache/arrow/vector/TestDecimal256Vector.java @@ -298,10 +298,30 @@ public void setBigEndianOversizedDoesNotCorruptNeighbor() { assertThrows( IllegalArgumentException.class, () -> decimalVector.setBigEndian(0, new byte[40])); + // The rejected call must leave its own slot untouched (still null) and the neighbor intact. + assertTrue(decimalVector.isNull(0)); assertEquals(neighbor, decimalVector.getObject(1).unscaledValue()); } } + @Test + public void setBigEndianSafeOversizedHasNoSideEffects() { + try (Decimal256Vector decimalVector = + TestUtils.newVector( + Decimal256Vector.class, "decimal", new ArrowType.Decimal(60, 0, 256), allocator); + ArrowBuf buf = allocator.buffer(40)) { + decimalVector.allocateNew(1); + final int capacityBefore = decimalVector.getValueCapacity(); + + // Oversize length must be rejected before handleSafe grows the vector or setBit runs. + assertThrows( + IllegalArgumentException.class, () -> decimalVector.setBigEndianSafe(5, 0, buf, 40)); + + assertEquals(capacityBefore, decimalVector.getValueCapacity()); + assertTrue(decimalVector.isNull(0)); + } + } + @Test public void setUsingArrowBufOfLEInts() { try (Decimal256Vector decimalVector = diff --git a/vector/src/test/java/org/apache/arrow/vector/TestDecimalVector.java b/vector/src/test/java/org/apache/arrow/vector/TestDecimalVector.java index 0638905799..28b6143a24 100644 --- a/vector/src/test/java/org/apache/arrow/vector/TestDecimalVector.java +++ b/vector/src/test/java/org/apache/arrow/vector/TestDecimalVector.java @@ -295,10 +295,30 @@ public void setBigEndianOversizedDoesNotCorruptNeighbor() { assertThrows( IllegalArgumentException.class, () -> decimalVector.setBigEndian(0, new byte[24])); + // The rejected call must leave its own slot untouched (still null) and the neighbor intact. + assertTrue(decimalVector.isNull(0)); assertEquals(neighbor, decimalVector.getObject(1).unscaledValue()); } } + @Test + public void setBigEndianSafeOversizedHasNoSideEffects() { + try (DecimalVector decimalVector = + TestUtils.newVector( + DecimalVector.class, "decimal", new ArrowType.Decimal(38, 0, 128), allocator); + ArrowBuf buf = allocator.buffer(24)) { + decimalVector.allocateNew(1); + final int capacityBefore = decimalVector.getValueCapacity(); + + // Oversize length must be rejected before handleSafe grows the vector or setBit runs. + assertThrows( + IllegalArgumentException.class, () -> decimalVector.setBigEndianSafe(5, 0, buf, 24)); + + assertEquals(capacityBefore, decimalVector.getValueCapacity()); + assertTrue(decimalVector.isNull(0)); + } + } + @Test public void setUsingArrowBufOfInts() { try (DecimalVector decimalVector =