Skip to content

[Fix] Validate ByteBuffer payload lengths before allocation #1008

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jt2594838 merged 1 commit into from
Oct 14, 2026
Merged

[Fix] Validate ByteBuffer payload lengths before allocation #1008

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 19 additions & 23 deletions java/tsfile/src/main/java/org/apache/tsfile/utils/ReadWriteIOUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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. */
Expand All @@ -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);
}

Expand Down Expand Up @@ -730,6 +719,7 @@ public static Map<String, String> readMap(ByteBuffer buffer) {
if (length == NO_BYTE_TO_READ) {
return null;
}
checkMapSize(buffer, length);
Map<String, String> map = new HashMap<>(length);
for (int i = 0; i < length; i++) {
// key
Expand Down Expand Up @@ -762,6 +752,7 @@ public static LinkedHashMap<String, String> readLinkedHashMap(ByteBuffer buffer)
if (length == NO_BYTE_TO_READ) {
return null;
}
checkMapSize(buffer, length);
LinkedHashMap<String, String> map = new LinkedHashMap<>(length);
for (int i = 0; i < length; i++) {
// key
Expand All @@ -773,6 +764,13 @@ public static LinkedHashMap<String, String> 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<Map<String, String>> readMaps(ByteBuffer buffer, int totalSize) {
List<Map<String, String>> results = new ArrayList<>(totalSize);
for (int i = 0; i < totalSize; i++) {
Expand Down Expand Up @@ -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. */
Expand Down
237 changes: 237 additions & 0 deletions java/tsfile/src/test/java/org/apache/tsfile/utils/ReadWriteIOUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
*/
package org.apache.tsfile.utils;

import org.apache.tsfile.common.conf.TSFileConfig;

import org.junit.Assert;
import org.junit.Test;

Expand All @@ -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;
Expand All @@ -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<String, String> map = new LinkedHashMap<>();
map.put(null, null);
map.put("", "");
map.put("key", "中文");
for (Map<String, String> value :
Arrays.asList(null, Collections.<String, String>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<String, String> 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
Expand Down
Loading