Repository navigation
Add ASan/UBSan CI job and a libFuzzer smoke test for decode - #36
Merged
Merged
Conversation
The per-point delta accumulator is an int32, and each decoded delta can be as large as +/-2^30 - so three adversarial max-size deltas (a 24-byte ASCII string) push the sum past INT32_MAX, which is signed-overflow UB. Found by the new fuzz harness before it even landed; UBSan reports 'signed integer overflow: 2147483646 + 1073741823' at the += line. Accumulate through uint32 with well-defined wrap-around instead. This matches the Java original's int semantics bit for bit, so well-formed input decodes identically; malformed input keeps yielding unspecified (but now UB-free) coordinates, as documented. Regression test decodes exactly that adversarial input.
Two new Ubuntu/clang jobs in ci.yml: - sanitizers: the full test suite built with -fsanitize=address,undefined -fno-sanitize-recover=all, so any UB report fails the job; - fuzz-smoke: a 60-second libFuzzer run over geo::decode, the only parser of untrusted input in the library. The fuzz harness (tests/fuzz/, opt-in via GEO_UTILS_CPP_BUILD_FUZZERS, Clang only) checks memory safety plus a round-trip property: re-encoding decoded points always preserves the count, and reproduces the points exactly when they are plausible coordinates (malformed input can decode to jumps beyond the format's 32-bit delta range, which legitimately don't survive re-encoding - hence the is_valid gate).
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.
What
Two new Ubuntu/clang jobs in
ci.yml:-fno-sanitize-recover=all: any UB report fails the job);geo::decode, the only parser of untrusted input in the library.The harness lives in
tests/fuzz/behind a new opt-inGEO_UTILS_CPP_BUILD_FUZZERSoption (Clang only). Besides memory safety it checks a round-trip property: re-encoding decoded points always preserves the count, and reproduces the points exactly when all of them are plausible coordinates (is_valid) — malformed input can decode to coordinate jumps beyond the format's 32-bit delta range, which legitimately don't survive re-encoding.Found (and fixed) by the harness before it even landed
The first commit fixes real UB the fuzzer immediately exposes: the per-point delta accumulator in
decodeis anint32, and each decoded delta can reach ±2³⁰ — a 24-byte ASCII string (}~~~~~@?}~~~~~@?}~~~~~@?) drives the sum pastINT32_MAX:The fix accumulates through
uint32with well-defined wrap-around — bit-for-bit the Java original'sintsemantics, so well-formed input decodes identically; malformed input keeps yielding unspecified (but now UB-free) coordinates, exactly asdecode's docs promise. Regression test included.Verification
fuzz-smokejob on this PR (macOS Command Line Tools ship no libFuzzer runtime — noted intests/fuzz/CMakeLists.txt).