Skip to content

fix(types): reject integers that wrap around in parseUInt64 - #8678

Merged
jpfr merged 2 commits into
open62541:masterfrom
b84330808:fix-parse-num-overflow
Nov 6, 2026
Merged

jpfr merged 2 commits into
open62541:masterfrom
b84330808:fix-parse-num-overflow

Conversation

@b84330808

Copy link
Copy Markdown
Contributor

parseUInt64() only reported an overflow when the new value was smaller than the one before the last digit. That misses wraps that land higher: 23058430092136939520 (2^61 * 10) wraps to 2^62, which is larger than 2^61, so it was accepted. The JSON and XML decoders then returned 4611686018427387904 for a UInt64 with that value, and -4611686018427387904 for an Int64 of -23058430092136939520. The hex branch has the same gap when the shift drops a set bit, e.g. 0x18000000000000000 becomes 0x8000000000000000.

This checks before the multiplication and the shift whether the result still fits, and drops the prev comparison. Only 64-bit values are affected; any missed wrap lands above 2^64/9, so 32-bit range checks downstream already rejected them.

Tests: new decimal, hex and signed cases in check_parse_num.c, and two JSON decode cases in check_types_builtin_json.c.

parseUInt64 detected an overflow only when the new value was smaller than the value before the
last digit. A wrapped value can still be larger: 23058430092136939520 (2^61 * 10) wraps around
to 2^62 and was accepted, so the JSON and XML decoders returned 2^62 for it (and -2^62 for its
negative form through parseInt64). The hex branch had the same problem when the shift dropped a
set bit, e.g. for 0x18000000000000000.

Check before the multiplication and the shift whether the result still fits.
@CLAassistant

CLAassistant commented Nov 4, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@jpfr

jpfr commented Nov 5, 2026

Copy link
Copy Markdown
Member

Dividing by 10 in every iteration can be very expensive. Check if it can be implemented more efficient - or if compilers optimize sufficiently.

@b84330808

Copy link
Copy Markdown
Contributor Author

Good point. It now compares against UINT64_MAX / 10 and UINT64_MAX % 10, which are compile-time constants, so there is no division at runtime (clang -O2 emits no div for this file on x86-64).

@jpfr
jpfr merged commit 9ef0d56 into open62541:master Nov 6, 2026
150 of 151 checks passed
@jpfr

jpfr commented Nov 6, 2026

Copy link
Copy Markdown
Member

Thanks, merged.

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.

3 participants