From c642be4711db18e59bcaf3b0401b3f9afbbed773 Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Sun, 25 Oct 2026 00:48:27 +0200 Subject: [PATCH] [Java] Check var-data string length against the buffer before allocating. The generated String accessor must reject a length that is negative or runs past the buffer before it allocates the temporary byte[] or moves the limit; checking after the allocation lets the wire size it. --- .../sbe/generation/java/JavaGenerator.java | 9 ++- .../sbe/ir/generated/FrameCodecDecoder.java | 15 ++++ .../sbe/ir/generated/TokenCodecDecoder.java | 60 +++++++++++++++ .../java/VarDataStringLengthTest.java | 73 +++++++++++++++++++ 4 files changed, 155 insertions(+), 2 deletions(-) create mode 100644 sbe-tool/src/test/java/uk/co/real_logic/sbe/generation/java/VarDataStringLengthTest.java diff --git a/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/JavaGenerator.java b/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/JavaGenerator.java index 609cae9beb..fa4472497d 100644 --- a/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/JavaGenerator.java +++ b/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/JavaGenerator.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2025 Real Logic Limited. + * Copyright 2013-2026 Real Logic Limited. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -1607,6 +1607,10 @@ private void generateDataDecodeMethods( indent + " final int headerLength = %3$d;\n" + indent + " final int limit = parentMessage.limit();\n" + indent + " final int dataLength = %4$s%5$s;\n" + + indent + " if (%8$sdataLength > buffer.capacity() - limit - headerLength)\n" + + indent + " {\n" + + indent + " throw new IndexOutOfBoundsException(\"var data length=\" + dataLength);\n" + + indent + " }\n\n" + indent + " parentMessage.limit(limit + headerLength + dataLength);\n\n" + indent + " if (0 == dataLength)\n" + indent + " {\n" + @@ -1622,7 +1626,8 @@ private void generateDataDecodeMethods( PrimitiveType.UINT32 == lengthType ? "(int)" : "", generateGet(lengthType, "limit", byteOrderStr), charset(characterEncoding), - accessOrderListenerCall); + accessOrderListenerCall, + PrimitiveType.UINT32 == lengthType ? "dataLength < 0 || " : ""); if (isAsciiEncoding(characterEncoding)) { diff --git a/sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/FrameCodecDecoder.java b/sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/FrameCodecDecoder.java index 3563540650..86ed25143a 100644 --- a/sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/FrameCodecDecoder.java +++ b/sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/FrameCodecDecoder.java @@ -578,6 +578,11 @@ public String packageName() final int headerLength = 2; final int limit = parentMessage.limit(); final int dataLength = (buffer.getShort(limit, BYTE_ORDER) & 0xFFFF); + if (dataLength > buffer.capacity() - limit - headerLength) + { + throw new IndexOutOfBoundsException("var data length=" + dataLength); + } + parentMessage.limit(limit + headerLength + dataLength); if (0 == dataLength) @@ -736,6 +741,11 @@ public String namespaceName() final int headerLength = 2; final int limit = parentMessage.limit(); final int dataLength = (buffer.getShort(limit, BYTE_ORDER) & 0xFFFF); + if (dataLength > buffer.capacity() - limit - headerLength) + { + throw new IndexOutOfBoundsException("var data length=" + dataLength); + } + parentMessage.limit(limit + headerLength + dataLength); if (0 == dataLength) @@ -894,6 +904,11 @@ public String semanticVersion() final int headerLength = 2; final int limit = parentMessage.limit(); final int dataLength = (buffer.getShort(limit, BYTE_ORDER) & 0xFFFF); + if (dataLength > buffer.capacity() - limit - headerLength) + { + throw new IndexOutOfBoundsException("var data length=" + dataLength); + } + parentMessage.limit(limit + headerLength + dataLength); if (0 == dataLength) diff --git a/sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/TokenCodecDecoder.java b/sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/TokenCodecDecoder.java index ea9144e0b8..0dc5b12034 100644 --- a/sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/TokenCodecDecoder.java +++ b/sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/TokenCodecDecoder.java @@ -1079,6 +1079,11 @@ public String name() final int headerLength = 2; final int limit = parentMessage.limit(); final int dataLength = (buffer.getShort(limit, BYTE_ORDER) & 0xFFFF); + if (dataLength > buffer.capacity() - limit - headerLength) + { + throw new IndexOutOfBoundsException("var data length=" + dataLength); + } + parentMessage.limit(limit + headerLength + dataLength); if (0 == dataLength) @@ -1237,6 +1242,11 @@ public String constValue() final int headerLength = 2; final int limit = parentMessage.limit(); final int dataLength = (buffer.getShort(limit, BYTE_ORDER) & 0xFFFF); + if (dataLength > buffer.capacity() - limit - headerLength) + { + throw new IndexOutOfBoundsException("var data length=" + dataLength); + } + parentMessage.limit(limit + headerLength + dataLength); if (0 == dataLength) @@ -1395,6 +1405,11 @@ public String minValue() final int headerLength = 2; final int limit = parentMessage.limit(); final int dataLength = (buffer.getShort(limit, BYTE_ORDER) & 0xFFFF); + if (dataLength > buffer.capacity() - limit - headerLength) + { + throw new IndexOutOfBoundsException("var data length=" + dataLength); + } + parentMessage.limit(limit + headerLength + dataLength); if (0 == dataLength) @@ -1553,6 +1568,11 @@ public String maxValue() final int headerLength = 2; final int limit = parentMessage.limit(); final int dataLength = (buffer.getShort(limit, BYTE_ORDER) & 0xFFFF); + if (dataLength > buffer.capacity() - limit - headerLength) + { + throw new IndexOutOfBoundsException("var data length=" + dataLength); + } + parentMessage.limit(limit + headerLength + dataLength); if (0 == dataLength) @@ -1711,6 +1731,11 @@ public String nullValue() final int headerLength = 2; final int limit = parentMessage.limit(); final int dataLength = (buffer.getShort(limit, BYTE_ORDER) & 0xFFFF); + if (dataLength > buffer.capacity() - limit - headerLength) + { + throw new IndexOutOfBoundsException("var data length=" + dataLength); + } + parentMessage.limit(limit + headerLength + dataLength); if (0 == dataLength) @@ -1869,6 +1894,11 @@ public String characterEncoding() final int headerLength = 2; final int limit = parentMessage.limit(); final int dataLength = (buffer.getShort(limit, BYTE_ORDER) & 0xFFFF); + if (dataLength > buffer.capacity() - limit - headerLength) + { + throw new IndexOutOfBoundsException("var data length=" + dataLength); + } + parentMessage.limit(limit + headerLength + dataLength); if (0 == dataLength) @@ -2027,6 +2057,11 @@ public String epoch() final int headerLength = 2; final int limit = parentMessage.limit(); final int dataLength = (buffer.getShort(limit, BYTE_ORDER) & 0xFFFF); + if (dataLength > buffer.capacity() - limit - headerLength) + { + throw new IndexOutOfBoundsException("var data length=" + dataLength); + } + parentMessage.limit(limit + headerLength + dataLength); if (0 == dataLength) @@ -2185,6 +2220,11 @@ public String timeUnit() final int headerLength = 2; final int limit = parentMessage.limit(); final int dataLength = (buffer.getShort(limit, BYTE_ORDER) & 0xFFFF); + if (dataLength > buffer.capacity() - limit - headerLength) + { + throw new IndexOutOfBoundsException("var data length=" + dataLength); + } + parentMessage.limit(limit + headerLength + dataLength); if (0 == dataLength) @@ -2343,6 +2383,11 @@ public String semanticType() final int headerLength = 2; final int limit = parentMessage.limit(); final int dataLength = (buffer.getShort(limit, BYTE_ORDER) & 0xFFFF); + if (dataLength > buffer.capacity() - limit - headerLength) + { + throw new IndexOutOfBoundsException("var data length=" + dataLength); + } + parentMessage.limit(limit + headerLength + dataLength); if (0 == dataLength) @@ -2501,6 +2546,11 @@ public String description() final int headerLength = 2; final int limit = parentMessage.limit(); final int dataLength = (buffer.getShort(limit, BYTE_ORDER) & 0xFFFF); + if (dataLength > buffer.capacity() - limit - headerLength) + { + throw new IndexOutOfBoundsException("var data length=" + dataLength); + } + parentMessage.limit(limit + headerLength + dataLength); if (0 == dataLength) @@ -2659,6 +2709,11 @@ public String referencedName() final int headerLength = 2; final int limit = parentMessage.limit(); final int dataLength = (buffer.getShort(limit, BYTE_ORDER) & 0xFFFF); + if (dataLength > buffer.capacity() - limit - headerLength) + { + throw new IndexOutOfBoundsException("var data length=" + dataLength); + } + parentMessage.limit(limit + headerLength + dataLength); if (0 == dataLength) @@ -2817,6 +2872,11 @@ public String packageName() final int headerLength = 2; final int limit = parentMessage.limit(); final int dataLength = (buffer.getShort(limit, BYTE_ORDER) & 0xFFFF); + if (dataLength > buffer.capacity() - limit - headerLength) + { + throw new IndexOutOfBoundsException("var data length=" + dataLength); + } + parentMessage.limit(limit + headerLength + dataLength); if (0 == dataLength) diff --git a/sbe-tool/src/test/java/uk/co/real_logic/sbe/generation/java/VarDataStringLengthTest.java b/sbe-tool/src/test/java/uk/co/real_logic/sbe/generation/java/VarDataStringLengthTest.java new file mode 100644 index 0000000000..22b2732884 --- /dev/null +++ b/sbe-tool/src/test/java/uk/co/real_logic/sbe/generation/java/VarDataStringLengthTest.java @@ -0,0 +1,73 @@ +/* + * Copyright 2026 Real Logic Limited. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package uk.co.real_logic.sbe.generation.java; + +import extension.CarDecoder; +import org.agrona.concurrent.UnsafeBuffer; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.nio.ByteOrder; +import java.nio.charset.StandardCharsets; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class VarDataStringLengthTest +{ + private static final int GROUP_HEADER_LENGTH = 4; + private static final int MANUFACTURER_OFFSET = CarDecoder.BLOCK_LENGTH + 2 * GROUP_HEADER_LENGTH; + private static final int MAX_FITTING_LENGTH = 12; + + // Car with empty groups and a manufacturer length prefix followed by MAX_FITTING_LENGTH bytes. + private static UnsafeBuffer carWithManufacturerLength(final int length) + { + final UnsafeBuffer buffer = new UnsafeBuffer(new byte[MANUFACTURER_OFFSET + 4 + MAX_FITTING_LENGTH]); + buffer.putInt(MANUFACTURER_OFFSET, length, ByteOrder.LITTLE_ENDIAN); + return buffer; + } + + private static CarDecoder decoderAtManufacturer(final UnsafeBuffer buffer) + { + final CarDecoder decoder = new CarDecoder(); + decoder.wrap(buffer, 0, CarDecoder.BLOCK_LENGTH, CarDecoder.SCHEMA_VERSION); + assertEquals(0, decoder.fuelFigures().count()); + assertEquals(0, decoder.performanceFigures().count()); + assertEquals(MANUFACTURER_OFFSET, decoder.limit()); + return decoder; + } + + @ParameterizedTest + @ValueSource(ints = { Integer.MAX_VALUE, -1, MAX_FITTING_LENGTH + 1 }) + void shouldRejectStringLengthBeyondBufferBeforeAllocating(final int length) + { + final CarDecoder decoder = decoderAtManufacturer(carWithManufacturerLength(length)); + + assertThrows(IndexOutOfBoundsException.class, decoder::manufacturer); + assertEquals(MANUFACTURER_OFFSET, decoder.limit()); + } + + @ParameterizedTest + @ValueSource(ints = { 0, 6, MAX_FITTING_LENGTH }) + void shouldDecodeStringThatFitsInBuffer(final int length) + { + final String manufacturer = "abcdefghijkl".substring(0, length); + final UnsafeBuffer buffer = carWithManufacturerLength(length); + buffer.putBytes(MANUFACTURER_OFFSET + 4, manufacturer.getBytes(StandardCharsets.UTF_8)); + + assertEquals(manufacturer, decoderAtManufacturer(buffer).manufacturer()); + } +}