From 47fe4b9cee945cdb76c7dae0d5a0c535af2af0cd Mon Sep 17 00:00:00 2001 From: Isaac Nickaein Date: Tue, 15 Jan 2019 20:22:11 +0330 Subject: [PATCH 1/4] Add unit test for parsing deeply-nested array --- test/src/unit-regression.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/src/unit-regression.cpp b/test/src/unit-regression.cpp index e739e3c3d2..b39a9adc47 100644 --- a/test/src/unit-regression.cpp +++ b/test/src/unit-regression.cpp @@ -1708,6 +1708,16 @@ TEST_CASE("regression tests") const auto data = j.get(); CHECK(expected == data); } + + SECTION("issue #1419 - Segmentation fault (stack overflow) due to unbounded recursion") + { + const int depth = 8000000; + + std::string s(depth, '['); + s += std::string(depth, ']'); + + CHECK_NOTHROW(nlohmann::json::parse(s)); + } } TEST_CASE("regression tests, exceptions dependent", "[!throws]") From f0883dda8f12ab11a6d89a507f67bffdd9d20362 Mon Sep 17 00:00:00 2001 From: Isaac Nickaein Date: Tue, 15 Jan 2019 20:28:01 +0330 Subject: [PATCH 2/4] During destruction, flatten children of objects to avoid recursion --- include/nlohmann/json.hpp | 49 ++++++++++++++++++++++++++++++++ single_include/nlohmann/json.hpp | 49 ++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+) diff --git a/include/nlohmann/json.hpp b/include/nlohmann/json.hpp index 3b3da50fff..6639ce6204 100644 --- a/include/nlohmann/json.hpp +++ b/include/nlohmann/json.hpp @@ -994,6 +994,55 @@ class basic_json void destroy(value_t t) noexcept { + // flatten the current json_value to a heap-allocated stack + std::vector stack; + + // move the top-level items to stack + if (t == value_t::array) + { + stack.reserve(array->size()); + std::move(array->begin(), array->end(), std::back_inserter(stack)); + } + else if (t == value_t::object) + { + stack.reserve(object->size()); + + for (auto&& it : *object) + { + stack.push_back(std::move(it.second)); + } + } + + while (!stack.empty()) + { + // move the last item to local variable to be processed + basic_json current_item(std::move(stack.back())); + stack.pop_back(); + + // if current_item is array/object, move + // its children to the stack to be processed later + if (current_item.is_array()) + { + stack.reserve(stack.size() + current_item.m_value.array->size()); + + std::move(current_item.m_value.array->begin(), current_item.m_value.array->end(), + std::back_inserter(stack)); + + current_item.m_value.array->clear(); + } + else if (current_item.is_object()) + { + stack.reserve(stack.size() + current_item.m_value.object->size()); + + for (auto&& it : *current_item.m_value.object) + { + stack.push_back(std::move(it.second)); + } + } + + // current_item is destroyed here + } + switch (t) { case value_t::object: diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 70b6c8a855..3e882353b9 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -13462,6 +13462,55 @@ class basic_json void destroy(value_t t) noexcept { + // flatten the current json_value to a heap-allocated stack + std::vector stack; + + // move the top-level items to stack + if (t == value_t::array) + { + stack.reserve(array->size()); + std::move(array->begin(), array->end(), std::back_inserter(stack)); + } + else if (t == value_t::object) + { + stack.reserve(object->size()); + + for (auto&& it : *object) + { + stack.push_back(std::move(it.second)); + } + } + + while (!stack.empty()) + { + // move the last item to local variable to be processed + basic_json current_item(std::move(stack.back())); + stack.pop_back(); + + // if current_item is array/object, move + // its children to the stack to be processed later + if (current_item.is_array()) + { + stack.reserve(stack.size() + current_item.m_value.array->size()); + + std::move(current_item.m_value.array->begin(), current_item.m_value.array->end(), + std::back_inserter(stack)); + + current_item.m_value.array->clear(); + } + else if (current_item.is_object()) + { + stack.reserve(stack.size() + current_item.m_value.object->size()); + + for (auto&& it : *current_item.m_value.object) + { + stack.push_back(std::move(it.second)); + } + } + + // current_item is destroyed here + } + switch (t) { case value_t::object: From 68d0a7b246ed7a9641a12025e8a8c7ac80127e54 Mon Sep 17 00:00:00 2001 From: Isaac Nickaein Date: Sat, 9 Nov 2019 21:19:12 +0330 Subject: [PATCH 3/4] Reduce depth in unit-test to avoid choking valgrind --- test/src/unit-regression.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/src/unit-regression.cpp b/test/src/unit-regression.cpp index a5da394c45..faf2da54b6 100644 --- a/test/src/unit-regression.cpp +++ b/test/src/unit-regression.cpp @@ -1780,10 +1780,10 @@ TEST_CASE("regression tests") SECTION("issue #1419 - Segmentation fault (stack overflow) due to unbounded recursion") { - const int depth = 8000000; + const auto depth = 5000000; - std::string s(depth, '['); - s += std::string(depth, ']'); + std::string s(2 * depth, '['); + std::fill(s.begin() + depth, s.end(), ']'); CHECK_NOTHROW(nlohmann::json::parse(s)); } From 7e2445a0f42e01466da0a6ef2b08abd18c75efd2 Mon Sep 17 00:00:00 2001 From: Isaac Nickaein Date: Sat, 9 Nov 2019 21:42:39 +0330 Subject: [PATCH 4/4] Move deep JSON test to a separate unit-test --- test/CMakeLists.txt | 1 + test/src/unit-large_json.cpp | 49 ++++++++++++++++++++++++++++++++++++ test/src/unit-regression.cpp | 10 -------- 3 files changed, 50 insertions(+), 10 deletions(-) create mode 100644 test/src/unit-large_json.cpp diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 762e5582d6..df2f055bf6 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -127,6 +127,7 @@ set(files src/unit-iterators2.cpp src/unit-json_patch.cpp src/unit-json_pointer.cpp + src/unit-large_json.cpp src/unit-merge_patch.cpp src/unit-meta.cpp src/unit-modifiers.cpp diff --git a/test/src/unit-large_json.cpp b/test/src/unit-large_json.cpp new file mode 100644 index 0000000000..8122856fc3 --- /dev/null +++ b/test/src/unit-large_json.cpp @@ -0,0 +1,49 @@ +/* + __ _____ _____ _____ + __| | __| | | | JSON for Modern C++ (test suite) +| | |__ | | | | | | version 3.7.1 +|_____|_____|_____|_|___| https://github.com/nlohmann/json + +Licensed under the MIT License . +SPDX-License-Identifier: MIT +Copyright (c) 2013-2019 Niels Lohmann . + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. +*/ + +#include "doctest_compatibility.h" + +#include +using nlohmann::json; + +#include + +TEST_CASE("tests on very large JSONs") +{ + SECTION("issue #1419 - Segmentation fault (stack overflow) due to unbounded recursion") + { + const auto depth = 5000000; + + std::string s(2 * depth, '['); + std::fill(s.begin() + depth, s.end(), ']'); + + CHECK_NOTHROW(nlohmann::json::parse(s)); + } +} + diff --git a/test/src/unit-regression.cpp b/test/src/unit-regression.cpp index faf2da54b6..f0be313278 100644 --- a/test/src/unit-regression.cpp +++ b/test/src/unit-regression.cpp @@ -1778,16 +1778,6 @@ TEST_CASE("regression tests") CHECK(expected == data); } - SECTION("issue #1419 - Segmentation fault (stack overflow) due to unbounded recursion") - { - const auto depth = 5000000; - - std::string s(2 * depth, '['); - std::fill(s.begin() + depth, s.end(), ']'); - - CHECK_NOTHROW(nlohmann::json::parse(s)); - } - SECTION("issue #1445 - buffer overflow in dumping invalid utf-8 strings") { SECTION("a bunch of -1, ensure_ascii=true")