Skip to content

Commit bc3fef2

Browse files
committed
feat(fuzz): add cargo-fuzz targets + CI, fix RangeExpr overflow panics
Adds a coverage-guided libFuzzer suite (via cargo-fuzz) over the crates that parse or evaluate untrusted input, and a GitHub Actions workflow that smoke- fuzzes every target on PRs/pushes and runs a deeper corpus-accumulating pass on a nightly schedule. Motivation: the quality-evaluation reports in reports/ kept re-discovering the same class of defect by hand — arithmetic overflow and char-boundary panics reachable from attacker-controlled expression text, templates, and manifests (e.g. the expr report's exploratory findings). Encoding that class as a permanent executable check stops it from being re-litigated every review cycle. Six targets, each asserting "any input returns Ok or Err — never panic, abort, or hang": - expr_parse ParsedExpression::new - expr_evaluate parse + evaluate against a fuzzer-seeded symbol table - range_expr RangeExpr::from_str + len/get/iter - format_string FormatString::new + resolve_string_with - model_decode document_string_to_object + decode_{job,environment}_template - snapshot_decode decode_manifest + Manifest::validate The fuzz crate is outside the root workspace (its own empty [workspace] table) so the stable build/test/clippy/MSRV jobs are untouched; it builds only under a pinned nightly with AddressSanitizer, overflow-checks, and debug-assertions on. Small curated seed corpora (mined from the crates' own tests and sample templates) live in fuzz/seeds/ and are committed; the evolving runtime corpus and crash artifacts are git-ignored and cached by CI. The range_expr target immediately reproduced two live overflow panics (expr report finding X1 and a multi-chunk variant), both fixed here at the root: - IntRange::new: normalization arithmetic ((end-start)/step+1 and the endpoint computation) now uses checked ops and returns a parse error for endpoints whose element count or span overflows i64 (e.g. "0-9223372036854775807", full i64-span ranges). - RangeExpr merge: the adjacency test last.end + r.step now uses checked_add, treating overflow as "not adjacent" instead of panicking/wrapping. Adds a huge_span_does_not_overflow regression test. All 3356 existing openjd-expr tests still pass; clippy -D warnings is clean. Signed-off-by: Stephen Crowe <6042774+crowecawcaw@users.noreply.github.com>
1 parent 1eec70b commit bc3fef2

623 files changed

