Skip to content

[Java] Check var-data string length against the buffer before allocating #1150

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

Open
slachiewicz wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[Java] Check var-data string length against the buffer before allocating #1150

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
9 changes: 7 additions & 2 deletions sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/JavaGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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" +
Expand All @@ -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))
{
Expand Down
15 changes: 15 additions & 0 deletions sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/FrameCodecDecoder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
60 changes: 60 additions & 0 deletions sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/TokenCodecDecoder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
73 changes: 73 additions & 0 deletions sbe-tool/src/test/java/uk/co/real_logic/sbe/generation/java/VarDataStringLengthTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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());
}
}