Keep converted object keys alive while writing UBJSON and BJData

Since #5746, write_ubjson and write_ubjson_iterative pass each object key
to sanitize_utf8_for_write and keep the returned reference. When
object_t::key_type is not string_t but converts to it, the argument is a
temporary that is destroyed at the end of the statement, and the
function returns a reference to it in every case but a sanitized copy,
so the key bytes are read from a dead object (AddressSanitizer:
stack-use-after-scope). Default json and ordered_json are unaffected.

Bind the key to a named object_key_string_t first: a reference when
key_type is string_t, so no copy is added there, and a converted copy
otherwise. A deleted overload of sanitize_utf8_for_write for anything
other than string_t turns a recurrence into a compile error.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

docs/mkdocs/docs/api/basic_json/binary_t.md

@@ -64,7 +64,18 @@ values of that type directly to a `basic_json` instance, and they will automatic
 rather than arrays:
 
 ```cpp
-using custom_json = nlohmann::ordered_json::with_binary_t<std::vector<std::byte>>;
+using custom_json = nlohmann::basic_json<
+    nlohmann::ordered_map,  // ObjectType
+    std::vector,            // ArrayType
+    std::string,            // StringType
+    bool,                   // BooleanType
+    std::int64_t,           // NumberIntegerType
+    std::uint64_t,          // NumberUnsignedType
+    double,                 // NumberFloatType
+    std::allocator,         // AllocatorType
+    nlohmann::adl_serializer,
+    std::vector<std::byte>  // Custom BinaryType
+>;
 
 std::vector<std::byte> data{std::byte{1}, std::byte{2}, std::byte{3}};
 custom_json j = data;  // Creates a binary value, not an array

docs/mkdocs/docs/examples/as_base_class.cpp

@@ -15,7 +15,19 @@ class base_class_with_hidden_members
     }
 };
 
-using json = nlohmann::json::with_base_class_t<base_class_with_hidden_members>;
+using json = nlohmann::basic_json <
+             std::map,
+             std::vector,
+             std::string,
+             bool,
+             std::int64_t,
+             std::uint64_t,
+             double,
+             std::allocator,
+             nlohmann::adl_serializer,
+             std::vector<std::uint8_t>,
+             base_class_with_hidden_members
+             >;
 
 int main()
 {

include/nlohmann/detail/output/binary_writer.hpp

@@ -85,6 +85,12 @@ template<typename BasicJsonType, typename CharType, typename OutputSinkType = ou
 class binary_writer
 {
     using string_t = typename BasicJsonType::string_t;
+
+    /// an object key as string_t: a reference when object_t::key_type already is
+    /// string_t, otherwise a converted copy that outlives sanitize_utf8_for_write's result
+    using object_key_string_t = typename std::conditional <
+                                std::is_same<typename BasicJsonType::object_t::key_type, string_t>::value,
+                                const string_t&, string_t >::type;
     using binary_t = typename BasicJsonType::binary_t;
     using number_float_t = typename BasicJsonType::number_float_t;
 
@@ -819,8 +825,10 @@ class binary_writer
 
                 for (const auto& el : *j.m_data.m_value.object)
                 {
+                    // a converted key must outlive the reference returned by sanitize_utf8_for_write
+                    const object_key_string_t key_string = el.first;
                     string_t storage;
-                    const string_t& key = sanitize_utf8_for_write(el.first, j, storage);
+                    const string_t& key = sanitize_utf8_for_write(key_string, j, storage);
                     write_number_with_ubjson_prefix(key.size(), true, use_bjdata);
                     oa.write_characters(
                           reinterpret_cast<const CharType*>(key.data()),
@@ -1366,8 +1374,10 @@ class binary_writer
                     continue;
                 }
 
+                // a converted key must outlive the reference returned by sanitize_utf8_for_write
+                const object_key_string_t key_string = current.object_it->first;
                 string_t storage;
-                const string_t& key = sanitize_utf8_for_write(current.object_it->first, j, storage);
+                const string_t& key = sanitize_utf8_for_write(key_string, j, storage);
                 write_number_with_ubjson_prefix(key.size(), true, use_bjdata);
                 oa.write_characters(
                       reinterpret_cast<const CharType*>(key.data()),
@@ -2730,6 +2740,11 @@ class binary_writer
     itself in every case but a sanitized `replace`/`ignore` one, so @a
     storage must outlive the returned reference only then.
 
+    @a s must be an lvalue that outlives the returned reference. An object key
+    whose `key_type` is not @ref string_t must therefore first be converted
+    into a named string_t (see @ref object_key_string_t); the deleted overload
+    below enforces this at compile time.
+
     @param[in] s        the string (value or object key) to write
     @param[in] context  the value @a s belongs to (for diagnostics)
     @param[out] storage  backing storage for a sanitized copy
@@ -2759,6 +2774,10 @@ class binary_writer
         }
     }
 
+    /// deleted: anything but a string_t would bind a temporary that dies before the returned reference is used
+    template < typename T, enable_if_t < !std::is_same<T, string_t>::value, int > = 0 >
+    const string_t& sanitize_utf8_for_write(const T& /*s*/, const BasicJsonType& /*context*/, string_t& /*storage*/) const = delete;
+
     /*!
     @brief write an integer in the shortest encoding
 

single_include/nlohmann/json.hpp

@@ -21590,6 +21590,12 @@ template<typename BasicJsonType, typename CharType, typename OutputSinkType = ou
 class binary_writer
 {
     using string_t = typename BasicJsonType::string_t;
+
+    /// an object key as string_t: a reference when object_t::key_type already is
+    /// string_t, otherwise a converted copy that outlives sanitize_utf8_for_write's result
+    using object_key_string_t = typename std::conditional <
+                                std::is_same<typename BasicJsonType::object_t::key_type, string_t>::value,
+                                const string_t&, string_t >::type;
     using binary_t = typename BasicJsonType::binary_t;
     using number_float_t = typename BasicJsonType::number_float_t;
 
@@ -22324,8 +22330,10 @@ class binary_writer
 
                 for (const auto& el : *j.m_data.m_value.object)
                 {
+                    // a converted key must outlive the reference returned by sanitize_utf8_for_write
+                    const object_key_string_t key_string = el.first;
                     string_t storage;
-                    const string_t& key = sanitize_utf8_for_write(el.first, j, storage);
+                    const string_t& key = sanitize_utf8_for_write(key_string, j, storage);
                     write_number_with_ubjson_prefix(key.size(), true, use_bjdata);
                     oa.write_characters(
                           reinterpret_cast<const CharType*>(key.data()),
@@ -22871,8 +22879,10 @@ class binary_writer
                     continue;
                 }
 
+                // a converted key must outlive the reference returned by sanitize_utf8_for_write
+                const object_key_string_t key_string = current.object_it->first;
                 string_t storage;
-                const string_t& key = sanitize_utf8_for_write(current.object_it->first, j, storage);
+                const string_t& key = sanitize_utf8_for_write(key_string, j, storage);
                 write_number_with_ubjson_prefix(key.size(), true, use_bjdata);
                 oa.write_characters(
                       reinterpret_cast<const CharType*>(key.data()),
@@ -24235,6 +24245,11 @@ class binary_writer
     itself in every case but a sanitized `replace`/`ignore` one, so @a
     storage must outlive the returned reference only then.
 
+    @a s must be an lvalue that outlives the returned reference. An object key
+    whose `key_type` is not @ref string_t must therefore first be converted
+    into a named string_t (see @ref object_key_string_t); the deleted overload
+    below enforces this at compile time.
+
     @param[in] s        the string (value or object key) to write
     @param[in] context  the value @a s belongs to (for diagnostics)
     @param[out] storage  backing storage for a sanitized copy
@@ -24264,6 +24279,10 @@ class binary_writer
         }
     }
 
+    /// deleted: anything but a string_t would bind a temporary that dies before the returned reference is used
+    template < typename T, enable_if_t < !std::is_same<T, string_t>::value, int > = 0 >
+    const string_t& sanitize_utf8_for_write(const T& /*s*/, const BasicJsonType& /*context*/, string_t& /*storage*/) const = delete;
+
     /*!
     @brief write an integer in the shortest encoding
 

tests/src/unit-binary_utf8_error_handler.cpp

@@ -12,7 +12,10 @@
 #include <nlohmann/json.hpp>
 using nlohmann::json;
 
+#include <map>
+#include <memory>
 #include <string>
+#include <utility>
 #include <vector>
 
 namespace
@@ -55,6 +58,49 @@ std::string dump_and_parse(const std::string& raw, eh error_handler)
     return json::parse(json(raw).dump(-1, ' ', false, error_handler)).get<std::string>();
 }
 
+// an object key type that is not string_t, but converts implicitly to it
+class converting_key
+{
+  public:
+    converting_key(const char* s) : m_value(s) {} // NOLINT(google-explicit-constructor,hicpp-explicit-conversions)
+    converting_key(std::string s) : m_value(std::move(s)) {} // NOLINT(google-explicit-constructor,hicpp-explicit-conversions)
+
+    // the conversion yields a temporary string_t
+    operator std::string() const // NOLINT(google-explicit-constructor,hicpp-explicit-conversions)
+    {
+        return m_value;
+    }
+
+    // read by the exception messages when JSON_DIAGNOSTICS is enabled
+    const char* data() const noexcept
+    {
+        return m_value.data();
+    }
+
+    friend bool operator<(const converting_key& lhs, const converting_key& rhs)
+    {
+        return lhs.m_value < rhs.m_value;
+    }
+
+  private:
+    std::string m_value;
+};
+
+// ObjectType using converting_key; the Key template argument is ignored
+template<typename Key, typename Value, typename Compare, typename Allocator>
+class converting_key_object : public std::map<converting_key, Value, std::less<converting_key>, // NOLINT(modernize-use-transparent-functors)
+    typename std::allocator_traits<Allocator>::template rebind_alloc<std::pair<const converting_key, Value>>>
+{
+    using base_type = std::map<converting_key, Value, std::less<converting_key>, // NOLINT(modernize-use-transparent-functors)
+                               typename std::allocator_traits<Allocator>::template rebind_alloc<std::pair<const converting_key, Value>>>;
+
+                             public:
+                               using base_type::base_type;
+                               using base_type::operator=;
+};
+
+using converting_key_json = nlohmann::basic_json<converting_key_object>;
+
 } // namespace
 
 TEST_CASE("UTF-8 error_handler for the binary readers and writers")
@@ -370,3 +416,109 @@ TEST_CASE("UTF-8 error_handler for the binary readers and writers")
         CHECK(json::from_bson(bson_bytes)["k"].get<std::string>() == ill_formed_cases()[0].bytes);
     }
 }
+
+// The UBJSON and BJData writers bind the (possibly sanitized) key to a const
+// string_t&. If key_type is not string_t but converts to it, the converted
+// temporary must outlive that reference; this was a use-after-scope found by
+// AddressSanitizer. Keys exceed the small string optimization on purpose.
+TEST_CASE("UBJSON and BJData writers with an object_t whose key_type is not string_t")
+{
+    const std::string long_prefix(70, 'k');
+
+    SECTION("well-formed keys, every error_handler")
+    {
+        const std::string key1 = long_prefix + "-first";
+        const std::string key2 = long_prefix + "-second";
+
+        converting_key_json::object_t o;
+        o.emplace(converting_key(key1), 1);
+        o.emplace(converting_key(key2), "value");
+        const converting_key_json v(std::move(o));
+
+        json expected;
+        expected[key1] = 1;
+        expected[key2] = "value";
+
+        const bool combos[3][2] = {{false, false}, {true, false}, {true, true}};
+        for (const auto h : all_handlers())
+        {
+            CAPTURE(static_cast<int>(h))
+            for (const auto& combo : combos)
+            {
+                const bool use_count = combo[0];
+                const bool use_type = combo[1];
+                CAPTURE(use_count)
+                CAPTURE(use_type)
+
+                CHECK(json::from_ubjson(converting_key_json::to_ubjson(v, use_count, use_type, h)) == expected);
+                CHECK(json::from_bjdata(converting_key_json::to_bjdata(v, use_count, use_type, json::bjdata_version_t::draft2, h)) == expected);
+                CHECK(json::from_bjdata(converting_key_json::to_bjdata(v, use_count, use_type, json::bjdata_version_t::draft3, h)) == expected);
+            }
+        }
+    }
+
+    SECTION("ill-formed keys")
+    {
+        for (const auto& c : ill_formed_cases())
+        {
+            CAPTURE(c.name)
+            const std::string key = long_prefix + c.bytes;
+
+            converting_key_json::object_t o;
+            o.emplace(converting_key(key), 1);
+            const converting_key_json v(std::move(o));
+
+            CHECK_THROWS_AS(converting_key_json::to_ubjson(v, false, false, eh::strict), converting_key_json::type_error&);
+            CHECK_THROWS_AS(converting_key_json::to_bjdata(v, false, false, json::bjdata_version_t::draft2, eh::strict), converting_key_json::type_error&);
+
+            for (const auto h :
+                    {
+                        eh::replace, eh::ignore
+                    })
+            {
+                CAPTURE(static_cast<int>(h))
+                const std::string expected = dump_and_parse(key, h);
+
+                CHECK(json::from_ubjson(converting_key_json::to_ubjson(v, false, false, h)).begin().key() == expected);
+                CHECK(json::from_bjdata(converting_key_json::to_bjdata(v, false, false, json::bjdata_version_t::draft2, h)).begin().key() == expected);
+            }
+
+            CHECK(json::from_ubjson(converting_key_json::to_ubjson(v, false, false, eh::keep)).begin().key() == key);
+            CHECK(json::from_bjdata(converting_key_json::to_bjdata(v, false, false, json::bjdata_version_t::draft2, eh::keep)).begin().key() == key);
+        }
+    }
+
+    SECTION("nested deeper than the recursion limit")
+    {
+        // wrap the previous value, innermost first
+        converting_key_json v = 42;
+        json expected = 42;
+        for (int i = 199; i >= 0; --i)
+        {
+            const std::string key = "level-" + std::to_string(i) + "-" + std::string(64, 'x');
+
+            converting_key_json::object_t o;
+            o.emplace(converting_key(key), std::move(v));
+            v = converting_key_json(std::move(o));
+
+            json e;
+            e[key] = std::move(expected);
+            expected = std::move(e);
+        }
+
+        for (const auto h : all_handlers())
+        {
+            CAPTURE(static_cast<int>(h))
+            for (const bool use_count :
+                    {
+                        false, true
+                    })
+            {
+                CAPTURE(use_count)
+
+                CHECK(json::from_ubjson(converting_key_json::to_ubjson(v, use_count, false, h)) == expected);
+                CHECK(json::from_bjdata(converting_key_json::to_bjdata(v, use_count, false, json::bjdata_version_t::draft2, h)) == expected);
+            }
+        }
+    }
+}

tests/src/unit-bson.cpp

@@ -46,7 +46,9 @@ class huge_binary_t : public std::vector<std::uint8_t>
     }
 };
 
-using huge_binary_json = nlohmann::json::with_binary_t<huge_binary_t>;
+using huge_binary_json = nlohmann::basic_json <
+                         std::map, std::vector, std::string, bool, std::int64_t, std::uint64_t,
+                         double, std::allocator, nlohmann::adl_serializer, huge_binary_t, void >;
 
 // a string type that can be made to report a size beyond INT32_MAX without
 // allocating that much memory, so BSON length overflow can be tested for
@@ -94,7 +96,9 @@ class huge_string_t : public std::string
     bool pretend_huge = false;
 };
 
-using huge_string_json = nlohmann::json::with_string_t<huge_string_t>;
+using huge_string_json = nlohmann::basic_json <
+                         std::map, std::vector, huge_string_t, bool, std::int64_t, std::uint64_t,
+                         double, std::allocator, nlohmann::adl_serializer, std::vector<std::uint8_t>, void >;
 } // namespace
 
 TEST_CASE("BSON")

tests/src/unit-custom-base-class.cpp

@@ -400,7 +400,20 @@ class base_class_with_hidden_members
     std::size_t m_size = 42;
 };
 
-using json_with_hidden_base_members = nlohmann::json::with_base_class_t<base_class_with_hidden_members>;
+using json_with_hidden_base_members =
+    nlohmann::basic_json <
+    std::map,
+    std::vector,
+    std::string,
+    bool,
+    std::int64_t,
+    std::uint64_t,
+    double,
+    std::allocator,
+    nlohmann::adl_serializer,
+    std::vector<std::uint8_t>,
+    base_class_with_hidden_members
+    >;
 
 TEST_CASE("JSON Node as_base_class")
 {
@@ -446,7 +459,19 @@ struct const_member_base
     const int id = 7; // NOLINT(misc-non-private-member-variables-in-classes)
 };
 
-using json_with_const_base = nlohmann::json::with_base_class_t<const_member_base>;
+using json_with_const_base = nlohmann::basic_json <
+                             std::map,
+                             std::vector,
+                             std::string,
+                             bool,
+                             std::int64_t,
+                             std::uint64_t,
+                             double,
+                             std::allocator,
+                             nlohmann::adl_serializer,
+                             std::vector<std::uint8_t>,
+                             const_member_base
+                             >;
 
 // build an array nested @a depth levels deep, with the innermost value 1;
 // every level is constructed (never assigned), since const_member_base does

tests/src/unit-custom-binary-type.cpp

@@ -26,11 +26,15 @@ namespace
 
 // a BinaryType whose value type is signed: the elements must still be
 // processed as the numbers 0..255
-using char_binary_json = nlohmann::json::with_binary_t<std::vector<char>>;
+using char_binary_json = nlohmann::basic_json <
+                         std::map, std::vector, std::string, bool, std::int64_t, std::uint64_t,
+                         double, std::allocator, nlohmann::adl_serializer, std::vector<char>, void >;
 
 #ifdef JSON_HAS_CPP_17
     // a BinaryType whose value type is not an integer type at all
-    using byte_binary_json = nlohmann::json::with_binary_t<std::vector<std::byte>>;
+    using byte_binary_json = nlohmann::basic_json <
+    std::map, std::vector, std::string, bool, std::int64_t, std::uint64_t,
+    double, std::allocator, nlohmann::adl_serializer, std::vector<std::byte>, void >;
 #endif
 
 } // namespace

tests/src/unit-msgpack.cpp

@@ -2201,7 +2201,10 @@ struct huge_array : std::vector<T, A>
     }
 };
 
-using huge_array_json = nlohmann::json::with_array_t<huge_array>;
+using huge_array_json = nlohmann::basic_json <
+                        std::map, huge_array, std::string, bool, std::int64_t, std::uint64_t,
+                        double, std::allocator, nlohmann::adl_serializer,
+                        std::vector<std::uint8_t>, void >;
 
 TEST_CASE("MessagePack Size above uint32 for array")
 {
@@ -2246,7 +2249,18 @@ template<typename K, typename V,
     }
 };
 
-using huge_object_json = nlohmann::json::with_object_t<huge_map>;
+using huge_object_json = nlohmann::basic_json <
+                         huge_map,
+                         std::vector,
+                         std::string,
+                         bool,
+                         std::int64_t,
+                         std::uint64_t,
+                         double,
+                         std::allocator,
+                         nlohmann::adl_serializer,
+                         std::vector<std::uint8_t>,
+                         void >;
 
 TEST_CASE("MessagePack Size above uint32 for object")
 {
@@ -2281,7 +2295,18 @@ struct huge_string : std::string
     }
 };
 
-using huge_string_json = nlohmann::json::with_string_t<huge_string>;
+using huge_string_json = nlohmann::basic_json <
+                         std::map,
+                         std::vector,
+                         huge_string,
+                         bool,
+                         std::int64_t,
+                         std::uint64_t,
+                         double,
+                         std::allocator,
+                         nlohmann::adl_serializer,
+                         std::vector<std::uint8_t>,
+                         void >;
 
 TEST_CASE("MessagePack Size above uint32 for string")
 {
@@ -2304,7 +2329,18 @@ struct huge_binary : std::vector<std::uint8_t>
     }
 };
 
-using huge_binary_json = nlohmann::json::with_binary_t<huge_binary>;
+using huge_binary_json = nlohmann::basic_json <
+                         std::map,
+                         std::vector,
+                         std::string,
+                         bool,
+                         std::int64_t,
+                         std::uint64_t,
+                         double,
+                         std::allocator,
+                         nlohmann::adl_serializer,
+                         huge_binary,
+                         void >;
 
 TEST_CASE("MessagePack Size above uint32 for binary")
 {
@@ -2354,10 +2390,14 @@ class beyond_uint32_string_t : public std::string
     }
 };
 
-using beyond_uint32_string_json = nlohmann::json::with_string_t<beyond_uint32_string_t>;
+using beyond_uint32_string_json = nlohmann::basic_json <
+                                  std::map, std::vector, beyond_uint32_string_t, bool, std::int64_t, std::uint64_t,
+                                  double, std::allocator, nlohmann::adl_serializer, std::vector<std::uint8_t>, void >;
 #endif
 
-using beyond_uint32_binary_json = nlohmann::json::with_binary_t<beyond_uint32_binary_t>;
+using beyond_uint32_binary_json = nlohmann::basic_json <
+                                  std::map, std::vector, std::string, bool, std::int64_t, std::uint64_t,
+                                  double, std::allocator, nlohmann::adl_serializer, beyond_uint32_binary_t, void >;
 } // namespace
 
 TEST_CASE("MessagePack lengths beyond UINT32_MAX cannot be serialized")

tests/src/unit-ordered_json2.cpp

@@ -217,7 +217,16 @@ void int_to_string(alt_string& target, std::size_t value)
     target = std::to_string(value).c_str();
 }
 
-using alt_json = nlohmann::json::with_string_t<alt_string>;
+using alt_json = nlohmann::basic_json <
+                 std::map,
+                 std::vector,
+                 alt_string,
+                 bool,
+                 std::int64_t,
+                 std::uint64_t,
+                 double,
+                 std::allocator,
+                 nlohmann::adl_serializer >;
 
 bool operator<(const char* op1, const alt_string& op2) noexcept
 {