fix: UB in SchemaValidator::Double when casting out-of-range doubles

[KowalskiThomas]

What is this PR?

This PR fixes an undefined behaviour happening when trying to cast a double to an integer (int64_t or uint64_t) outside its valid range.

/home/user/rapidjson/include/rapidjson/schema.h:345:50: runtime error: 1e+33 is outside the range of representable values of type 'unsigned long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/user/rapidjson/include/rapidjson/schema.h:345:50 in
MS: 4 ChangeByte-ChangeByte-InsertRepeatedBytes-CrossOver-; base unit: d06ccadf2965827126a941a193346fc522f20a85

The proposed fix is to set the integer representation to 0 if it's outside the valid value range.

The PR also adds a test validating the new behaviour.

[tencent-adm]

All committers have signed the CLA.

[xhon-pelushi]

Built this locally and tried to break it. The fix is correct, the bounds are right, and clamping is safe here for a reason worth stating explicitly. One real gap: the new test passes without the fix, so as written it doesn't guard against regression in a normal build.

Reproduced, including a case the report doesn't mention

On master (24b5e7a), with clang -fsanitize=undefined, my matrix trips both branches, not just the positive one:

include/rapidjson/schema.h:345:50: runtime error: 1e+33 is outside the range of representable values of type 'unsigned long'
include/rapidjson/schema.h:344:49: runtime error: -1e+33 is outside the range of representable values of type 'long'

And NaN / ±Infinity trip it too — reachable through kParseNanAndInfFlag, so no exotic literal needed:

include/rapidjson/schema.h:345:50: runtime error: nan is outside the range of representable values of type 'unsigned long'
include/rapidjson/schema.h:344:49: runtime error: -inf is outside the range of representable values of type 'long'

All of these are clean on the PR head under both clang -fsanitize=undefined and gcc -fsanitize=float-cast-overflow.

The bounds are right, and match the existing convention

Both are exactly the guards GenericValue::IsLosslessDouble() already uses at document.h:1139 and document.h:1133, which is reassuring:

• d >= static_cast<double>(INT64_MIN) — -2^63 is exactly representable, so every double in [-2^63, 0) casts fine.
• d < 18446744073709551616.0 — worth noting for future readers that the literal is doing real work here. static_cast<double>(UINT64_MAX) rounds up to 2^64, so a d <= static_cast<double>(UINT64_MAX) guard would wrongly admit d == 2^64. The literal 2^64 with a strict < is the correct bound.

Clamping to 0 can't cause a false duplicate — and here's why

The obvious worry with n.u.u = 0 is that 1e33, 2e33 and a genuine 0 all collide, turning uniqueItems into a false-positive machine. They don't, because WriteNumber() hashes the whole struct — WriteBuffer(kNumberType, &n, sizeof(n)) — and n.d still holds the exact double. sizeof(Number) is 16 with no padding on LP64, and whichever union member is assigned covers all 8 bytes, so there are no uninitialised bytes feeding the hash either.

Verified by running the semantics rather than assuming them (PR head, gcc, float-cast-overflow on, no diagnostics):

[1e33, 2e33]                    accept=true    distinct huge positives -> unique
[1e33, 1e33]                    accept=false   identical -> duplicate
[-1e33, -2e33]                  accept=true    distinct huge negatives -> unique
[0, 1e33]                       accept=true    real zero vs clamped-to-zero -> unique
[0, -1e33]                      accept=true    real zero vs clamped negative -> unique
[1e33, 3e33, 5e33]              accept=true    three clamped values -> unique
[NaN, Infinity]                 accept=true    unique
[Infinity, -Infinity]           accept=true    unique
[1e19, 10000000000000000000]    accept=false   double vs uint64 of equal value -> duplicate

That last one matters: the cross-type equality the hasher deliberately maintains (Int/Uint64 also populate n.d) is unaffected.

schematest is 140/140 passing on the PR head.

One incidental effect: hash values change on clang builds, because clang's out-of-range conversion happens to yield 2^63 where the fix yields 0. internal::Hasher hashes aren't persisted or part of the public API, so this is only relevant if something in-tree compares hashes across builds — I couldn't find anything that does.

Sibling-site check: nothing else is unguarded

The only other double→integer casts in the headers are document.h:1134 and document.h:1141, and both sit behind short-circuit && range checks, so they never execute out of range. CheckDoubleMinimum / CheckDoubleMaximum / CheckDoubleMultipleOf stay in floating point throughout. Hasher::Double really was the only unguarded site.

The gap: the new test passes on master

Built schematest.cpp from this PR against unpatched master headers:

[ RUN      ] SchemaValidator.UniqueItemsDoubleOutOfIntRangeUB
[       OK ] SchemaValidator.UniqueItemsDoubleOutOfIntRangeUB (1 ms)
[  PASSED  ] 1 test.

It only surfaces the bug when a sanitizer is attached, and then only one that includes float-cast-overflow. The reason it can't fail on its own is the same reason clamping is safe: n.d disambiguates every value, so no semantic outcome depends on what the out-of-range conversion produces — and what it produces is genuinely arbitrary. Same machine, same input, two compilers:

             gcc -O2                 clang -O2
1e+33   -> (uint64_t) 0               9223372036854775808
inf     -> (uint64_t) 0               9223372036854775808
nan     -> (uint64_t) 9223372036854775808   9223372036854775808
-1e+33  -> (int64_t) -9223372036854775808   -9223372036854775808

The part I'd change: RAPIDJSON_BUILD_UBSAN sets plain -fsanitize=undefined, and on GCC that does not include float-cast-overflow — it has to be requested by name. Minimal check on this box:

gcc   -fsanitize=undefined                      -> no diagnostic
gcc   -fsanitize=undefined,float-cast-overflow   -> diagnosed
clang -fsanitize=undefined                       -> diagnosed

So on a GCC UBSan build the new test has no teeth, which is plausibly how this survived in the first place. Adding -fsanitize=float-cast-overflow alongside -fsanitize=undefined in the GCC branch (CMakeLists.txt:93) would give the test real value and catch the whole class rather than this one site. Worth doing in this PR or a follow-up, whichever the maintainers prefer.

I'd also suggest extending the test with kParseNanAndInfFlag cases for NaN and ±Infinity, since those hit the same casts and are reachable through a documented flag rather than needing a 1e33 literal.

What I couldn't test

I only exercised x86-64 Linux with GCC 13.3 and clang 18.1, so the "arbitrary conversion result" table is two data points, not a survey. Separately and unrelated to this PR: a from-scratch -DRAPIDJSON_BUILD_TESTS=ON build of the unittest target fails on GCC 13 with -Werror=stringop-overflow in test/unittest/itoatest.cpp:103 (writing 16 bytes into a region of size 11), so I compiled schematest.cpp directly instead. That file is untouched by this PR.