From a3587a48e7819b38dd18fc1e18b644958c387064 Mon Sep 17 00:00:00 2001 From: Dmitry Matsiukhov <46869283+dimmarvel@users.noreply.github.com> Date: Thu, 17 Jul 2025 15:14:41 +0300 Subject: [PATCH] fix array entry deserialization and JSON parsing edge cases + tests (#545) credits go to Lilith (>_>) of Cisco Talos (TALOS-2018-0637) and moneromooo-monero (DoS/RPC fixes PR#4438) --- .../storages/portable_storage_from_bin.h | 8 +++ .../storages/portable_storage_from_json.h | 15 +++-- .../epee_levin_protocol_handler_async.cpp | 62 ++++++++++++++++++- 3 files changed, 78 insertions(+), 7 deletions(-) diff --git a/contrib/epee/include/storages/portable_storage_from_bin.h b/contrib/epee/include/storages/portable_storage_from_bin.h index 2f84d4be..1c21258b 100644 --- a/contrib/epee/include/storages/portable_storage_from_bin.h +++ b/contrib/epee/include/storages/portable_storage_from_bin.h @@ -59,6 +59,7 @@ namespace epee storage_entry load_storage_entry(); void read(section& sec); void read(std::string& str); + void read(array_entry &ae); private: struct recursuion_limitation_guard { @@ -114,6 +115,7 @@ namespace epee void throwable_buffer_reader::read(t_pod_type& pod_val) { RECURSION_LIMITATION(); + static_assert(std::is_pod::value, "POD type expected"); read(&pod_val, sizeof(pod_val)); } @@ -277,5 +279,11 @@ namespace epee m_ptr+=len; m_count -= len; } + inline + void throwable_buffer_reader::read(array_entry &ae) + { + RECURSION_LIMITATION(); + CHECK_AND_ASSERT_THROW_MES(false, "Reading array entry is not supported"); + } } } \ No newline at end of file diff --git a/contrib/epee/include/storages/portable_storage_from_json.h b/contrib/epee/include/storages/portable_storage_from_json.h index 3b323697..29ba98b1 100644 --- a/contrib/epee/include/storages/portable_storage_from_json.h +++ b/contrib/epee/include/storages/portable_storage_from_json.h @@ -28,6 +28,8 @@ #include "parserse_base_utils.h" #include "file_io_utils.h" +#define EPEE_JSON_RECURSION_LIMIT_INTERNAL 100 + namespace epee { namespace serialization @@ -54,9 +56,10 @@ namespace epee ASSERT_MES_AND_THROW("json parse error"); }*/ template - inline void run_handler(typename t_storage::hsection current_section, std::string::const_iterator& sec_buf_begin, std::string::const_iterator buf_end, t_storage& stg) + inline void run_handler(typename t_storage::hsection current_section, std::string::const_iterator& sec_buf_begin, std::string::const_iterator buf_end, t_storage& stg, unsigned int recursion) { - + CHECK_AND_ASSERT_THROW_MES(recursion < EPEE_JSON_RECURSION_LIMIT_INTERNAL, + "Wrong JSON data: recursion limitation (" << EPEE_JSON_RECURSION_LIMIT_INTERNAL << ") exceeded"); std::string::const_iterator sub_element_start; std::string name; typename t_storage::harray h_array = nullptr; @@ -167,7 +170,7 @@ namespace epee //sub section here typename t_storage::hsection new_sec = stg.open_section(name, current_section, true); CHECK_AND_ASSERT_THROW_MES(new_sec, "Failed to insert new section in json: " << std::string(it, buf_end)); - run_handler(new_sec, it, buf_end, stg); + run_handler(new_sec, it, buf_end, stg, recursion + 1); state = match_state_wonder_after_value; }else if(*it == '[') {//array of something @@ -196,7 +199,7 @@ namespace epee typename t_storage::hsection new_sec = nullptr; h_array = stg.insert_first_section(name, new_sec, current_section); CHECK_AND_ASSERT_THROW_MES(h_array&&new_sec, "failed to create new section"); - run_handler(new_sec, it, buf_end, stg); + run_handler(new_sec, it, buf_end, stg, recursion + 1); state = match_state_array_after_value; array_md = array_mode_sections; }else if(*it == '"') @@ -270,7 +273,7 @@ namespace epee typename t_storage::hsection new_sec = NULL; bool res = stg.insert_next_section(h_array, new_sec); CHECK_AND_ASSERT_THROW_MES(res&&new_sec, "failed to insert next section"); - run_handler(new_sec, it, buf_end, stg); + run_handler(new_sec, it, buf_end, stg, recursion + 1); state = match_state_array_after_value; }else CHECK_ISSPACE(); break; @@ -372,7 +375,7 @@ namespace epee std::string::const_iterator sec_buf_begin = buff_json.begin(); try { - run_handler(nullptr, sec_buf_begin, buff_json.end(), stg); + run_handler(nullptr, sec_buf_begin, buff_json.end(), stg, 0); return true; } catch(const std::exception& ex) diff --git a/tests/unit_tests/epee_levin_protocol_handler_async.cpp b/tests/unit_tests/epee_levin_protocol_handler_async.cpp index fe8260a1..dbc6602a 100644 --- a/tests/unit_tests/epee_levin_protocol_handler_async.cpp +++ b/tests/unit_tests/epee_levin_protocol_handler_async.cpp @@ -12,7 +12,9 @@ #include "net/levin_protocol_handler_async.h" #include "net/net_utils_base.h" #include "unit_tests_utils.h" - +#include "storages/parserse_base_utils.h" +#include "storages/portable_storage_base.h" +#include "storages/portable_storage.h" namespace { struct test_levin_connection_context : public epee::net_utils::connection_context_base @@ -504,3 +506,61 @@ TEST_F(test_levin_protocol_handler__hanle_recv_with_invalid_data, handles_unexpe ASSERT_FALSE(m_conn->m_protocol_handler.handle_recv(m_buf.data(), m_buf.size())); } + +using epee::serialization::portable_storage; +using epee::serialization::array_entry; +using epee::serialization::section; +using epee::serialization::throwable_buffer_reader; + +/** + * Purpose: + * Verify what the deserialization of array_entry no longer uses memcpy to + * overwrite the boost::variant memory directly. Instead, an unsupported-array-entry + * path should throw an exception indicating array_entry deserialization isn't supported. + */ +TEST(levin_protocol_variant_memcpy, memcpy_variant_verify) +{ + std::string buf; // raw buffer simulating an array_entry section + + buf.push_back(static_cast(SERIALIZE_FLAG_ARRAY | SERIALIZE_TYPE_ARRAY)); + buf.push_back(static_cast(1 << 2)); + buf.append(sizeof(array_entry), char(0x41)); + + throwable_buffer_reader reader(reinterpret_cast(buf.data()), buf.size()); + + EXPECT_THROW( + reader.load_storage_array_entry(SERIALIZE_TYPE_ARRAY), + std::runtime_error + ) << "Expected load_storage_array_entry to throw due to array_entry"; +} + +/** + * Purpose: + * Construct a JSON string nested deeper than the built-in recursion limit (100 levels). + */ +TEST(json_parse_deep, parser_deep) +{ + const int depth = 200; + std::string json; + json.reserve(depth * 10); + + // Build a deeply nested JSON + // {"level": {"level": { ... {"level":1} ... }}} + for (int i = 0; i < depth; ++i) + { + json += '{'; + json += "\"level\":"; + } + json += '1'; + + for (int i = 0; i < depth; ++i) + { + json += '}'; + } + + portable_storage storage; + bool ok = epee::serialization::json::load_from_json(json, storage); + + EXPECT_FALSE(ok) << "Expected load_from_json to fail when depth " << depth + << " exceeds the 100-level recursion limit."; +}