diff --git a/java/tsfile/src/main/java/org/apache/tsfile/utils/ReadWriteIOUtils.java b/java/tsfile/src/main/java/org/apache/tsfile/utils/ReadWriteIOUtils.java index 59d2da32b..c98be6345 100644 --- a/java/tsfile/src/main/java/org/apache/tsfile/utils/ReadWriteIOUtils.java +++ b/java/tsfile/src/main/java/org/apache/tsfile/utils/ReadWriteIOUtils.java @@ -30,6 +30,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.nio.BufferUnderflowException; import java.nio.ByteBuffer; import java.nio.charset.CharacterCodingException; import java.util.ArrayList; @@ -135,8 +136,13 @@ public static byte readByte(ByteBuffer buffer) { * @param buffer buffer * @param size size * @return bytes array + * @throws BufferUnderflowException if size exceeds the remaining bytes in buffer */ public static byte[] readBytes(ByteBuffer buffer, int size) { + // Check the buffer limit before allocating an array from an untrusted length. + if (size > buffer.remaining()) { + throw new BufferUnderflowException(); + } byte[] res = new byte[size]; buffer.get(res); return res; @@ -642,28 +648,12 @@ public static String readVarIntString(InputStream inputStream) throws IOExceptio /** Read string from byteBuffer. */ public static String readString(ByteBuffer buffer) { - int strLength = readInt(buffer); - if (strLength < 0) { - return null; - } else if (strLength == 0) { - return ""; - } - byte[] bytes = new byte[strLength]; - buffer.get(bytes, 0, strLength); - return new String(bytes, 0, strLength, TSFileConfig.STRING_CHARSET); + return readStringWithLength(buffer, readInt(buffer)); } /** String length's type is varInt */ public static String readVarIntString(ByteBuffer buffer) { - int strLength = ReadWriteForEncodingUtils.readVarInt(buffer); - if (strLength < 0) { - return null; - } else if (strLength == 0) { - return ""; - } - byte[] bytes = new byte[strLength]; - buffer.get(bytes, 0, strLength); - return new String(bytes, 0, strLength, TSFileConfig.STRING_CHARSET); + return readStringWithLength(buffer, ReadWriteForEncodingUtils.readVarInt(buffer)); } /** Read string from byteBuffer with user define length. */ @@ -673,8 +663,7 @@ public static String readStringWithLength(ByteBuffer buffer, int length) { } else if (length == 0) { return ""; } - byte[] bytes = new byte[length]; - buffer.get(bytes, 0, length); + byte[] bytes = readBytes(buffer, length); return new String(bytes, 0, length, TSFileConfig.STRING_CHARSET); } @@ -730,6 +719,7 @@ public static Map readMap(ByteBuffer buffer) { if (length == NO_BYTE_TO_READ) { return null; } + checkMapSize(buffer, length); Map map = new HashMap<>(length); for (int i = 0; i < length; i++) { // key @@ -762,6 +752,7 @@ public static LinkedHashMap readLinkedHashMap(ByteBuffer buffer) if (length == NO_BYTE_TO_READ) { return null; } + checkMapSize(buffer, length); LinkedHashMap map = new LinkedHashMap<>(length); for (int i = 0; i < length; i++) { // key @@ -773,6 +764,13 @@ public static LinkedHashMap readLinkedHashMap(ByteBuffer buffer) return map; } + private static void checkMapSize(ByteBuffer buffer, int size) { + // Each entry needs two string length prefixes, even for null or empty strings. + if (size < 0 || size > buffer.remaining() / (2 * INT_LEN)) { + throw new BufferUnderflowException(); + } + } + public static List> readMaps(ByteBuffer buffer, int totalSize) { List> results = new ArrayList<>(totalSize); for (int i = 0; i < totalSize; i++) { @@ -811,9 +809,7 @@ public static Binary readBinary(InputStream inputStream) throws IOException { */ public static byte[] readByteBufferWithSelfDescriptionLength(ByteBuffer buffer) { int byteLength = ReadWriteForEncodingUtils.readUnsignedVarInt(buffer); - byte[] bytes = new byte[byteLength]; - buffer.get(bytes); - return bytes; + return readBytes(buffer, byteLength); } /** read bytes from buffer with offset position to the end of buffer. */ diff --git a/java/tsfile/src/test/java/org/apache/tsfile/utils/ReadWriteIOUtilsTest.java b/java/tsfile/src/test/java/org/apache/tsfile/utils/ReadWriteIOUtilsTest.java index 3b0b20a24..4322903ce 100644 --- a/java/tsfile/src/test/java/org/apache/tsfile/utils/ReadWriteIOUtilsTest.java +++ b/java/tsfile/src/test/java/org/apache/tsfile/utils/ReadWriteIOUtilsTest.java @@ -18,6 +18,8 @@ */ package org.apache.tsfile.utils; +import org.apache.tsfile.common.conf.TSFileConfig; + import org.junit.Assert; import org.junit.Test; @@ -26,9 +28,13 @@ import java.io.ByteArrayOutputStream; import java.io.DataOutputStream; import java.io.IOException; +import java.nio.BufferUnderflowException; import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; import static org.junit.Assert.fail; @@ -37,6 +43,237 @@ public class ReadWriteIOUtilsTest { protected static final int DEFAULT_BUFFER_SIZE = 4096; + @Test + public void testReadStringRejectsOversizedLength() { + for (int length : new int[] {2, Integer.MAX_VALUE}) { + ByteBuffer buffer = ByteBuffer.allocate(5).putInt(length).put((byte) 'a'); + buffer.flip(); + Assert.assertThrows( + BufferUnderflowException.class, () -> ReadWriteIOUtils.readString(buffer)); + } + } + + @Test + public void testReadBinaryRejectsOversizedLength() { + for (int length : new int[] {2, Integer.MAX_VALUE}) { + ByteBuffer buffer = ByteBuffer.allocate(5).putInt(length).put((byte) 1); + buffer.flip(); + Assert.assertThrows( + BufferUnderflowException.class, () -> ReadWriteIOUtils.readBinary(buffer)); + } + } + + @Test + public void testLengthPrefixedValuesRejectTruncatedLength() { + for (int size = 0; size < Integer.BYTES; size++) { + ByteBuffer buffer = ByteBuffer.allocate(size); + Assert.assertThrows( + BufferUnderflowException.class, () -> ReadWriteIOUtils.readString(buffer.duplicate())); + Assert.assertThrows( + BufferUnderflowException.class, () -> ReadWriteIOUtils.readBinary(buffer.duplicate())); + } + } + + @Test + public void testLengthPrefixedValuesRespectBufferLimit() { + for (boolean direct : new boolean[] {false, true}) { + ByteBuffer buffer = direct ? ByteBuffer.allocateDirect(32) : ByteBuffer.allocate(32); + buffer.putInt(7).putInt(5).put((byte) 'a').flip(); + buffer.position(Integer.BYTES); + for (ByteBuffer view : new ByteBuffer[] {buffer, buffer.asReadOnlyBuffer()}) { + Assert.assertThrows( + BufferUnderflowException.class, () -> ReadWriteIOUtils.readString(view.duplicate())); + Assert.assertThrows( + BufferUnderflowException.class, () -> ReadWriteIOUtils.readBinary(view.duplicate())); + Assert.assertEquals(Integer.BYTES, view.position()); + Assert.assertEquals(9, view.limit()); + } + } + } + + @Test + public void testExplicitLengthsRespectRemainingBytes() { + ByteBuffer buffer = ByteBuffer.allocate(32); + buffer.putInt(7).put((byte) 'a').flip(); + buffer.position(Integer.BYTES); + for (int length : new int[] {2, Integer.MAX_VALUE}) { + Assert.assertThrows( + BufferUnderflowException.class, () -> ReadWriteIOUtils.readBytes(buffer, length)); + Assert.assertThrows( + BufferUnderflowException.class, + () -> ReadWriteIOUtils.readStringWithLength(buffer, length)); + Assert.assertEquals(Integer.BYTES, buffer.position()); + } + } + + @Test + public void testReadVarIntStringRejectsOversizedLength() { + for (int length : new int[] {2, Integer.MAX_VALUE}) { + ByteBuffer buffer = ByteBuffer.allocate(32); + ReadWriteForEncodingUtils.writeVarInt(length, buffer); + buffer.put((byte) 'a').flip(); + Assert.assertThrows( + BufferUnderflowException.class, () -> ReadWriteIOUtils.readVarIntString(buffer)); + } + } + + @Test + public void testReadSelfDescribedBytesRejectsOversizedLength() { + for (int length : new int[] {2, Integer.MAX_VALUE}) { + ByteBuffer buffer = ByteBuffer.allocate(32); + ReadWriteForEncodingUtils.writeUnsignedVarInt(length, buffer); + buffer.put((byte) 1).flip(); + Assert.assertThrows( + BufferUnderflowException.class, + () -> ReadWriteIOUtils.readByteBufferWithSelfDescriptionLength(buffer)); + } + } + + @Test + public void testMapReadersRejectInvalidEntryCount() { + for (int count : new int[] {-2, 2, Integer.MAX_VALUE}) { + ByteBuffer buffer = ByteBuffer.allocate(12).putInt(count).putInt(-1).putInt(-1); + buffer.flip(); + Assert.assertThrows( + BufferUnderflowException.class, () -> ReadWriteIOUtils.readMap(buffer.duplicate())); + Assert.assertThrows( + BufferUnderflowException.class, + () -> ReadWriteIOUtils.readLinkedHashMap(buffer.duplicate())); + } + } + + @Test + public void testMapReadersRejectOversizedStrings() { + for (ByteBuffer buffer : + new ByteBuffer[] { + ByteBuffer.allocate(12).putInt(1).putInt(Integer.MAX_VALUE).putInt(0), + ByteBuffer.allocate(12).putInt(1).putInt(0).putInt(Integer.MAX_VALUE) + }) { + buffer.flip(); + Assert.assertThrows( + BufferUnderflowException.class, () -> ReadWriteIOUtils.readMap(buffer.duplicate())); + Assert.assertThrows( + BufferUnderflowException.class, + () -> ReadWriteIOUtils.readLinkedHashMap(buffer.duplicate())); + } + } + + @Test + public void testStringBufferReadersPreserveValuesAndPositions() { + for (boolean varInt : new boolean[] {false, true}) { + for (boolean direct : new boolean[] {false, true}) { + ByteBuffer buffer = direct ? ByteBuffer.allocateDirect(64) : ByteBuffer.allocate(64); + buffer.putInt(7); + for (String value : new String[] {null, "", "中文"}) { + if (varInt) { + ReadWriteIOUtils.writeVar(value, buffer); + } else { + ReadWriteIOUtils.write(value, buffer); + } + } + int end = buffer.position(); + buffer.putInt(77).flip(); + buffer.position(Integer.BYTES); + for (ByteBuffer view : new ByteBuffer[] {buffer.duplicate(), buffer.asReadOnlyBuffer()}) { + for (String value : new String[] {null, "", "中文"}) { + Assert.assertEquals( + value, + varInt + ? ReadWriteIOUtils.readVarIntString(view) + : ReadWriteIOUtils.readString(view)); + } + Assert.assertEquals(end, view.position()); + Assert.assertEquals(77, view.getInt()); + } + } + } + } + + @Test + public void testReadStringWithLengthPreservesNullEmptyAndUnicode() { + byte[] bytes = "中文".getBytes(TSFileConfig.STRING_CHARSET); + ByteBuffer buffer = ByteBuffer.wrap(bytes).asReadOnlyBuffer(); + Assert.assertNull(ReadWriteIOUtils.readStringWithLength(buffer, -1)); + Assert.assertNull(ReadWriteIOUtils.readStringWithLength(buffer, -2)); + Assert.assertEquals("", ReadWriteIOUtils.readStringWithLength(buffer, 0)); + Assert.assertEquals(0, buffer.position()); + Assert.assertEquals("中文", ReadWriteIOUtils.readStringWithLength(buffer, bytes.length)); + Assert.assertFalse(buffer.hasRemaining()); + } + + @Test + public void testBinaryBufferReaderPreservesValuesAndPositions() { + for (boolean direct : new boolean[] {false, true}) { + ByteBuffer buffer = direct ? ByteBuffer.allocateDirect(32) : ByteBuffer.allocate(32); + buffer.putInt(7); + ReadWriteIOUtils.write(new Binary(new byte[0]), buffer); + ReadWriteIOUtils.write(new Binary(new byte[] {1, 2, 3}), buffer); + int end = buffer.position(); + buffer.putInt(77).flip(); + buffer.position(Integer.BYTES); + for (ByteBuffer view : new ByteBuffer[] {buffer.duplicate(), buffer.asReadOnlyBuffer()}) { + Assert.assertArrayEquals(new byte[0], ReadWriteIOUtils.readBinary(view).getValues()); + Assert.assertArrayEquals( + new byte[] {1, 2, 3}, ReadWriteIOUtils.readBinary(view).getValues()); + Assert.assertEquals(end, view.position()); + Assert.assertEquals(77, view.getInt()); + } + } + } + + @Test + public void testReadSelfDescribedBytesPreservesValuesAndPositions() { + ByteBuffer buffer = ByteBuffer.allocate(16); + ReadWriteForEncodingUtils.writeUnsignedVarInt(0, buffer); + ReadWriteForEncodingUtils.writeUnsignedVarInt(3, buffer); + buffer.put(new byte[] {1, 2, 3}); + int end = buffer.position(); + buffer.putInt(77).flip(); + Assert.assertArrayEquals( + new byte[0], ReadWriteIOUtils.readByteBufferWithSelfDescriptionLength(buffer)); + Assert.assertArrayEquals( + new byte[] {1, 2, 3}, ReadWriteIOUtils.readByteBufferWithSelfDescriptionLength(buffer)); + Assert.assertEquals(end, buffer.position()); + Assert.assertEquals(77, buffer.getInt()); + } + + @Test + public void testNegativeBinaryLengthsRemainRejected() { + Assert.assertThrows( + NegativeArraySizeException.class, + () -> ReadWriteIOUtils.readBinary(ByteBuffer.allocate(4).putInt(0, -1))); + Assert.assertThrows( + NegativeArraySizeException.class, + () -> ReadWriteIOUtils.readBytes(ByteBuffer.allocate(0), -1)); + } + + @Test + public void testMapReadersPreserveNullEmptyAndOrder() throws IOException { + Map map = new LinkedHashMap<>(); + map.put(null, null); + map.put("", ""); + map.put("key", "中文"); + for (Map value : + Arrays.asList(null, Collections.emptyMap(), map)) { + ByteArrayOutputStream stream = new ByteArrayOutputStream(); + ReadWriteIOUtils.write(value, stream); + byte[] bytes = stream.toByteArray(); + ByteBuffer buffer = ByteBuffer.allocateDirect(bytes.length + 8); + buffer.putInt(7).put(bytes).putInt(77).flip(); + buffer.position(Integer.BYTES); + for (ByteBuffer view : new ByteBuffer[] {buffer.duplicate(), buffer.asReadOnlyBuffer()}) { + Assert.assertEquals(value, ReadWriteIOUtils.readMap(view.duplicate())); + Map linked = ReadWriteIOUtils.readLinkedHashMap(view); + Assert.assertEquals(value, linked); + if (value != null) { + Assert.assertEquals(new ArrayList<>(value.keySet()), new ArrayList<>(linked.keySet())); + } + Assert.assertEquals(Integer.BYTES + bytes.length, view.position()); + Assert.assertEquals(77, view.getInt()); + } + } + } + @Test public void stringSerdeTest() { // 1. not null value