Skip to content

Commit 22f9750

Browse files
authored
MINOR: Check source range in ArrowBuf.setBytes for direct ByteBuffers (#1380)
## What's Changed `ArrowBuf.setBytes(long index, ByteBuffer src, int srcIndex, int length)` bounds-checks only the destination. When `src` is a direct buffer it copies `length` bytes from `getByteBufferAddress(src) + srcIndex` with `MemoryUtil.copyMemory`, so a source range past `src.capacity()` (or a negative `srcIndex`) silently copies adjacent native memory into the ArrowBuf, even with bounds checking enabled. For example, `buf.setBytes(0, ByteBuffer.allocateDirect(8), 0, 64)` succeeds. The heap path already rejects this through `ByteBuffer.position/limit`, and the `byte[]` and `ArrowBuf` overloads (and `NettyArrowBuf.setBytes(ByteBuf)`) check the source with `isOutOfBounds`. This adds the same source check before either path, so the `set/setSafe(int, ByteBuffer, int, int)` methods on the variable-width vectors fail with `IndexOutOfBoundsException` instead of reading out of bounds. Valid ranges behave as before. Added a regression test to `TestArrowBuf`.
1 parent de73e14 commit 22f9750

2 files changed

Lines changed: 17 additions & 0 deletions

File tree

‎memory/memory-core/src/main/java/org/apache/arrow/memory/ArrowBuf.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -897,6 +897,12 @@ public void setBytes(long index, ByteBuffer src) {
897897
public void setBytes(long index, ByteBuffer src, int srcIndex, int length) {
898898
// bound check for this ArrowBuf where the data will be copied into
899899
checkIndex(index, length);
900+
// bound check for src ByteBuffer
901+
if (isOutOfBounds(srcIndex, length, src.capacity())) {
902+
throw new IndexOutOfBoundsException(
903+
String.format(
904+
"index: %d, length: %d (expected: range(0, %d))", srcIndex, length, src.capacity()));
905+
}
900906
if (src.isDirect()) {
901907
// copy length bytes of data from src ByteBuffer starting at address
902908
// srcAddress into this ArrowBuf at address dstAddress

‎memory/memory-core/src/test/java/org/apache/arrow/memory/TestArrowBuf.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,17 @@ public void testSetBytesUnsliced() {
112112
}
113113
}
114114

115+
@Test
116+
public void testSetBytesDirectOutOfBoundsSource_RaisesIndexOutOfBoundsException() {
117+
ByteBuffer data = ByteBuffer.allocateDirect(8);
118+
try (BufferAllocator allocator = new RootAllocator(128);
119+
ArrowBuf buf = allocator.buffer(64)) {
120+
assertThrows(IndexOutOfBoundsException.class, () -> buf.setBytes(0, data, 0, 64));
121+
assertThrows(IndexOutOfBoundsException.class, () -> buf.setBytes(0, data, 4, 8));
122+
assertThrows(IndexOutOfBoundsException.class, () -> buf.setBytes(0, data, -1, 4));
123+
}
124+
}
125+
115126
/** ARROW-9221: guard against big-endian byte buffers. */
116127
@Test
117128
public void testSetBytesBigEndian() {

0 commit comments

Comments
 (0)