Lines changed: 5230 additions & 7 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.github/workflows/fuzz.yml‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
name: Fuzz
2+
3+
# Smoke-fuzz every fuzz target on pull requests and on merges to main. The
4+
# run is short and time-boxed per target so it gates without slowing the merge
5+
# queue. Any panic, abort, overflow, or char-boundary slice in a fuzzed entry
6+
# point fails the job. Coverage plateaus within about a minute per target once
7+
# seeded, so a longer run buys little here; deeper campaigns are left to manual
8+
# local runs (see fuzz/README.md).
9+
#
10+
# The fuzz crate (fuzz/) is deliberately outside the root workspace and builds
11+
# only with a nightly toolchain under AddressSanitizer, so it lives in its own
12+
# workflow rather than the stable CI matrix.
13+
14+
on:
15+
push:
16+
branches: [main]
17+
pull_request:
18+
branches: [main, release, "patch_*"]
19+
workflow_dispatch:
20+
21+
concurrency:
22+
group: fuzz-${{ github.workflow }}-${{ github.ref }}
23+
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
24+
25+
env:
26+
CARGO_INCREMENTAL: 0
27+
CARGO_NET_RETRY: 10
28+
RUSTUP_MAX_RETRIES: 10
29+
RUST_BACKTRACE: 1
30+
# Per-target smoke budget, in seconds. Six targets run sequentially in one
31+
# job after a single build.
32+
FUZZ_SECONDS: 60
33+
34+
jobs:
35+
fuzz:
36+
name: Fuzz
37+
# libFuzzer + cargo-fuzz's sanitizer build is best supported on Linux.
38+
runs-on: ubuntu-latest
39+
steps:
40+
- uses: actions/checkout@v6
41+
42+
# cargo-fuzz requires a nightly toolchain (sanitizer -Z flags). Pin the
43+
# nightly so a bad nightly (e.g. an ASan codegen ICE) can't randomly break
44+
# the fuzz job; bump this date deliberately.
45+
- name: Install Rust nightly
46+
run: |
47+
rustup toolchain install nightly-2026-05-15 --profile minimal --component rust-src
48+
rustup override set nightly-2026-05-15
49+
50+
- name: Compute rustc hash
51+
id: rustc
52+
run: echo "hash=$(rustc +nightly-2026-05-15 --version --verbose | sha256sum | cut -c1-16)" >> "$GITHUB_OUTPUT"
53+
54+
- name: Restore cargo + fuzz cache
55+
uses: actions/cache@v5
56+
with:
57+
path: |
58+
~/.cargo/registry/index
59+
~/.cargo/registry/cache
60+
~/.cargo/git/db
61+
~/.cargo/bin/cargo-fuzz
62+
fuzz/target
63+
key: fuzz-${{ steps.rustc.outputs.hash }}-${{ hashFiles('**/Cargo.lock', 'fuzz/Cargo.toml') }}
64+
restore-keys: |
65+
fuzz-${{ steps.rustc.outputs.hash }}-
66+
fuzz-
67+
68+
- name: Install cargo-fuzz
69+
run: which cargo-fuzz || cargo install cargo-fuzz --locked --version ^0.13
70+
71+
# One build pass produces all six target binaries.
72+
- name: Build fuzz targets
73+
run: cargo fuzz build
74+
75+
# Loop over every target, each seeded with its committed corpus in
76+
# fuzz/seeds/<target>. A crash in any target fails the whole job (set -e).
77+
- name: Run fuzz targets
78+
run: |
79+
set -euo pipefail
80+
for target in expr_parse expr_evaluate range_expr format_string model_decode snapshot_decode; do
81+
echo "::group::fuzz $target (${FUZZ_SECONDS}s)"
82+
cargo fuzz run "$target" "fuzz/seeds/$target" \
83+
-- -max_total_time="$FUZZ_SECONDS" -timeout=25 -rss_limit_mb=4096
84+
echo "::endgroup::"
85+
done
86+
87+
# If any target produced a crash artifact, surface it so the failure is
88+
# actionable from the run page instead of just a red X.
89+
- name: Upload crash artifacts
90+
if: failure()
91+
uses: actions/upload-artifact@v7
92+
with:
93+
name: fuzz-artifacts
94+
path: fuzz/artifacts
95+
if-no-files-found: ignore

‎crates/openjd-expr/src/range_expr.rs‎

