Skip to content

guard skip_number against end of non-null-terminated input#2637

Closed
uwezkhan wants to merge 1 commit into
stephenberry:mainfrom
uwezkhan:skip-number-end-guard
Closed

guard skip_number against end of non-null-terminated input#2637
uwezkhan wants to merge 1 commit into
stephenberry:mainfrom
uwezkhan:skip-number-end-guard

Conversation

@uwezkhan

Copy link
Copy Markdown
Contributor

Repro: read a number-valued JSON pointer over a non-null-terminated buffer that ends right after the digits, e.g. glz::get_view_json<"/h", opts{.null_terminated=false}> over {"h":-0} with no closing brace. ASAN reports a heap-buffer-overflow read of size 1 in skip_number_with_validation.

Cause: after scanning the digit run, skip_number and skip_number_with_validation peek at *it for a trailing ./e/exponent sign without rechecking it != end. The JSON pointer path (parse_value) forces validate_skipped, so it routes through the validating skip. skip_ws only guarantees a byte at the number's start, not after the digits, so a number that ends the buffer over-reads by one byte.

Fix: carry null_terminated through skip_number_opts and bound each post-advance peek against end, the same way skip_ws already handles non-null-terminated input. Behavioral difference is only at the buffer edge: before, a number ending at end peeked past it; after, that number is treated as complete and a dangling sign or exponent is a syntax error. The null-terminated path is unchanged and still relies on the trailing sentinel, so there is no cost there.

@stephenberry

Copy link
Copy Markdown
Owner

Thanks for catching this, and apologies for the overlap. This exact fix landed in #2636 (merged earlier today), which guards the same two functions in util/parse.hpp: skip_number (gated non-validating loop) and skip_number_with_validation (end-bounded peeks). I verified your repro against current main under ASAN: it reports the heap-buffer-overflow before #2636 and runs clean after, and the non_null_terminated_scanner_bounds suite already covers the validating number-skip boundary. Closing as superseded, but the parallel discovery is much appreciated.

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.

2 participants