diff --git a/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthViewVector.java b/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthViewVector.java index ea9de8320e..566ddee384 100644 --- a/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthViewVector.java +++ b/vector/src/main/java/org/apache/arrow/vector/BaseVariableWidthViewVector.java @@ -911,7 +911,8 @@ private void splitAndTransferViewBufferAndDataBuffer( final int readBufOffset = viewBuffer.getInt( ((long) i * ELEMENT_SIZE) + LENGTH_WIDTH + PREFIX_WIDTH + BUF_INDEX_WIDTH); - final ArrowBuf dataBuf = dataBuffers.get(readBufIndex); + final ArrowBuf dataBuf = + getValidatedDataBuffer(dataBuffers, readBufIndex, readBufOffset, stringLength); // allocate data buffer ArrowBuf currentDataBuf = target.allocateOrGetLastDataBuffer(stringLength); @@ -1472,7 +1473,9 @@ private void copyFromNotNull(int fromIndex, int thisIndex, ValueVector from, int + LENGTH_WIDTH + PREFIX_WIDTH + BUF_INDEX_WIDTH); - final ArrowBuf dataBuf = ((BaseVariableWidthViewVector) from).dataBuffers.get(bufIndex); + final ArrowBuf dataBuf = + getValidatedDataBuffer( + ((BaseVariableWidthViewVector) from).dataBuffers, bufIndex, dataOffset, viewLength); final ArrowBuf thisDataBuf = allocateOrGetLastDataBuffer(viewLength); viewBuffer.setBytes(start, from.getDataBuffer(), copyStart, LENGTH_WIDTH + PREFIX_WIDTH); @@ -1507,7 +1510,7 @@ public ArrowBufPointer getDataPointer(int index, ArrowBufPointer reuse) { } else { final int bufIndex = viewBuffer.getInt(((long) index * ELEMENT_SIZE) + LENGTH_WIDTH + PREFIX_WIDTH); - ArrowBuf dataBuf = dataBuffers.get(bufIndex); + ArrowBuf dataBuf = getValidatedDataBuffer(dataBuffers, bufIndex, 0, length); reuse.set(dataBuf, 0, length); } } @@ -1534,11 +1537,40 @@ public int hashCode(int index, ArrowBufHasher hasher) { final int dataOffset = viewBuffer.getInt( ((long) index * ELEMENT_SIZE) + LENGTH_WIDTH + PREFIX_WIDTH + BUF_INDEX_WIDTH); - ArrowBuf dataBuf = dataBuffers.get(bufIndex); + ArrowBuf dataBuf = getValidatedDataBuffer(dataBuffers, bufIndex, dataOffset, length); return ByteFunctionHelpers.hash(hasher, dataBuf, dataOffset, dataOffset + length); } } + /** + * Returns the out-of-line data buffer referenced by a view element, after checking that the + * element's buffer index, offset and length stay within that buffer. View and data buffers can + * come from an untrusted source (for example an IPC stream), so a corrupt view must not be able + * to dereference a missing buffer or read past the end of an existing one. + */ + private static ArrowBuf getValidatedDataBuffer( + List dataBuffers, int bufferIndex, int dataOffset, int dataLength) { + if (bufferIndex < 0 || bufferIndex >= dataBuffers.size()) { + throw new IllegalArgumentException( + "View element references data buffer " + + bufferIndex + + " but only " + + dataBuffers.size() + + " are present"); + } + ArrowBuf dataBuf = dataBuffers.get(bufferIndex); + if (dataOffset < 0 || dataLength < 0 || (long) dataOffset + dataLength > dataBuf.capacity()) { + throw new IllegalArgumentException( + "View element data (offset " + + dataOffset + + ", length " + + dataLength + + ") is out of bounds for data buffer of capacity " + + dataBuf.capacity()); + } + return dataBuf; + } + /** * Retrieves the data of a variable-width element at a given index in the vector. * @@ -1564,7 +1596,8 @@ protected byte[] getData(int index) { final int dataOffset = viewBuffer.getInt( ((long) index * ELEMENT_SIZE) + LENGTH_WIDTH + PREFIX_WIDTH + BUF_INDEX_WIDTH); - dataBuffers.get(bufferIndex).getBytes(dataOffset, result, 0, dataLength); + getValidatedDataBuffer(dataBuffers, bufferIndex, dataOffset, dataLength) + .getBytes(dataOffset, result, 0, dataLength); } else { // data is in the view buffer viewBuffer.getBytes((long) index * ELEMENT_SIZE + BUF_INDEX_WIDTH, result, 0, dataLength); @@ -1583,7 +1616,7 @@ protected void getData(int index, ReusableBuffer buffer) { final int dataOffset = viewBuffer.getInt( ((long) index * ELEMENT_SIZE) + LENGTH_WIDTH + PREFIX_WIDTH + BUF_INDEX_WIDTH); - ArrowBuf dataBuf = dataBuffers.get(bufferIndex); + ArrowBuf dataBuf = getValidatedDataBuffer(dataBuffers, bufferIndex, dataOffset, dataLength); buffer.set(dataBuf, dataOffset, dataLength); } else { // data is in the value buffer diff --git a/vector/src/test/java/org/apache/arrow/vector/TestVariableWidthViewVector.java b/vector/src/test/java/org/apache/arrow/vector/TestVariableWidthViewVector.java index baf5e672c8..d0db5ef807 100644 --- a/vector/src/test/java/org/apache/arrow/vector/TestVariableWidthViewVector.java +++ b/vector/src/test/java/org/apache/arrow/vector/TestVariableWidthViewVector.java @@ -157,6 +157,43 @@ public void testInlineAllocation() { } } + @Test + public void testGetRejectsOutOfBoundsViewOffset() { + // A view element longer than INLINE_SIZE keeps its data in a separate data buffer and encodes + // the buffer index and offset inline in the view buffer. Those fields are trusted verbatim when + // a vector is loaded from an IPC stream, so a corrupt offset must be rejected rather than used + // to read past the data buffer. + try (final ViewVarCharVector vector = new ViewVarCharVector("myvector", allocator)) { + vector.allocateNew(16, 1); + vector.setSafe(0, STR2); + vector.setValueCount(1); + assertArrayEquals(STR2, vector.get(0)); + + final long offsetPosition = + BaseVariableWidthViewVector.LENGTH_WIDTH + + BaseVariableWidthViewVector.PREFIX_WIDTH + + BaseVariableWidthViewVector.BUF_INDEX_WIDTH; + vector.viewBuffer.setInt(offsetPosition, Integer.MAX_VALUE); + + assertThrows(IllegalArgumentException.class, () -> vector.get(0)); + } + } + + @Test + public void testGetRejectsOutOfBoundsViewBufferIndex() { + try (final ViewVarCharVector vector = new ViewVarCharVector("myvector", allocator)) { + vector.allocateNew(16, 1); + vector.setSafe(0, STR2); + vector.setValueCount(1); + + final long bufIndexPosition = + BaseVariableWidthViewVector.LENGTH_WIDTH + BaseVariableWidthViewVector.PREFIX_WIDTH; + vector.viewBuffer.setInt(bufIndexPosition, 99); + + assertThrows(IllegalArgumentException.class, () -> vector.get(0)); + } + } + @Test public void testDataBufferBasedAllocationInSameBuffer() { try (final ViewVarCharVector viewVarCharVector = new ViewVarCharVector("myvector", allocator)) {