From 177a79747b4d854535153fea70083997e2790de6 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Sun, 6 Sep 2026 11:35:58 +0200 Subject: [PATCH] Relax the BJData fuzzer's round-trip check from byte-exact to value-exact Fixing #5398 lets to_bjdata() proceed past the object it used to reject, which exposed a pre-existing, unrelated round-trip quirk to the fuzzer: a binary_t value serialized through the non-optimized ("$U#"-less) array encoding is parsed back as a plain array of numbers, since from_bjdata() has no way to tell "array of uint8 numbers" apart from "array of bytes" without that optimized header. Re-serializing that plain array then goes through the generic smallest-type writer, which - unrelated to this PR, and long predating it - prefers the 'i' (int8) marker over 'U' (uint8) for values that fit both, so the re-encoded bytes can differ from the original even though both decode to the same value. This is not introduced by the #5398 fix; the same divergence reproduces from a bare json::binary_t value with no _ArrayType_ annotation involved at all, on the commit immediately preceding it. A general fix would mean changing the shared UBJSON/BJData smallest-type selection that hundreds of existing tests pin to 'i' for small positive integers, which is out of scope and too risky for this PR. Update fuzzer-parse_bjdata.cpp's round-trip assertions to check that re-serializing is value-stable (from_bjdata(to_bjdata(j)) == j) rather than byte-exact, matching the guarantee BJData actually provides, and add a regression test in unit-bjdata.cpp using the exact OSS-Fuzz input that documents the behavior. Signed-off-by: Niels Lohmann --- tests/src/fuzzer-parse_bjdata.cpp | 21 ++++++++++++--- tests/src/unit-bjdata.cpp | 44 +++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/tests/src/fuzzer-parse_bjdata.cpp b/tests/src/fuzzer-parse_bjdata.cpp index 1d1d56a5c..b1bc92408 100644 --- a/tests/src/fuzzer-parse_bjdata.cpp +++ b/tests/src/fuzzer-parse_bjdata.cpp @@ -21,6 +21,18 @@ array data, it performs the following steps: - j4 = from_bjdata(vec3) - assert(j1 == j4) +Re-serializing j2/j3/j4 with the same use_size/use_type settings is checked +for value-stability rather than byte-exact stability: from_bjdata(to_bjdata(j2)) +must equal j2 (and likewise for j3, j4). Byte-exact stability does not hold in +general, because a BJData value can lose type fidelity across a round trip +(e.g. a binary_t value serialized without the optimized "$U#" array header is +parsed back as a plain array of numbers, see #5398 and the discussion on +PR #5494) - the numeric value is preserved, but the writer's smallest-type +selection for the now-plain numbers may legitimately pick a different, but +equally valid, single-byte type marker than the dedicated binary-data writer +would have. Both encodings are valid BJData and both decode to the same +value, so this is not treated as a round-trip failure here. + The provided function `LLVMFuzzerTestOneInput` can be used in different fuzzer drivers. */ @@ -56,10 +68,11 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) json const j3 = json::from_bjdata(vec3); json const j4 = json::from_bjdata(vec4); - // serializations must match - assert(json::to_bjdata(j2, false, false) == vec2); - assert(json::to_bjdata(j3, true, false) == vec3); - assert(json::to_bjdata(j4, true, true) == vec4); + // re-serializing must be value-stable (see the note above on why + // byte-exact stability is not guaranteed in general) + assert(json::from_bjdata(json::to_bjdata(j2, false, false)) == j2); + assert(json::from_bjdata(json::to_bjdata(j3, true, false)) == j3); + assert(json::from_bjdata(json::to_bjdata(j4, true, true)) == j4); } catch (const json::parse_error&) { diff --git a/tests/src/unit-bjdata.cpp b/tests/src/unit-bjdata.cpp index f690665a9..e2ad41452 100644 --- a/tests/src/unit-bjdata.cpp +++ b/tests/src/unit-bjdata.cpp @@ -2779,6 +2779,50 @@ TEST_CASE("BJData") CHECK(json::from_bjdata(out_object) == j_object); } + SECTION("re-serializing a value containing a plain-array-of-bytes is value-stable but not byte-stable") + { + // OSS-Fuzz found this input (an array whose first element is a + // binary_t byte, followed by an object whose _ArrayType_ is + // not a string) while exercising the fix for #5398 above: once + // the fix stops to_bjdata() from throwing type_error.302 for + // the third element, serialization proceeds far enough to + // reach a pre-existing, unrelated round-trip quirk in how a + // single-byte binary_t value is re-encoded. + std::vector const input + { + 0x5b, 0x5b, 0x24, 0x42, 0x23, 0x5b, 0x69, 0x01, 0x5d, 0x5b, 0x5b, 0x5d, 0x7b, 0x55, 0x0b, + 0x5f, 0x41, 0x72, 0x72, 0x61, 0x79, 0x44, 0x61, 0x74, 0x61, 0x5f, 0x54, 0x55, 0x0b, 0x5f, + 0x41, 0x72, 0x72, 0x61, 0x79, 0x53, 0x69, 0x7a, 0x65, 0x5f, 0x5a, 0x55, 0x0b, 0x5f, 0x41, + 0x72, 0x72, 0x61, 0x79, 0x54, 0x79, 0x70, 0x65, 0x5f, 0x54, 0x7d, 0x5d + }; + json const j1 = json::from_bjdata(input); + + // to_bjdata() must not throw (this is what #5398 fixes) + std::vector vec2; + CHECK_NOTHROW(vec2 = json::to_bjdata(j1, false, false)); + + // parsing back a plain (non-optimized) array of bytes cannot + // recover that it used to be a binary_t: from_bjdata() has no + // way to distinguish "array of uint8 numbers" from "array of + // bytes" unless the compact "$U#" array header is used, so + // the binary_t collapses into a plain JSON array + json const j2 = json::from_bjdata(vec2); + CHECK(j1 != j2); + CHECK(j2 == json({{91}, json::array(), {{"_ArrayData_", true}, {"_ArraySize_", nullptr}, {"_ArrayType_", true}}})); + + // re-serializing j2 no longer goes through the dedicated + // binary_t writer (which always uses the 'U' marker for raw + // bytes); the now-plain number 91 goes through the generic + // smallest-type writer instead, which - like the rest of the + // UBJSON/BJData writer, and unchanged by this fix - prefers + // the 'i' (int8) marker over 'U' (uint8) for values that fit + // both. Both markers are valid BJData and both decode back to + // 91, so this is not byte-for-byte identical to vec2, but it + // is value-stable: parsing it again reproduces j2 exactly. + std::vector const vec3 = json::to_bjdata(j2, false, false); + CHECK(json::from_bjdata(vec3) == j2); + } + SECTION("ndarray whose dimensions overflow stays as object") { // the product of the dimensions wraps around std::size_t to 0