Skip to content

Skip memcpy and memcmp for empty ranges - #780

Open
sjawhar wants to merge 3 commits into
danielaparker:masterfrom
theorem-labs:memcpy-empty-ranges
Open

sjawhar wants to merge 3 commits into
danielaparker:masterfrom
theorem-labs:memcpy-empty-ranges

Conversation

@sjawhar

@sjawhar sjawhar commented Oct 26, 2026

Copy link
Copy Markdown

This builds on #779. Merge #778 and #779 first.

memcpy and memcmp with a null pointer are undefined even when the length is 0. jsoncons makes such calls at eight sites when an empty string, byte string, input, typed array, byte string view or buffer has null data. Under clang 20 -fsanitize=undefined, unit_tests at master 91c5e03 reports three:

include/jsoncons/utility/heap_string.hpp:159:24: runtime error: null pointer passed as argument 2, which is declared to never be null
include/jsoncons/basic_json.hpp:580:35: runtime error: null pointer passed as argument 2, which is declared to never be null
include/jsoncons/source.hpp:598:24: runtime error: null pointer passed as argument 2, which is declared to never be null

Fix. Act only when there is something to copy or compare, as allocated_storage::reserve does (bigint.hpp:264). read_typed_array repeated one copy eleven times. It becomes one copy_typed_array, which also holds #779's float range check. The new tests print reports at master, but they pass unless UBSan runs with halt_on_error=1. CI sanitizes with ASan only. With the patch, unit_tests passes under clang 20 -fsanitize=undefined, with none of these reports, and g++ 13. #STREAM_MOVE fixes the memcpy in stream_source::move_assignment.

Written with AI assistance (Claude). I reviewed the change.

json_traits<std::chrono::duration> multiplied epoch counts by 1000 or
10^9, and cast doubles, bigints and uint64 values to Rep, with no range
check. A CBOR epoch of 10000000000 s or a BSON datetime in year 9999
read as nanoseconds was undefined behaviour. 1e300 or NaN read as an
integer duration was also undefined behaviour. A uint64 value above
INT64_MAX wrapped to a negative count.

Check the scaled count against Rep and return not_epoch when it does not
fit. The traits check an integer product before they multiply. not_epoch
is the error the traits already return for a bad epoch string. A new
in_integer_range in utility/conversion.hpp checks a double or bigint
count. A count that fits Rep only after the division, such as a BSON
datetime read as duration<int32_t>, still converts.

The double branch that dc382d9 added to the nanoseconds traits was
never reached, so 1.5 s read as 1000000000 ns. Now the traits scale
the double before they truncate it.
basic_json::try_as_integer, basic_staj_event::get_ and
basic_staj_cursor::read_typed_array cast a double to the target integer
type with no range check. The cast is undefined behaviour outside the
target type's range. json(1e300).as<int64_t>() gives INT64_MIN, and
UBSan reports "1e+300 is outside the range of representable values of
type 'long'". The jsonschema type keyword reaches it with the instance
1e308 of the JSON-Schema-Test-Suite.

Return conv_errc::not_integer instead, the error the first two already
return for a string that is not an integer. read_typed_array gets an
error_code overload, which the decoders use, and the old overload throws
ser_error. Each caller that relied on the cast now handles the error.
The jsonschema type keyword still treats 1e308 as not an integer. The
eight jsonschema keywords that read a size throw schema_error, the error
make_json_schema documents. They throw it for any size that is not a
number or is out of range. That includes the integer -1 as well as the
double -1.0.
memcpy and memcmp with a null pointer are undefined even when the length
is 0. UBSan reports it when an empty string, byte string, input, typed
array, byte string view or read buffer has a null data pointer. The
calls are in heap_string_factory::create, short_string_storage,
chars_source::read, stream_source::read, read_typed_array,
stream_sink::append, binary_stream_sink::append and
byte_traits::compare. Act only when there is something to copy or
compare, as the bigint storage already does. read_typed_array copied
each element type the same way eleven times. That copy, with its range
check for floats, is now one function.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant