From d7cb4f79ca056d3871aa3be431a184a04a76b225 Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Sun, 25 Oct 2026 00:55:34 +0200 Subject: [PATCH] [C++] Bound var data against the remaining buffer so a uint64 length cannot wrap the position. Compare dataLength with m_bufferLength - pos rather than checking pos + dataLength, which wraps for a uint64 length and lets the position move backwards. Do not go back to the addition. --- .../sbe/generation/cpp/CppGenerator.java | 25 +++++- sbe-tool/src/test/cpp/CMakeLists.txt | 4 + .../src/test/cpp/VarDataUint64LengthTest.cpp | 63 +++++++++++++++ sbe-tool/src/test/cpp/WireBuffer.h | 81 +++++++++++++++++++ .../uint64-var-data-length-schema.xml | 25 ++++++ 5 files changed, 194 insertions(+), 4 deletions(-) create mode 100644 sbe-tool/src/test/cpp/VarDataUint64LengthTest.cpp create mode 100644 sbe-tool/src/test/cpp/WireBuffer.h create mode 100644 sbe-tool/src/test/resources/uint64-var-data-length-schema.xml diff --git a/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/cpp/CppGenerator.java b/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/cpp/CppGenerator.java index 9b622e3989..aea5bb38e4 100755 --- a/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/cpp/CppGenerator.java +++ b/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/cpp/CppGenerator.java @@ -1143,7 +1143,8 @@ private void generateVarData( indent + " %5$s lengthFieldValue;\n" + indent + " std::memcpy(&lengthFieldValue, m_buffer + lengthPosition, sizeof(%5$s));\n" + indent + " std::uint64_t dataLength = %4$s(lengthFieldValue);\n" + - indent + " sbePosition(lengthPosition + lengthOfLengthField + dataLength);\n" + + indent + " std::uint64_t pos = sbeCheckPosition(lengthPosition + lengthOfLengthField);\n" + + generateDataPositionAdvance(indent) + indent + " return dataLength;\n" + indent + " }\n", propertyName, @@ -1160,8 +1161,10 @@ private void generateVarData( "%6$s" + indent + " %4$s lengthFieldValue;\n" + indent + " std::memcpy(&lengthFieldValue, m_buffer + sbePosition(), sizeof(%4$s));\n" + - indent + " const char *fieldPtr = m_buffer + sbePosition() + %3$d;\n" + - indent + " sbePosition(sbePosition() + %3$d + %5$s(lengthFieldValue));\n" + + indent + " std::uint64_t dataLength = %5$s(lengthFieldValue);\n" + + indent + " std::uint64_t pos = sbeCheckPosition(sbePosition() + %3$d);\n" + + indent + " const char *fieldPtr = m_buffer + pos;\n" + + generateDataPositionAdvance(indent) + indent + " return fieldPtr;\n" + indent + " }\n", formatPropertyName(propertyName), @@ -1184,7 +1187,7 @@ private void generateVarData( indent + " std::uint64_t dataLength = %4$s(lengthFieldValue);\n" + indent + " std::uint64_t bytesToCopy = length < dataLength ? length : dataLength;\n" + indent + " std::uint64_t pos = sbePosition();\n" + - indent + " sbePosition(pos + dataLength);\n" + + generateDataPositionAdvance(indent) + indent + " std::memcpy(dst, m_buffer + pos, static_cast(bytesToCopy));\n" + indent + " return bytesToCopy;\n" + indent + " }\n", @@ -1303,6 +1306,20 @@ private void generateVarData( } } + /* + * Advance the position past var data of dataLength bytes starting at pos, which must not lie past the end of + * the buffer. The check is a subtraction because pos + dataLength can wrap for a 64-bit length. + */ + private static String generateDataPositionAdvance(final String indent) + { + return + indent + " if (SBE_BOUNDS_CHECK_EXPECT((dataLength > (m_bufferLength - pos)), false))\n" + + indent + " {\n" + + indent + " throw std::runtime_error(\"buffer too short for data [E100]\");\n" + + indent + " }\n" + + indent + " sbePosition(pos + dataLength);\n"; + } + private void generateVarDataDescriptors( final StringBuilder sb, final Token token, diff --git a/sbe-tool/src/test/cpp/CMakeLists.txt b/sbe-tool/src/test/cpp/CMakeLists.txt index ef756b03c8..d8b18b1a79 100644 --- a/sbe-tool/src/test/cpp/CMakeLists.txt +++ b/sbe-tool/src/test/cpp/CMakeLists.txt @@ -46,6 +46,7 @@ set(ACCESS_ORDER_SCHEMA ${CODEC_SCHEMA_DIR}/field-order-check-schema.xml) set(VERSIONED_MESSAGE_V1_SCHEMA ${CODEC_SCHEMA_DIR}/versioned-message-v1.xml) set(VERSIONED_MESSAGE_V2_SCHEMA ${CODEC_SCHEMA_DIR}/versioned-message-v2.xml) set(UINT64_LENGTH_SCHEMA ${CODEC_SCHEMA_DIR}/uint64-length-schema.xml) +set(UINT64_VAR_DATA_LENGTH_SCHEMA ${CODEC_SCHEMA_DIR}/uint64-var-data-length-schema.xml) set(GENERATED_CODECS ${CXX_CODEC_TARGET_DIR} @@ -67,6 +68,7 @@ add_custom_command( ${VERSIONED_MESSAGE_V1_SCHEMA} ${VERSIONED_MESSAGE_V2_SCHEMA} ${UINT64_LENGTH_SCHEMA} + ${UINT64_VAR_DATA_LENGTH_SCHEMA} sbe-jar ${SBE_JAR} COMMAND ${Java_JAVA_EXECUTABLE} --add-opens java.base/jdk.internal.misc=ALL-UNNAMED @@ -93,6 +95,7 @@ add_custom_command( ${VERSIONED_MESSAGE_V1_SCHEMA} ${VERSIONED_MESSAGE_V2_SCHEMA} ${UINT64_LENGTH_SCHEMA} + ${UINT64_VAR_DATA_LENGTH_SCHEMA} ) add_custom_target(codecs DEPENDS ${GENERATED_CODECS}) @@ -114,6 +117,7 @@ sbe_test(FieldAccessOrderCheckTest codecs) target_compile_definitions(FieldAccessOrderCheckTest PRIVATE SBE_ENABLE_PRECEDENCE_CHECKS_IN_TESTS) sbe_test(VersionedMessageTest codecs) sbe_test(Uint64LengthTest codecs) +sbe_test(VarDataUint64LengthTest codecs) # OtfMessageEncoderTest lives in the otf/ subdirectory and provides its own main() that takes the # IR file path as argv[1]; it is handed the generated code-generation-schema IR. diff --git a/sbe-tool/src/test/cpp/VarDataUint64LengthTest.cpp b/sbe-tool/src/test/cpp/VarDataUint64LengthTest.cpp new file mode 100644 index 0000000000..6827b50f90 --- /dev/null +++ b/sbe-tool/src/test/cpp/VarDataUint64LengthTest.cpp @@ -0,0 +1,63 @@ +/* + * 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. + */ +#include +#include +#include +#include + +#include "gtest/gtest.h" +#include "uint64_var_data_length/WideLength.h" +#include "WireBuffer.h" + +using namespace uint64::var::data::length; + +class VarDataUint64LengthTest : public testing::Test +{ +public: + WideLength &decode(WireBuffer &buffer) + { + return m_decoder.wrapForDecode(buffer.data(), 0, WideLength::sbeBlockLength(), 0, buffer.size()); + } + +private: + WideLength m_decoder; +}; + +TEST_F(VarDataUint64LengthTest, shouldRejectLengthThatWrapsPosition) +{ + WireBuffer buffer; + buffer.u32(1).u64(std::numeric_limits::max() - 3).bytes("abcd"); + char dst[16]; + + EXPECT_THROW((void)decode(buffer).getPayload(dst, sizeof(dst)), std::runtime_error); + EXPECT_THROW((void)decode(buffer).payload(), std::runtime_error); + EXPECT_THROW((void)decode(buffer).skipPayload(), std::runtime_error); +} + +TEST_F(VarDataUint64LengthTest, shouldDecodeVarDataEndingAtBufferEnd) +{ + WireBuffer buffer; + buffer.u32(1).u64(4).bytes("abcd"); + char dst[16]; + + EXPECT_EQ(decode(buffer).getPayload(dst, sizeof(dst)), 4u); + EXPECT_EQ(std::string(dst, 4), "abcd"); + EXPECT_EQ(std::string(decode(buffer).payload(), 4), "abcd"); + + WideLength &decoder = decode(buffer); + EXPECT_EQ(decoder.skipPayload(), 4u); + EXPECT_EQ(decoder.sbePosition(), buffer.size()); +} diff --git a/sbe-tool/src/test/cpp/WireBuffer.h b/sbe-tool/src/test/cpp/WireBuffer.h new file mode 100644 index 0000000000..a26f88f05a --- /dev/null +++ b/sbe-tool/src/test/cpp/WireBuffer.h @@ -0,0 +1,81 @@ +/* + * 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. + */ +#ifndef _SBE_TEST_WIRE_BUFFER_H +#define _SBE_TEST_WIRE_BUFFER_H + +#include +#include +#include + +/* + * Buffers are built byte by byte in little-endian order and sized exactly to the bytes written, so a read past + * the end of the buffer is a read past the end of the allocation. + */ +class WireBuffer +{ +public: + WireBuffer &u8(const std::uint8_t value) + { + m_bytes.push_back(static_cast(value)); + return *this; + } + + WireBuffer &u16(const std::uint16_t value) + { + return putLittleEndian(value, 2); + } + + WireBuffer &u32(const std::uint32_t value) + { + return putLittleEndian(value, 4); + } + + WireBuffer &u64(const std::uint64_t value) + { + return putLittleEndian(value, 8); + } + + WireBuffer &bytes(const std::string &value) + { + m_bytes.insert(m_bytes.end(), value.begin(), value.end()); + return *this; + } + + char *data() + { + return m_bytes.data(); + } + + std::uint64_t size() const + { + return m_bytes.size(); + } + +private: + WireBuffer &putLittleEndian(std::uint64_t value, const int length) + { + for (int i = 0; i < length; i++) + { + m_bytes.push_back(static_cast(value & 0xFF)); + value >>= 8; + } + return *this; + } + + std::vector m_bytes; +}; + +#endif diff --git a/sbe-tool/src/test/resources/uint64-var-data-length-schema.xml b/sbe-tool/src/test/resources/uint64-var-data-length-schema.xml new file mode 100644 index 0000000000..dffdb62807 --- /dev/null +++ b/sbe-tool/src/test/resources/uint64-var-data-length-schema.xml @@ -0,0 +1,25 @@ + + + + + + + + + + + + + + + + + + +