Range-check integral double literals in IntegerJsonHandler - #1450
Open
bjornblissing wants to merge 2 commits into
Open
bjornblissing wants to merge 2 commits into
bjornblissing wants to merge 2 commits into
Conversation
IntegerJsonHandler<T>::readDouble rejects non-integral doubles, but cast integral ones directly to T with no range check. A JSON number written in float-like lexical form with a magnitude outside the range of T (e.g. 1e20 for an int64_t field) therefore reached a static_cast that is undefined behavior for out-of-range conversions and can silently produce a garbage value. Fields typed this way include bufferView.byteOffset/byteLength, so a garbage value can propagate into buffer bounds checks elsewhere. Reject out-of-range values the same way non-integral doubles are already rejected, by forwarding to JsonHandler::readDouble so a warning is reported instead of performing the unchecked cast. The bounds cannot be compared via static_cast<double>(max()): for 64-bit types that rounds up to 2^63 (or 2^64), which would admit the first out-of-range value. Compare against the exclusive upper bound 2^digits instead, which is always exactly representable as a double.
Cover IntegerJsonHandler<T>::readDouble's boundary behavior for int64_t and uint64_t directly, and extend the glTF accessor count test with out-of-range integral doubles (1e20, 2^63). These values previously produced garbage results via undefined-behavior casts instead of being rejected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
IntegerJsonHandler<T>::readDouble(CesiumJsonReader/include/CesiumJsonReader/IntegerJsonHandler.h) already rejects JSON numbers with a fractional part, but an integral-valued double (e.g.1e20) was cast directly toTwithstatic_cast<T>(intPart)and no range check. If the value's magnitude is outside the range ofT, thatstatic_castis undefined behavior and can silently produce a garbage value.This matters beyond generic robustness: fields such as
bufferView.byteOffsetandbufferView.byteLengthare parsed through this handler, so a crafted JSON document with an out-of-range float-like literal for one of these fields can inject a garbage value that later flows into buffer bounds checks.The fix rejects out-of-range integral doubles the same way non-integral ones are already rejected, by forwarding to
JsonHandler::readDoubleso a warning is reported instead of performing the unchecked cast. The range comparison avoidsstatic_cast<double>(std::numeric_limits<T>::max()), since for 64-bit types that rounds up to2^63(or2^64), which would incorrectly admit the first out-of-range value; instead it compares against the exclusive upper bound2^digits, which is always exactly representable as adouble.Issue number or link
N/A
Author checklist
CHANGES.mdwith a short summary of my change (for user-facing changes).Testing plan
bufferView.byteOffset) set to a float-like literal whose magnitude is outside the range of the target type, such as1e20for anint64_tfield. Before the fix, this reaches an undefined-behaviorstatic_castand may produce a garbage value silently. After the fix,readDoublerejects it and a warning is reported, matching the existing behavior for non-integral doubles.std::numeric_limits<T>::max()/lowest()) are classified correctly, including for 64-bit integer types where the naivestatic_cast<double>(max())comparison would be off by using a rounded bound.This is a targeted validation fix in the JSON integer parsing path with no user-facing format or API changes.