Repository navigation
Fuzz the archive reader, and fix the crash it found - #130
Merged
Merged
Conversation
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.
Plan item T7 (fuzz the archive reader). An archive given to
--obligation-inis untrusted input, and the reader was tested only with the hand-written mutations ofobligation_archive.test. This adds a libFuzzer target, runs it every night, and fixes the crash its first run found.The bug it found
A product, quotient, or remainder with one operand made replay crash. The validator checked the operand count, and on failure built its error message from both operands anyway (
Obligation.cpp,validateLogicNode), reading past the end of the operand vector. Under AddressSanitizer it is a SEGV inIntrusiveRefCntPtr::operator->; in a release build replay printed whatever lay there:The other cases of the validator check the count before they read an operand; these three now do too. The fuzzer found it within about 7,000 inputs.
The target
clang/tools/cpp-verify-archive-fuzzer/ArchiveFuzzer.cppstates, for any byte string, what replay relies on, and aborts when one does not hold:deserializeObligationModulesreturns an error, or modules thatvalidateObligationModuleaccepts;simplifyObligationModulesucceeds on it, and again on its result, which serializes to the same bytes (canonicalization is a fixpoint, as replay's convergence claim needs);Loweredis a verdict replay reports, not a broken property.It is built with
add_llvm_fuzzer, as LLVM's fuzzers are: with-DLLVM_USE_SANITIZE_COVERAGE=Onand a Clang host it is a libFuzzer binary; otherwise itsDummyArchiveFuzzer.cppmain runs the target on the files it is given (llvm::runFuzzerOnInputs), so every build has it and lit runs it.Nightly CI
.github/workflows/cppverify-fuzz.ymlbuilds the verifier and the target with Clang 18, AddressSanitizer, assertions, and sanitizer coverage, writes an archive for every test program underclang/test/Verify(--lower-only --obligation-out), splits them into records of at most 64 KiB (cppverify-fuzz-seeds.py), and fuzzes for half an hour on every core from those seeds and the corpus earlier nights grew (kept in the Actions cache, merged to its minimal covering set after each run). A crash or a failed property fails the job and uploads the inputs. A pull request that changes the fuzzer or the workflow runs it for five minutes, so this PR runs it.Testing
suite/archive_fuzzer.test(new) writes archives of programs with every kind of term (obligation_ir,collections,sequences,inductive_predicates,choose,heap_reading_spec, a BMC archive ofbmc_incremental, and a small program with*,/,%), mutates one into a product with one operand (mutate_obligation_archive.py unary-product, new), checks replay's error, and runs the target on all of them. It takes 4 s.UNARYcheck fails (the message above); with this PR it passes, as doesobligation_archive.test.Docs
Not done