Skip to content

Commit 4ea35df

Browse files
committed
GH-1217: validate view data offsets in BaseVariableWidthViewVector
1 parent c7e8e75 commit 4ea35df

2 files changed

Lines changed: 76 additions & 6 deletions

File tree

vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthViewVector.java

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -911,7 +911,8 @@ private void splitAndTransferViewBufferAndDataBuffer(
911911
final int readBufOffset =
912912
viewBuffer.getInt(
913913
((long) i * ELEMENT_SIZE) + LENGTH_WIDTH + PREFIX_WIDTH + BUF_INDEX_WIDTH);
914-
final ArrowBuf dataBuf = dataBuffers.get(readBufIndex);
914+
final ArrowBuf dataBuf =
915+
getValidatedDataBuffer(dataBuffers, readBufIndex, readBufOffset, stringLength);
915916

916917
// allocate data buffer
917918
ArrowBuf currentDataBuf = target.allocateOrGetLastDataBuffer(stringLength);
@@ -1472,7 +1473,9 @@ private void copyFromNotNull(int fromIndex, int thisIndex, ValueVector from, int
14721473
+ LENGTH_WIDTH
14731474
+ PREFIX_WIDTH
14741475
+ BUF_INDEX_WIDTH);
1475-
final ArrowBuf dataBuf = ((BaseVariableWidthViewVector) from).dataBuffers.get(bufIndex);
1476+
final ArrowBuf dataBuf =
1477+
getValidatedDataBuffer(
1478+
((BaseVariableWidthViewVector) from).dataBuffers, bufIndex, dataOffset, viewLength);
14761479
final ArrowBuf thisDataBuf = allocateOrGetLastDataBuffer(viewLength);
14771480

14781481
viewBuffer.setBytes(start, from.getDataBuffer(), copyStart, LENGTH_WIDTH + PREFIX_WIDTH);
@@ -1507,7 +1510,7 @@ public ArrowBufPointer getDataPointer(int index, ArrowBufPointer reuse) {
15071510
} else {
15081511
final int bufIndex =
15091512
viewBuffer.getInt(((long) index * ELEMENT_SIZE) + LENGTH_WIDTH + PREFIX_WIDTH);
1510-
ArrowBuf dataBuf = dataBuffers.get(bufIndex);
1513+
ArrowBuf dataBuf = getValidatedDataBuffer(dataBuffers, bufIndex, 0, length);
15111514
reuse.set(dataBuf, 0, length);
15121515
}
15131516
}
@@ -1534,11 +1537,40 @@ public int hashCode(int index, ArrowBufHasher hasher) {
15341537
final int dataOffset =
15351538
viewBuffer.getInt(
15361539
((long) index * ELEMENT_SIZE) + LENGTH_WIDTH + PREFIX_WIDTH + BUF_INDEX_WIDTH);
1537-
ArrowBuf dataBuf = dataBuffers.get(bufIndex);
1540+
ArrowBuf dataBuf = getValidatedDataBuffer(dataBuffers, bufIndex, dataOffset, length);
15381541
return ByteFunctionHelpers.hash(hasher, dataBuf, dataOffset, dataOffset + length);
15391542
}
15401543
}
15411544

