Skip to content

ci(fuzz): keep failing inputs out of public logs and artifacts - #864

Open
LeeroyHannigan wants to merge 4 commits into
mainfrom
ci/fuzz-keep-inputs-private
Open

LeeroyHannigan wants to merge 4 commits into
mainfrom
ci/fuzz-keep-inputs-private

Conversation

@LeeroyHannigan

Copy link
Copy Markdown
Collaborator

What

  • The Fuzz step writes the cargo-fuzz and libFuzzer output to a file on the runner, and prints only the final statistics of a clean run. When libFuzzer fails, or leaves any file under fuzz/artifacts, the step fails with a fixed message. The change removes the upload step. A new Build step keeps build errors in the log.
  • The job runs only when the repository owner is ExtendDB, so a fork skips it. .cargo/** is in the pull_request paths.
  • sigv4_timestamp gets only the x-amz-date value. The change removes build_header_map.
  • policy_document gets a dictionary in fuzz/dicts/ and two seeds. The Fuzz step passes the dictionary of a target with -dict.

The workflow does not add -a. cargo-fuzz 0.13.2 builds with -Cdebug-assertions unless the command passes -O, so overflow checks and debug_assert! are already on.

Why

@robinnsc raised these points on #798.

  • The log and the artifacts of a run in this repository are public. libFuzzer prints a failing input of 256 bytes or less as bytes and base64, and cargo-fuzz prints it in full. libFuzzer also writes and prints an input that runs for more than 10 seconds, on a run that passes. The SigV4 targets parse a request before the server authenticates the caller. A failing or slow input to them can be a denial of service, and SECURITY.md asks for private reports of those.
  • A fork inherits the schedule but not the runner group, so it queues six jobs for a runner label that it does not have. The resolver setting in .cargo/config.toml applies when the workflow builds fuzz/Cargo.lock.
  • validate_timestamp reads only x-amz-date, so the extra headers used part of each input on bytes that it does not read.
  • The policy seeds have no Principal, NotPrincipal or Sid key, and mutations seldom make one.

Refs #798

Testing done

  • actionlint with shellcheck passes on the workflow, with the runner label declared.
  • A local copy of the Fuzz step script, run on a local build:
    • A clean run printed only the Done and stat:: lines.
    • With -rss_limit_mb=1, the run failed with an out-of-memory input. The step printed only the error line, and the lines that print the input stayed in the log file.
    • With a file under fuzz/artifacts after a clean run, the step failed with the error line.
  • cargo fuzz build -v on nightly-2026-11-01 passed -Cdebug-assertions and -Zsanitizer=address to every target crate.
  • sigv4_timestamp: a 30-second run from the seeds made 4.7 million executions and found no failing input.
  • policy_document, four local 60-second runs for each setup:
    • From the old seeds with no dictionary, the start coverage was 397 and the final coverage was 1341 to 1595. No corpus input had a Principal, NotPrincipal or Sid key.
    • From the new seeds with the dictionary, the start coverage was 532 and the final coverage was 1472 to 1675. Each corpus had 63 to 117 statements with a Principal or NotPrincipal key, and 92 to 122 with a Sid.
  • The fuzz/Cargo.lock step and its version check pass. cargo +1.97.0 fmt --check and cargo +1.97.0 clippy --all-targets -- -D warnings pass in fuzz/.

Checklist

  • I have read CONTRIBUTING.md
  • All tests pass (cargo test --workspace)
  • Code is formatted (cargo fmt --check)
  • Clippy is clean (cargo clippy -- -W clippy::pedantic)
  • I have added or updated tests for new functionality
  • I have updated documentation if behavior changed
  • Breaking changes are noted below (if any)
  • If this changes the wire protocol, Storage trait, auth model, on-disk
    format, or public CLI surface, an RFC has been accepted or is linked
    below. Otherwise, an ADR captures the decision (link below).

The change touches only fuzz/ and the Fuzz workflow, so the workspace tests do not cover it. Clippy ran with -D warnings in fuzz/, without the pedantic group.

ADR / RFC: n/a

Not in this pull request

Separate issues track these points from the #798 comments:


By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache License 2.0 and I agree to the Developer Certificate of
Origin (DCO). See CONTRIBUTING.md for details.

A fork inherits the daily schedule but not the ExtendDB runner group, so
in a fork that enables Actions the schedule queues six jobs for a runner
label that the fork does not have. The job now runs only when the
repository owner is ExtendDB. The runner group belongs to the
organization, so the check reads the repository owner. A pull request
from a fork still runs, because its workflow runs in the base
repository.

The root .cargo/config.toml sets the resolver that cargo metadata uses
when the workflow builds fuzz/Cargo.lock, so the pull_request paths now
include .cargo/.
The log and the artifacts of a workflow run in a public repository are
public. When an input crashed, timed out or ran out of memory, the
workflow uploaded it as an artifact, libFuzzer printed it in the log as
bytes and as base64 when it was 256 bytes or less, and cargo-fuzz
printed it in full. libFuzzer also wrote an input that ran for more than
10 seconds, and printed it under the same size limit, on a run that
passed. The SigV4 targets parse a request before the caller is
authenticated, so such an input can be a denial of service that
SECURITY.md asks to be reported privately.

The Fuzz step now writes the cargo-fuzz and libFuzzer output to a file
under $RUNNER_TEMP and prints only the final statistics of a clean run.
It fails with a fixed message when libFuzzer exits with an error or
leaves any file under fuzz/artifacts, so a slow input fails the job too.
The upload step is removed. A new Build step runs cargo fuzz build
before the Fuzz step, so build errors stay in the log.

Tests: a local run of the step script printed only the statistics for a
clean run. With an out-of-memory failure it printed only the error line,
and the lines that print the input stayed in the log file.
validate_timestamp reads one header, x-amz-date. The target built up to
three arbitrary headers before it, so part of each input went to header
names and values that validate_timestamp does not read. The whole input
is now the x-amz-date value. build_header_map, which only this target
used, is removed. The two seeds lose the leading byte that set the
header count to zero.

Tests: a local 30-second run from the seeds made 4.7 million executions
and found no failing input.
Most mutations of the three policy seeds break the JSON, and no seed
has a Principal, NotPrincipal or Sid key. In four local 60-second runs
from those seeds, no input in the final corpus had a statement with one
of those keys, so the runs did not reach parse_principal_value or the
Sid check in from_json_for_write.

fuzz/dicts/policy_document.dict lists the policy keys, the Effect and
Version values, the condition operators with their ForAllValues:,
ForAnyValue: and IfExists forms, and CIDR forms. The Fuzz step passes
the dictionary of a target with -dict when fuzz/dicts has one. One new
seed has a Principal object, NotResource, a valid Sid, and ForAllValues
and IfExists conditions. The other is a single statement object with
NotPrincipal, NotAction, a Sid that the write path refuses, and the
other operator families.

Tests: the two seeds raise the start coverage that libFuzzer reports
from 397 to 532. In four local 60-second runs with the seeds and the
dictionary, each final corpus had 63 to 117 statements with a
Principal or NotPrincipal key and 92 to 122 with a Sid. The final
coverage was 1472 to 1675, against 1341 to 1595 from the old seeds.

This branch has not been deployed

No deployments
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.

1 participant