From c71c162e533563a5283c2a6762e8ff92e757ebb1 Mon Sep 17 00:00:00 2001 From: Nikhil Goyal <135103317+ngoyal88@users.noreply.github.com> Date: Sat, 25 Apr 2026 19:58:14 +0530 Subject: [PATCH 1/2] fix(writer): harden growable buffer resize overflow checks Guard `used + count` and resize doubling arithmetic in `mpack_growable_writer_flush()` to prevent `size_t` wraparound. Return `mpack_error_memory` on unrepresentable growth requests instead of entering unsafe/non-terminating resize behavior. --- src/mpack/mpack-writer.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/mpack/mpack-writer.c b/src/mpack/mpack-writer.c index 029b065..178f4e2 100644 --- a/src/mpack/mpack-writer.c +++ b/src/mpack/mpack-writer.c @@ -210,10 +210,18 @@ static void mpack_growable_writer_flush(mpack_writer_t* writer, const char* data (int)count, (int)mpack_writer_buffer_left(writer), (int)used, (int)size); // grow to fit the data - // TODO: this really needs to correctly test for overflow - size_t new_size = size * 2; - while (new_size < used + count) + if (count > SIZE_MAX - used) { + mpack_writer_flag_error(writer, mpack_error_memory); + return; + } + size_t new_size = (size > SIZE_MAX / 2) ? SIZE_MAX : size * 2; + while (new_size < used + count) { + if (new_size > SIZE_MAX / 2) { + new_size = SIZE_MAX; + break; + } new_size *= 2; + } mpack_log("flush growing buffer size from %i to %i\n", (int)size, (int)new_size); From 32c0c47d2fc859c208a73bc46ed55bd387c95d60 Mon Sep 17 00:00:00 2001 From: Nikhil Goyal <135103317+ngoyal88@users.noreply.github.com> Date: Sat, 25 Apr 2026 20:19:29 +0530 Subject: [PATCH 2/2] fix(node): add overflow-safe bounds checks for node parsing and allocation Prevent `size_t` wraparound in tree parsing/accounting paths by validating arithmetic before use: - guard `data_length + bytes` checks in reserve/growth logic - guard `node_count + total` against max node limit overflow - guard dynamic page allocation size computation for child nodes Fail safely with existing MPack error paths (`mpack_error_too_big` / `mpack_error_memory`) instead of relying on wrapped arithmetic. --- src/mpack/mpack-node.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/mpack/mpack-node.c b/src/mpack/mpack-node.c index 8af1562..e7d9c7c 100644 --- a/src/mpack/mpack-node.c +++ b/src/mpack/mpack-node.c @@ -91,8 +91,7 @@ static bool mpack_tree_reserve_fill(mpack_tree_t* tree) { // if the necessary bytes would put us over the maximum tree // size, fail right away. - // TODO: check for overflow? - if (tree->data_length + bytes > tree->max_size) { + if (tree->data_length > tree->max_size || bytes > tree->max_size - tree->data_length) { mpack_tree_flag_error(tree, mpack_error_too_big); return false; } @@ -107,12 +106,17 @@ static bool mpack_tree_reserve_fill(mpack_tree_t* tree) { } // expand the buffer if needed - if (tree->data_length + bytes > tree->buffer_capacity) { + if (tree->data_length > tree->buffer_capacity || bytes > tree->buffer_capacity - tree->data_length) { - // TODO: check for overflow? + size_t needed = tree->data_length + bytes; size_t new_capacity = (tree->buffer_capacity == 0) ? MPACK_BUFFER_SIZE : tree->buffer_capacity; - while (new_capacity < tree->data_length + bytes) + while (new_capacity < needed) { + if (new_capacity > SIZE_MAX / 2) { + new_capacity = SIZE_MAX; + break; + } new_capacity *= 2; + } if (new_capacity > tree->max_size) new_capacity = tree->max_size; @@ -282,12 +286,12 @@ static bool mpack_tree_parse_children(mpack_tree_t* tree, mpack_node_data_t* nod total *= 2; } - // Make sure we are under our total node limit (TODO can this overflow?) - tree->node_count += total; - if (tree->node_count > tree->max_nodes) { + // Make sure we are under our total node limit. + if (tree->node_count > tree->max_nodes || total > tree->max_nodes - tree->node_count) { mpack_tree_flag_error(tree, mpack_error_too_big); return false; } + tree->node_count += total; // Each node is at least one byte. Count these bytes now to make // sure there is enough data left. @@ -322,9 +326,13 @@ static bool mpack_tree_parse_children(mpack_tree_t* tree, mpack_node_data_t* nod mpack_tree_page_t* page; if (total > MPACK_NODES_PER_PAGE || parser->nodes_left > MPACK_NODES_PER_PAGE / 8) { - // TODO: this should check for overflow - page = (mpack_tree_page_t*)MPACK_MALLOC( - sizeof(mpack_tree_page_t) + sizeof(mpack_node_data_t) * (total - 1)); + size_t page_size; + if (total - 1 > (SIZE_MAX - sizeof(mpack_tree_page_t)) / sizeof(mpack_node_data_t)) { + mpack_tree_flag_error(tree, mpack_error_memory); + return false; + } + page_size = sizeof(mpack_tree_page_t) + sizeof(mpack_node_data_t) * (total - 1); + page = (mpack_tree_page_t*)MPACK_MALLOC(page_size); if (page == NULL) { mpack_tree_flag_error(tree, mpack_error_memory); return false;