From f0edab2363ae7dd50ee04bfacdb56f47ed6e7fb7 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Thu, 29 Dec 2016 17:00:02 +0100 Subject: [PATCH] :ambulance: fix for #408 --- src/json.hpp | 37 ++++++--------------------------- src/json.hpp.re2c | 37 ++++++--------------------------- test/src/unit-regression.cpp | 40 ++++++++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 62 deletions(-) diff --git a/src/json.hpp b/src/json.hpp index aab3b34f..f8d948f1 100644 --- a/src/json.hpp +++ b/src/json.hpp @@ -6879,27 +6879,6 @@ class basic_json } } - /*! - @brief checks if a given length does not exceed the size of a given vector - - To secure the access to the byte vector during CBOR/MessagePack - deserialization, bytes are copied from the vector into buffers. This - function checks if the number of bytes to copy (@a len) does not exceed the - size of the given vector @a vec. - - @param[in] vec byte vector - @param[in] len length - - @throws out_of_range if `len > v.size()` - */ - static void check_length(const std::vector& vec, const size_t& len) - { - if (len > vec.size()) - { - throw std::out_of_range("len out of range"); - } - } - /*! @brief create a JSON value from a given MessagePack vector @@ -6916,6 +6895,9 @@ class basic_json */ static basic_json from_msgpack_internal(const std::vector& v, size_t& idx) { + // make sure reading 1 byte is safe + check_length(v.size(), 1, idx); + // store and increment index const size_t current_idx = idx++; @@ -7153,6 +7135,9 @@ class basic_json */ static basic_json from_cbor_internal(const std::vector& v, size_t& idx) { + // make sure reading 1 byte is safe + check_length(v.size(), 1, idx); + // store and increment index const size_t current_idx = idx++; @@ -7674,11 +7659,6 @@ class basic_json */ static basic_json from_msgpack(const std::vector& v) { - if (v.empty()) - { - throw std::invalid_argument("empty vector"); - } - size_t i = 0; return from_msgpack_internal(v, i); } @@ -7736,11 +7716,6 @@ class basic_json */ static basic_json from_cbor(const std::vector& v) { - if (v.empty()) - { - throw std::invalid_argument("empty vector"); - } - size_t i = 0; return from_cbor_internal(v, i); } diff --git a/src/json.hpp.re2c b/src/json.hpp.re2c index 7b6b7ec4..2faf3858 100644 --- a/src/json.hpp.re2c +++ b/src/json.hpp.re2c @@ -6879,27 +6879,6 @@ class basic_json } } - /*! - @brief checks if a given length does not exceed the size of a given vector - - To secure the access to the byte vector during CBOR/MessagePack - deserialization, bytes are copied from the vector into buffers. This - function checks if the number of bytes to copy (@a len) does not exceed the - size of the given vector @a vec. - - @param[in] vec byte vector - @param[in] len length - - @throws out_of_range if `len > v.size()` - */ - static void check_length(const std::vector& vec, const size_t& len) - { - if (len > vec.size()) - { - throw std::out_of_range("len out of range"); - } - } - /*! @brief create a JSON value from a given MessagePack vector @@ -6916,6 +6895,9 @@ class basic_json */ static basic_json from_msgpack_internal(const std::vector& v, size_t& idx) { + // make sure reading 1 byte is safe + check_length(v.size(), 1, idx); + // store and increment index const size_t current_idx = idx++; @@ -7153,6 +7135,9 @@ class basic_json */ static basic_json from_cbor_internal(const std::vector& v, size_t& idx) { + // make sure reading 1 byte is safe + check_length(v.size(), 1, idx); + // store and increment index const size_t current_idx = idx++; @@ -7674,11 +7659,6 @@ class basic_json */ static basic_json from_msgpack(const std::vector& v) { - if (v.empty()) - { - throw std::invalid_argument("empty vector"); - } - size_t i = 0; return from_msgpack_internal(v, i); } @@ -7736,11 +7716,6 @@ class basic_json */ static basic_json from_cbor(const std::vector& v) { - if (v.empty()) - { - throw std::invalid_argument("empty vector"); - } - size_t i = 0; return from_cbor_internal(v, i); } diff --git a/test/src/unit-regression.cpp b/test/src/unit-regression.cpp index 6123352d..421a386c 100644 --- a/test/src/unit-regression.cpp +++ b/test/src/unit-regression.cpp @@ -570,4 +570,44 @@ TEST_CASE("regression tests") std::vector vec5 {0xfb, 0x8f, 0x0a}; CHECK_THROWS_AS(json::from_cbor(vec5), std::out_of_range); } + + SECTION("issue #408 - Heap-buffer-overflow (OSS-Fuzz issue 344)") + { + // original test case + std::vector vec1 {0x87}; + CHECK_THROWS_AS(json::from_msgpack(vec1), std::out_of_range); + + // more test cases for MessagePack + for (uint8_t b : + { + 0x81, 0x82, 0x83, 0x84, 0x85, 0x86, 0x87, 0x88, 0x89, 0x8a, 0x8b, 0x8c, 0x8d, 0x8e, 0x8f, // fixmap + 0x91, 0x92, 0x93, 0x94, 0x95, 0x96, 0x97, 0x98, 0x99, 0x9a, 0x9b, 0x9c, 0x9d, 0x9e, 0x9f, // fixarray + 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7, 0xa8, 0xa9, 0xaa, 0xab, 0xac, 0xad, 0xae, 0xaf, // fixstr + 0xb0, 0xb1, 0xb2, 0xb3, 0xb4, 0xb5, 0xb6, 0xb7, 0xb8, 0xb9, 0xba, 0xbb, 0xbc, 0xbd, 0xbe, 0xbf + }) + { + std::vector vec(1, b); + CHECK_THROWS_AS(json::from_msgpack(vec), std::out_of_range); + } + + // more test cases for CBOR + for (uint8_t b : + { + 0x61, 0x62, 0x63, 0x64, 0x65, 0x66, 0x67, 0x68, 0x69, 0x6a, 0x6b, 0x6c, 0x6d, 0x6e, 0x6f, + 0x70, 0x71, 0x72, 0x73, 0x74, 0x75, 0x76, 0x77, // UTF-8 string + 0x81, 0x82, 0x83, 0x84, 0x85, 0x86, 0x87, 0x88, 0x89, 0x8a, 0x8b, 0x8c, 0x8d, 0x8e, 0x8f, + 0x90, 0x91, 0x92, 0x93, 0x94, 0x95, 0x96, 0x97, // array + 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7, 0xa8, 0xa9, 0xaa, 0xab, 0xac, 0xad, 0xae, 0xaf, + 0xb0, 0xb1, 0xb2, 0xb3, 0xb4, 0xb5, 0xb6, 0xb7 // map + }) + { + std::vector vec(1, b); + CHECK_THROWS_AS(json::from_cbor(vec), std::out_of_range); + } + + // special case: empty input + std::vector vec2; + CHECK_THROWS_AS(json::from_cbor(vec2), std::out_of_range); + CHECK_THROWS_AS(json::from_msgpack(vec2), std::out_of_range); + } }