* Check the fuzzers' UBJSON/BJData round-trip invariants in the unit tests
The strongest correctness checks for the UBJSON and BJData writers lived
only in the OSS-Fuzz drivers: anything from_ubjson()/from_bjdata()
returns must serialize with every option combination, parse back, and
re-serialize stably. Those checks only run at OSS-Fuzz, so regressions
surfaced days later as external reports - the same BJData assert pair
was reported five times over three years, and #5494's harness change
was followed by OSS-Fuzz 563659413 within a day.
Add "UBJSON round-trip invariants" and "BJData round-trip invariants"
test cases that run the drivers' checks on a fixed, deterministic corpus
(tests/src/round_trip_corpus.hpp): integer and float boundaries,
non-finite numbers, strings, binary values, optimized containers, deep
nesting, the JData annotated-array matrix, and seeded random containers.
They also check two properties the drivers do not: the first round trip
preserves the value, and re-serializing reproduces the exact bytes. For
BJData both exclude values containing a binary value, which is read back
as an array of integers unless it was written as a Draft 3 optimized
binary array; this carve-out is now documented in bjdata.md. Run against
the headers before #5542, the BJData test fails, including on the shape
from OSS-Fuzz 563659413.
Also document how OSS-Fuzz reports are handled (reference them as
"OSS-Fuzz: <id>", turn the reproducer into a unit test, keep drivers and
unit tests in sync) in tests/fuzzing.md, and link it from the PR
template and the quality assurance page.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Add the OSS-Fuzz reproducers for 474400817 and 474480402 as unit tests
Following the convention added to tests/fuzzing.md, the reproducers of
the two BJData fuzzer asserts tracked since January are now unit tests:
- 474400817 (assert(false)): an empty object _ArraySize_ was written as
the ND-array header length, which from_bjdata() could not read back.
Fixed by #5455.
- 474480402 (to_bjdata(j2, false, false) == vec2): a one-byte Draft 3
binary array is written in Draft 2 mode as a uint8 array and then
re-serialized with the int8 marker. This is the documented exception to
byte stability, not a library bug; OSS-Fuzz closed it after #5494
relaxed the harness to value stability. The test pins the exact bytes
so the exception stays deliberate.
The 563659413 reproducer is already a unit test (#5542). A comment also
ties the existing UBJSON excessive-count test to the timeout OSS-Fuzz
reported for that shape (testcase 6347769435193344).
OSS-Fuzz: 474400817
OSS-Fuzz: 474480402
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Fix GCC -Weffc++ and -Wuseless-cast warnings in the round-trip corpus
Initialize the atoms in the member initialization list, and drop the cast of
the generator's result, which already is std::size_t on 64-bit Linux.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
---------
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
write_bjdata_ndarray() encoded a JData-annotated object as a BJData
ND-array whenever its dimensions' product matched _ArrayData_.size(),
which lost information in two ways:
- _ArrayData_ was never required to be an array. null has size 0, any
other scalar has size 1, and iterating an object visits its values, so
e.g. {"_ArraySize_":[1],"_ArrayData_":5} was written as the array [5],
and an object _ArrayData_ came back as an array.
- The reader only restores an annotated object from an ND-array with at
least two non-zero dimensions that is not a 1xN row vector; an empty,
1-D, row-vector, or zero-sized shape is read back as a plain array. The
writer nonetheless emitted ND-array headers for these shapes, so the
annotation was silently dropped.
OSS-Fuzz issue 563659413 hit this in parse_bjdata_fuzzer: an empty binary
_ArraySize_ is written as a plain object and read back as an empty array,
after which {"_ArrayType_":"int16","_ArraySize_":[],"_ArrayData_":null}
was encoded as the ND-array header "[$I#[]" and re-read as [], failing the
harness's value-stability check.
Such objects now fall back to a plain object encoding, which round-trips.
Genuine ND-arrays (two or more positive dimensions, not a 1xN row vector)
are encoded exactly as before. Existing fallback tests that used 1-D
shapes are moved to 2-D shapes so they keep exercising the check they
were written for, and the BJData documentation is updated.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Fix to_bjdata() emitting unparsable output when _ArraySize_ is not an array
write_bjdata_ndarray() never checked that _ArraySize_ is an array. The shape
is written verbatim as the header length, so a null shape emitted 'Z' and an
object shape emitted '{' after the '#', neither of which from_bjdata()
accepts, and the round-trip guarantee in the BJData docs was broken.
Both slipped through the existing validation: for null, empty() is true so
the element count starts at 0 and the per-dimension loop never runs, and for
an object the loop walks its values, which can satisfy the non-negative
integer check. When _ArrayData_ then matched that count, the writer took the
ndarray path.
Require the shape to be an array, so anything else falls back to a plain
object encoding that round-trips, as the fallback rule in the docs already
specifies.
Signed-off-by: qatcod <79017227+qatcod@users.noreply.github.com>
* Document that _ArraySize_ must be an array in the ndarray requirements
The list at bjdata.md is the exhaustive set of conditions for the ndarray
encoding, but it only implied this one through 'every entry of'.
Signed-off-by: qatcod <79017227+qatcod@users.noreply.github.com>
---------
Signed-off-by: qatcod <79017227+qatcod@users.noreply.github.com>
* Do not write BJData ndarrays whose size overflows std::size_t
write_bjdata_ndarray() multiplied the _ArraySize_ dimensions into a
std::size_t without checking for overflow. A product that wraps around
to a value that happens to match the size of _ArrayData_ passed the
length check, and the writer emitted an ndarray header announcing an
element count that cannot be represented:
{"_ArrayType_":"uint8","_ArraySize_":[9223372036854775808,2],"_ArrayData_":[]}
was encoded as 5b 24 55 23 5b 4d 00 00 00 00 00 00 00 80 69 02 5d, an
ndarray of 2^64 elements followed by no data. Reading that back throws
out_of_range.408 ("excessive ndarray size caused overflow"), so to_bjdata
produced output that from_bjdata rejects. This is reachable by parsing
untrusted JSON and re-encoding it as BJData.
Mirror the overflow check the binary reader already performs, and also
reject a single dimension that does not fit into std::size_t, which the
previous cast silently truncated where std::size_t is narrower than 64
bits. Such objects now fall back to a plain object encoding, which is
what the surrounding type and length validation already does for
annotations it cannot represent, and they round-trip unchanged.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
* Document when to_bjdata converts a JData annotation to an ND-array
The BJData page described the 1-D vector case as the only situation in
which an object carrying _ArrayType_/_ArraySize_/_ArrayData_ is not
written as a compact ND-array. The writer has always had several other
fallbacks -- an unknown _ArrayType_, a dimension that is not a
non-negative integer, an _ArrayData_ whose length does not match the
product of the dimensions, and elements that are not numbers of the
annotated kind -- all of which cause the value to be serialized as a
regular JSON object instead.
Spell out the conditions, including the size-overflow check added in the
preceding commit, so the documented behavior matches the implementation.
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
---------
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Niels Lohmann
Qatadaha Bin Matloob
co63oc
Nebojša Cvetković
