From ca67860168e2819564c85d0bfaa54d4ced53d3f2 Mon Sep 17 00:00:00 2001 From: MiniRML <134226498+MiniRML@users.noreply.github.com> Date: Mon, 12 Oct 2026 08:26:16 -0700 Subject: [PATCH] fix(nid): validate buffer ranges and string terminators --- core/libs/CMakeLists.txt | 9 +++ core/libs/nid/include/nid/NidPatcherUtils.hpp | 5 +- core/libs/nid/tests/BufferValidation.cpp | 60 +++++++++++++++++++ 3 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 core/libs/nid/tests/BufferValidation.cpp diff --git a/core/libs/CMakeLists.txt b/core/libs/CMakeLists.txt index d094f39cf..d2e26d023 100644 --- a/core/libs/CMakeLists.txt +++ b/core/libs/CMakeLists.txt @@ -161,6 +161,15 @@ if(BUILD_TESTING) set_tests_properties(guest_posix_scalar_io PROPERTIES ENVIRONMENT_MODIFICATION "PATH=path_list_prepend:$") endif() + add_test_executable(nid_buffer_validation_tests nid/tests/BufferValidation.cpp) + target_include_directories(nid_buffer_validation_tests PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/nid/include) + target_compile_features(nid_buffer_validation_tests PRIVATE cxx_std_20) + if(MINGW) + target_compile_options(nid_buffer_validation_tests PRIVATE -fasynchronous-unwind-tables) + endif() + add_test(NAME nid_buffer_validation COMMAND nid_buffer_validation_tests) + set_tests_properties(nid_buffer_validation PROPERTIES TIMEOUT 10) + find_package(Python3 QUIET COMPONENTS Interpreter) if(Python3_Interpreter_FOUND) add_test(NAME nid_patcher_macho COMMAND ${Python3_EXECUTABLE} diff --git a/core/libs/nid/include/nid/NidPatcherUtils.hpp b/core/libs/nid/include/nid/NidPatcherUtils.hpp index f46775612..33ecd14f8 100644 --- a/core/libs/nid/include/nid/NidPatcherUtils.hpp +++ b/core/libs/nid/include/nid/NidPatcherUtils.hpp @@ -61,7 +61,7 @@ inline std::string StripNidPostfix(const std::string& name) { template T Read(const std::vector& buf, std::size_t offset) { - if (offset + sizeof(T) > buf.size()) throw std::runtime_error("read out of bounds"); + if (offset > buf.size() || sizeof(T) > buf.size() - offset) throw std::runtime_error("read out of bounds"); T v; std::memcpy(&v, buf.data() + offset, sizeof(T)); return v; @@ -69,7 +69,7 @@ T Read(const std::vector& buf, std::size_t offset) { template void Write(std::vector& buf, std::size_t offset, const T& v) { - if (offset + sizeof(T) > buf.size()) throw std::runtime_error("write out of bounds"); + if (offset > buf.size() || sizeof(T) > buf.size() - offset) throw std::runtime_error("write out of bounds"); std::memcpy(buf.data() + offset, &v, sizeof(T)); } @@ -78,6 +78,7 @@ inline std::string ReadCStr(const std::vector& buf, std::size_t of std::string s; while (offset < buf.size() && buf[offset] != 0u) s.push_back(static_cast(buf[offset++])); + if (offset == buf.size()) throw std::runtime_error("unterminated cstr"); return s; } diff --git a/core/libs/nid/tests/BufferValidation.cpp b/core/libs/nid/tests/BufferValidation.cpp new file mode 100644 index 000000000..a006371d8 --- /dev/null +++ b/core/libs/nid/tests/BufferValidation.cpp @@ -0,0 +1,60 @@ +#include +#include +#include +#include +#include +#include + +namespace { + +void Require(bool condition) { + if (!condition) throw std::runtime_error("NID buffer validation failed"); +} + +template +void RequireRejected(TAction action) { + try { action(); } + catch (const std::runtime_error&) { return; } + throw std::runtime_error("Malformed NID buffer was accepted"); +} + +} + +int main() { + try { + using namespace Nid::Internal; + const std::vector unterminated{'n', 'a', 'm', 'e'}; + RequireRejected([&] { ReadCStr(unterminated, 0); }); + RequireRejected([&] { ReadCStr(unterminated, 3); }); + const std::vector strings{'a', 0, 'b', 'c', 0}; + Require(ReadCStr(strings, 0) == "a"); + Require(ReadCStr(strings, 1).empty()); + Require(ReadCStr(strings, 2) == "bc"); + Require(ReadCStr(strings, 4).empty()); + RequireRejected([&] { ReadCStr(strings, strings.size()); }); + RequireRejected([&] { ReadCStr({}, 0); }); + + std::vector bytes(8, 0xa5); + const std::uint64_t value = 0x1020304050607080; + Write(bytes, 0, value); + Require(Read(bytes, 0) == value); + Write(bytes, 4, 0x12345678); + Require(Read(bytes, 4) == 0x12345678); + const auto before = bytes; + for (const auto offset : {std::size_t{5}, std::size_t{8}, std::size_t{9}, + std::numeric_limits::max(), std::numeric_limits::max() - 1, + std::numeric_limits::max() - 3}) { + RequireRejected([&] { Read(bytes, offset); }); + RequireRejected([&] { Write(bytes, offset, 0); }); + Require(bytes == before); + } + RequireRejected([&] { Read(bytes, 1); }); + std::vector empty; + RequireRejected([&] { Read(empty, 0); }); + RequireRejected([&] { Write(empty, 0, 0); }); + std::cout << "NID buffer validation passed\n"; + } catch (const std::exception& error) { + std::cerr << error.what() << '\n'; + return 1; + } +}