1545+
/**
1546+
* Returns the out-of-line data buffer referenced by a view element, after checking that the
1547+
* element's buffer index, offset and length stay within that buffer. View and data buffers can
1548+
* come from an untrusted source (for example an IPC stream), so a corrupt view must not be able
1549+
* to dereference a missing buffer or read past the end of an existing one.
1550+
*/
1551+
private static ArrowBuf getValidatedDataBuffer(
1552+
List<ArrowBuf> dataBuffers, int bufferIndex, int dataOffset, int dataLength) {
1553+
if (bufferIndex < 0 || bufferIndex >= dataBuffers.size()) {
1554+
throw new IllegalArgumentException(
1555+
"View element references data buffer "
1556+
+ bufferIndex
1557+
+ " but only "
1558+
+ dataBuffers.size()
1559+
+ " are present");
1560+
}
1561+
ArrowBuf dataBuf = dataBuffers.get(bufferIndex);
1562+
if (dataOffset < 0 || dataLength < 0 || (long) dataOffset + dataLength > dataBuf.capacity()) {
1563+
throw new IllegalArgumentException(
1564+
"View element data (offset "
1565+
+ dataOffset
1566+
+ ", length "
1567+
+ dataLength
1568+
+ ") is out of bounds for data buffer of capacity "
1569+
+ dataBuf.capacity());
1570+
}
1571+
return dataBuf;
1572+
}
1573+
15421574
/**
15431575
* Retrieves the data of a variable-width element at a given index in the vector.
15441576
*
@@ -1564,7 +1596,8 @@ protected byte[] getData(int index) {
15641596
final int dataOffset =
15651597
viewBuffer.getInt(
15661598
((long) index * ELEMENT_SIZE) + LENGTH_WIDTH + PREFIX_WIDTH + BUF_INDEX_WIDTH);
1567-
dataBuffers.get(bufferIndex).getBytes(dataOffset, result, 0, dataLength);
1599+
getValidatedDataBuffer(dataBuffers, bufferIndex, dataOffset, dataLength)
1600+
.getBytes(dataOffset, result, 0, dataLength);
15681601
} else {
15691602
// data is in the view buffer
15701603
viewBuffer.getBytes((long) index * ELEMENT_SIZE + BUF_INDEX_WIDTH, result, 0, dataLength);
@@ -1583,7 +1616,7 @@ protected void getData(int index, ReusableBuffer<?> buffer) {
15831616
final int dataOffset =
15841617
viewBuffer.getInt(
15851618
((long) index * ELEMENT_SIZE) + LENGTH_WIDTH + PREFIX_WIDTH + BUF_INDEX_WIDTH);
1586-
ArrowBuf dataBuf = dataBuffers.get(bufferIndex);
1619+
ArrowBuf dataBuf = getValidatedDataBuffer(dataBuffers, bufferIndex, dataOffset, dataLength);
15871620
buffer.set(dataBuf, dataOffset, dataLength);
15881621
} else {
15891622
// data is in the value buffer

vector/src/test/java/org/apache/arrow/vector/TestVariableWidthViewVector.java

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,43 @@ public void testInlineAllocation() {
157157
}
158158
}
159159

160+
@Test
161+
public void testGetRejectsOutOfBoundsViewOffset() {
162+
// A view element longer than INLINE_SIZE keeps its data in a separate data buffer and encodes
163+
// the buffer index and offset inline in the view buffer. Those fields are trusted verbatim when
164+
// a vector is loaded from an IPC stream, so a corrupt offset must be rejected rather than used
165+
// to read past the data buffer.
166+
try (final ViewVarCharVector vector = new ViewVarCharVector("myvector", allocator)) {
167+
vector.allocateNew(16, 1);
168+
vector.setSafe(0, STR2);
169+
vector.setValueCount(1);
170+
assertArrayEquals(STR2, vector.get(0));
171+
172+
final long offsetPosition =
173+
BaseVariableWidthViewVector.LENGTH_WIDTH
174+
+ BaseVariableWidthViewVector.PREFIX_WIDTH
175+
+ BaseVariableWidthViewVector.BUF_INDEX_WIDTH;
176+
vector.viewBuffer.setInt(offsetPosition, Integer.MAX_VALUE);
177+
178+
assertThrows(IllegalArgumentException.class, () -> vector.get(0));
179+
}
180+
}
181+
182+
@Test
183+
public void testGetRejectsOutOfBoundsViewBufferIndex() {
184+
try (final ViewVarCharVector vector = new ViewVarCharVector("myvector", allocator)) {
185+
vector.allocateNew(16, 1);
186+
vector.setSafe(0, STR2);
187+
vector.setValueCount(1);
188+
189+
final long bufIndexPosition =
190+
BaseVariableWidthViewVector.LENGTH_WIDTH + BaseVariableWidthViewVector.PREFIX_WIDTH;
191+
vector.viewBuffer.setInt(bufIndexPosition, 99);
192+
193+
assertThrows(IllegalArgumentException.class, () -> vector.get(0));
194+
}
195+
}
196+
160197
@Test
161198
public void testDataBufferBasedAllocationInSameBuffer() {
162199
try (final ViewVarCharVector viewVarCharVector = new ViewVarCharVector("myvector", allocator)) {

0 commit comments

Comments
 (0)