From b3dac03f70abad298823f5d6faa13f5c38353def Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Mon, 15 Jun 2026 00:06:03 -0400 Subject: [PATCH] Reject too-small stackSize in WeaselJsonParser_create A stackSize smaller than the bootstrap symbols pushed by reset() (or a negative one) left the parser with an empty stack: reset() swallowed the push() overflow via std::ignore, and the first parse() then read past the empty stack (top() dereferences *(stackPtr-1)), crashing. Detect this in create() by checking empty() after construction and returning null, and guard against negative stackSize wrapping the allocation size. Also fix an incremental-build bug: the ld -r bundle step only had an order-only dependency on the object library under Ninja, so editing lib.cpp rebuilt the object but never relinked the libraries. Add the object files to DEPENDS so the bundle (and the .so/.a) rebuild on change. --- CMakeLists.txt | 5 ++++- include/weaseljson.h | 3 ++- src/lib.cpp | 15 ++++++++++++--- src/test.cpp | 19 +++++++++++++++++++ 4 files changed, 37 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index fcbe832..64a6ccd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -119,7 +119,10 @@ add_custom_command( OUTPUT ${CMAKE_BINARY_DIR}/${PROJECT_NAME}.o COMMAND ${LD_EXE} -r $ -o ${CMAKE_BINARY_DIR}/${PROJECT_NAME}.o - DEPENDS ${PROJECT_NAME}-object + # Depend on the target (for ordering / the Make generator) and on the object + # files themselves, so the bundle is rebuilt when the objects' contents change + # (a bare target dependency is only order-only under Ninja). + DEPENDS ${PROJECT_NAME}-object $ COMMAND_EXPAND_LISTS) add_library(${PROJECT_NAME} SHARED ${CMAKE_BINARY_DIR}/${PROJECT_NAME}.o) diff --git a/include/weaseljson.h b/include/weaseljson.h index 54ab0cb..c2e0c8b 100644 --- a/include/weaseljson.h +++ b/include/weaseljson.h @@ -50,7 +50,8 @@ enum WeaselJsonFlags { /** Create a parser. Increasing stack size increases memory usage but also * increases the depth of nested json accepted. `callbacks` and `userdata` must * outlive the returned parser. Returns null if there's insufficient available - * memory */ + * memory, or if `stackSize` is negative or too small to hold a minimal + * document. */ WeaselJsonParser *WeaselJsonParser_create(int stackSize, const WeaselJsonCallbacks *callbacks, void *userdata, int flags); diff --git a/src/lib.cpp b/src/lib.cpp index 9a3adf3..cd1ea32 100644 --- a/src/lib.cpp +++ b/src/lib.cpp @@ -10,12 +10,21 @@ extern "C" { __attribute__((visibility("default"))) WeaselJsonParser * WeaselJsonParser_create(int stackSize, const WeaselJsonCallbacks *callbacks, void *userdata, int flags) { - auto *buf = malloc(sizeof(Parser3) + stackSize * sizeof(*Parser3::stackPtr)); + if (stackSize < 0) { // avoid (size_t)stackSize wrapping to a huge allocation + return nullptr; + } + auto *buf = + malloc(sizeof(Parser3) + (size_t)stackSize * sizeof(*Parser3::stackPtr)); if (buf == nullptr) { return nullptr; } - return (WeaselJsonParser *)new (buf) - Parser3{callbacks, userdata, stackSize, flags}; + auto *parser = new (buf) Parser3{callbacks, userdata, stackSize, flags}; + if (parser->empty()) { // stack too small to hold reset()'s bootstrap symbols + parser->~Parser3(); + free(buf); + return nullptr; + } + return (WeaselJsonParser *)parser; } __attribute__((visibility("default"))) void diff --git a/src/test.cpp b/src/test.cpp index f798693..69d81bb 100644 --- a/src/test.cpp +++ b/src/test.cpp @@ -202,6 +202,25 @@ TEST_CASE("parser3") { } } +TEST_CASE("create rejects too-small stack") { + auto c = noopCallbacks(); + // The parser needs room for the bootstrap symbols pushed by reset(); a stack + // that's too small (or negative) must yield null rather than an unusable + // parser that crashes on the first parse() (out-of-bounds read of the empty + // stack). + for (int stackSize : {-1, 0, 1, 2}) { + REQUIRE(WeaselJsonParser_create(stackSize, &c, nullptr, 0) == nullptr); + } + // A just-big-enough stack still works for a minimal document. + auto *parser = WeaselJsonParser_create(3, &c, nullptr, 0); + REQUIRE(parser != nullptr); + std::string copy = "1"; + REQUIRE(WeaselJsonParser_parse(parser, copy.data(), copy.size()) == + WeaselJson_AGAIN); + REQUIRE(WeaselJsonParser_parse(parser, nullptr, 0) == WeaselJson_OK); + WeaselJsonParser_destroy(parser); +} + TEST_CASE("streaming") { testStreaming(json); } void doTestUnescapingUtf8(std::string const &escaped,