Lines changed: 67 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -116,19 +116,27 @@ impl IntRange {
116116
"Range: an ascending range must have a positive step",
117117
));
118118
}
119+
// All of the normalization arithmetic below can overflow i64 for
120+
// extreme but individually-valid endpoints — e.g. `0-9223372036854775807`
121+
// makes `count = (end - start) / step + 1` exceed i64::MAX, and a span
122+
// from i64::MIN to i64::MAX overflows the `end - start` subtraction
123+
// itself. These inputs are reachable from untrusted range-expression
124+
// text, so every step uses checked arithmetic and reports a clear
125+
// parse error instead of panicking (debug) or wrapping (release).
119126
if step < 0 {
120127
// Normalize descending to ascending form (matching Python _IntRange)
121-
let count = ((start - end) / (-step)) + 1;
122-
let last = start + (count - 1) * step; // smallest value
128+
let count = Self::checked_count(start, end, -step)?;
129+
// last (smallest value) = start + (count - 1) * step, step < 0
130+
let last = Self::checked_endpoint(start, count, step)?;
123131
Ok(Self {
124132
start: last,
125133
end: start,
126134
step: -step,
127135
})
128136
} else {
129137
// Normalize end to actual last value in the range
130-
let count = (end - start) / step + 1;
131-
let actual_end = start + (count - 1) * step;
138+
let count = Self::checked_count(end, start, step)?;
139+
let actual_end = Self::checked_endpoint(start, count, step)?;
132140
Ok(Self {
133141
start,
134142
end: actual_end,
@@ -137,10 +145,37 @@ impl IntRange {
137145
}
138146
}
139147

148+
/// Number of elements in a normalized range with the given `hi >= lo` and
149+
/// `abs_step > 0`, i.e. `(hi - lo) / abs_step + 1`, using checked
150+
/// arithmetic. Returns a parse error if the span or the `+ 1` overflows.
151+
fn checked_count(hi: i64, lo: i64, abs_step: i64) -> Result<i64, ExpressionError> {
152+
hi.checked_sub(lo)
153+
.and_then(|span| span.checked_div(abs_step))
154+
.and_then(|q| q.checked_add(1))
155+
.ok_or_else(Self::too_large_error)
156+
}
157+
158+
/// Compute `base + (count - 1) * step` with checked arithmetic. Returns a
159+
/// parse error on overflow.
160+
fn checked_endpoint(base: i64, count: i64, step: i64) -> Result<i64, ExpressionError> {
161+
count
162+
.checked_sub(1)
163+
.and_then(|n| n.checked_mul(step))
164+
.and_then(|offset| base.checked_add(offset))
165+
.ok_or_else(Self::too_large_error)
166+
}
167+
168+
fn too_large_error() -> ExpressionError {
169+
ExpressionError::parse_error("Range: the number of elements is too large to represent")
170+
}
171+
140172
/// Number of integers in this range.
141173
pub fn len(&self) -> usize {
142-
// After normalization, start <= end and step > 0 always
143-
((self.end - self.start) / self.step + 1) as usize
174+
// After normalization, start <= end and step > 0 always. The span and
175+
// count were bounds-checked in `new`, so the length fits in i64; clamp
176+
// into usize (which is >= 64 bits on supported targets) defensively.
177+
let count = (self.end - self.start) / self.step + 1;
178+
usize::try_from(count).unwrap_or(usize::MAX)
144179
}
145180

146181
/// Returns `true` if the range contains no elements.
@@ -320,7 +355,16 @@ impl RangeExpr {
320355
let mut merged = vec![ranges[0].clone()];
321356
for r in &ranges[1..] {
322357
let last = merged.last().unwrap();
323-
if last.step == r.step && last.end + r.step == r.start {
358+
// `last.end + r.step` can overflow i64 when `last.end` is near the
359+
// i64 bounds (reachable from untrusted multi-chunk range text). An
360+
// overflowing sum can never equal `r.start` (also an i64), so treat
361+
// overflow as "not adjacent" rather than panicking/wrapping.
362+
let adjacent = last.step == r.step
363+
&& last
364+
.end
365+
.checked_add(r.step)
366+
.is_some_and(|next| next == r.start);
367+
if adjacent {
324368
let new_end = r.end;
325369
let last_start = last.start;
326370
let step = last.step;
@@ -779,6 +823,22 @@ mod tests {
779823
assert!("5-1".parse::<RangeExpr>().is_err());
780824
}
781825

826+
// Regression: individually-valid endpoints whose element count or last
827+
// element overflows i64 must be rejected with a parse error, not panic
828+
// (debug) or wrap (release). Found by the `range_expr` fuzz target; see
829+
// the expr quality report's exploratory finding X1.
830+
#[test]
831+
fn huge_span_does_not_overflow() {
832+
// count = (end - start)/step + 1 overflows i64
833+
assert!("0-9223372036854775807".parse::<RangeExpr>().is_err());
834+
// full i64 span overflows the (end - start) subtraction itself
835+
assert!("-9223372036854775808-9223372036854775807"
836+
.parse::<RangeExpr>()
837+
.is_err());
838+
// descending form with a huge span
839+
assert!("9223372036854775807-0:-1".parse::<RangeExpr>().is_err());
840+
}
841+
782842
// ── slice() tests ──
783843

784844
#[test]

‎fuzz/.gitignore‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
target
2+
corpus
3+
artifacts
4+
coverage

0 commit comments

Comments
 (0)