Skip to content

Keep a valid address in pjuri fuzz mutations - #1828

Merged
benalleng merged 1 commit into
payjoin:masterfrom
erickcestari:custom-mutator-pjuri
Aug 21, 2026
Merged

Keep a valid address in pjuri fuzz mutations#1828
benalleng merged 1 commit into
payjoin:masterfrom
erickcestari:custom-mutator-pjuri

Conversation

@erickcestari

Copy link
Copy Markdown
Contributor

A bitcoin address is checksummed, so libFuzzer cannot construct one. Unlike a character-set check, which it learns from the comparisons it traces, a checksum gives it no way to turn the constant it needs into input bytes, so every input was rejected before the payjoin parameters were read and the body of the target never ran.

The custom mutator keeps a parseable address, substituting a canonical one when the input carries none, and spends the mutation budget on the query instead. A quarter of mutations still go through untouched to keep the reject paths covered. Over 60s from an empty corpus, coverage rises from 293 to 1433 edges.

Drop the TypeId assertions along the way: they compare compile-time constants, so they cannot fail, and their early returns would have silently disabled the rest of the target.

More context at: #1826

A bitcoin address is checksummed, so libFuzzer cannot construct one.
Unlike a character-set check, which it learns from the comparisons it
traces, a checksum gives it no way to turn the constant it needs into
input bytes, so every input was rejected before the payjoin parameters
were read and the body of the target never ran.

The custom mutator keeps a parseable address, substituting a canonical
one when the input carries none, and spends the mutation budget on the
query instead. A quarter of mutations still go through untouched to
keep the reject paths covered. Over 60s from an empty corpus, coverage
rises from 293 to 1433 edges.

Drop the TypeId assertions along the way: they compare compile-time
constants, so they cannot fail, and their early returns would have
silently disabled the rest of the target.

@caarloshenriq caarloshenriq left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cACK 292cc46

One question: split_uri falls back to DEFAULT_QUERY when the input has no ?, which means the first mutation from an empty corpus always starts with a full payjoin query. Is that intentional as a bootstrap, or would starting with an empty query expose more of the parameter-parsing edge cases (missing keys, partial query strings)?

@erickcestari

erickcestari commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

split_uri falls back to DEFAULT_QUERY when the input has no ?, which means the first mutation from an empty corpus always starts with a full payjoin query. Is that intentional as a bootstrap, or would starting with an empty query expose more of the parameter-parsing edge cases (missing keys, partial query strings)?

It's intentional. When the custom_mutator takes this path (75% of the time), it generates a valid input, so we fall back to DEFAULT_QUERY. The remaining 25% uses fuzzer_mutate, which performs the byte-level mutations provided by libFuzzer, allowing this path to exercise invalid inputs.

@coveralls

Copy link
Copy Markdown
Collaborator

Coverage Report for CI Build 32403746166

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage remained the same at 86.718%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 16458
Covered Lines: 14272
Line Coverage: 86.72%
Coverage Strength: 343.64 hits per line

💛 - Coveralls

@caarloshenriq caarloshenriq left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK 292cc46

Ran uri_deserialize_pjuri for 60s from an empty corpus on x86_64 Linux, 6 forks:

Metric Value
Coverage counters reached 1353
Feature edges 3146
Corpus size 612 inputs
Crashes / OOM / Timeouts 0 / 0 / 0

Coverage was still growing at 60s, which confirms the custom mutator is keeping the fuzzer out of the address checksum wall and spending budget on the payjoin parameters where it matters.

@benalleng benalleng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 292cc46

@benalleng
benalleng merged commit 28c9c33 into payjoin:master Aug 21, 2026
13 checks passed
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.

4 participants