Skip to content

Commit 0f6ea94

Browse files
fix: bound three unbounded file reads (resolves self-scan Critical) (#51)
## Summary Self-scanning panic-attack with the latest release surfaced a **Critical UnboundedAllocation** in `attestation/mod.rs::verify_attestation_file` — and, on closer inspection, two sibling sites the analyzer's per-file heuristic glossed over. All three were the last residual violations of the codebase-wide invariant that every file read must be capacity-bounded. ## Discovery path Ran the freshly-built release of panic-attack on its own source tree: ``` ./target/release/panic-attack assail . --output /tmp/panic-self-scan.json ``` Among the 12 findings, one Critical pointed at `src/attestation/mod.rs` — the heuristic correctly fired because that file has **zero** other `.take(...)` calls. The other two sites (`src/assemblyline.rs:120`, `src/assail/mod.rs:230`) live in files that DO use `.take(...)` elsewhere, so the analyzer's per-file all-or-nothing classifier flagged them as bounded overall. Manual triage caught those — separate concern, doesn't need analyzer changes here. ## Fixes All three follow the established pattern of every other read in the codebase (`File::open` + `.take(LIMIT)` + `.read_to_string`). | Site | Cap | Rationale | |---|---|---| | `src/attestation/mod.rs::verify_attestation_file` | 16 MiB | JSON envelopes embed three hashes plus a small signature; legitimate envelopes < 1 MiB. | | `src/assemblyline.rs::load_cache_file` | 256 MiB | Fingerprint caches grow with estate size; large enough for multi-thousand-repo rollups. | | `src/assail/mod.rs::load_user_classifications` | 4 MiB | Hand-edited audit a2ml registries are small. | ## Verification - [x] `cargo build --bin panic-attack --features signing,http` — clean - [x] `cargo clippy --all-targets --features signing,http -- -D warnings` — clean - [x] `cargo test --bin panic-attack --features signing,http` — 236 passed, 0 failed - [x] `cargo fmt --check` — clean - [x] **Self-scan before**: 12 findings (1 Critical UnboundedAllocation) - [x] **Self-scan after**: 11 findings (Critical resolved; residuals are intentional — `examples/vulnerable_program.rs` unsafe blocks, test unwraps, etc.) - [x] GPG-signed commit ## Out of scope The analyzer's per-file heuristic for `read_to_string` boundedness is correct-by-default (files that use `.take(...)` are *probably* bounded throughout), but the two sites it missed argue for a per-call check. That's a detector improvement, separate from these data-path fixes; would file as a follow-up if the team thinks it's worth it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ea02d50 commit 0f6ea94

3 files changed

Lines changed: 43 additions & 7 deletions

File tree

src/assail/mod.rs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -225,11 +225,25 @@ pub fn load_user_classifications(project_root: &Path) -> Vec<UserClassification>
225225
.join("assail-classifications.a2ml"),
226226
project_root.join(".panic-attack-classifications.a2ml"),
227227
];
228+
// User-classification a2ml files are hand-edited audit registries. A
229+
// legitimate one rarely exceeds a few dozen KiB. Capping at 4 MiB
230+
// stops a malicious or accidental input from exhausting memory during
231+
// a multi-thousand-repo mass-panic sweep.
232+
use std::io::Read;
233+
const CLASSIFICATIONS_FILE_READ_LIMIT: u64 = 4 * 1024 * 1024;
234+
228235
let mut content = String::new();
229236
for p in &candidate_paths {
230-
if let Ok(c) = fs::read_to_string(p) {
231-
content = c;
232-
break;
237+
if let Ok(mut f) = fs::File::open(p) {
238+
let mut buf = String::new();
239+
if (&mut f)
240+
.take(CLASSIFICATIONS_FILE_READ_LIMIT)
241+
.read_to_string(&mut buf)
242+
.is_ok()
243+
{
244+
content = buf;
245+
break;
246+
}
233247
}
234248
}
235249
if content.is_empty() {

src/assemblyline.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,20 @@ impl FingerprintCache {
115115
Ok(())
116116
}
117117

118-
/// Load fingerprint cache from a standalone cache JSON file
118+
/// Load fingerprint cache from a standalone cache JSON file.
119+
///
120+
/// Bounded at 256 MiB — fingerprint caches grow with the size of the
121+
/// estate being scanned, but even an aggressive multi-thousand-repo
122+
/// rollup should land well under this cap. A larger file is almost
123+
/// certainly corrupted or hostile.
119124
pub fn load_cache_file(path: &Path) -> Result<Self> {
120-
let content = fs::read_to_string(path)?;
125+
use std::io::Read;
126+
const CACHE_FILE_READ_LIMIT: u64 = 256 * 1024 * 1024;
127+
128+
let mut content = String::new();
129+
fs::File::open(path)?
130+
.take(CACHE_FILE_READ_LIMIT)
131+
.read_to_string(&mut content)?;
121132
let cache: Self = serde_json::from_str(&content)?;
122133
Ok(cache)
123134
}

src/attestation/mod.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,19 @@ pub enum VerifyResult {
6767
/// with a list of failure reasons otherwise.
6868
pub fn verify_attestation_file(path: &std::path::Path) -> anyhow::Result<VerifyResult> {
6969
use sha2::{Digest, Sha256};
70-
71-
let content = std::fs::read_to_string(path)
70+
use std::io::Read;
71+
72+
// Attestation envelopes are JSON. They embed three hashes plus a small
73+
// signature; legitimate envelopes are well under 1 MiB. Capping at
74+
// 16 MiB stops a malicious or accidental input from exhausting memory
75+
// before verification can even begin.
76+
const ATTESTATION_FILE_READ_LIMIT: u64 = 16 * 1024 * 1024;
77+
78+
let mut content = String::new();
79+
std::fs::File::open(path)
80+
.map_err(|e| anyhow::anyhow!("opening {}: {}", path.display(), e))?
81+
.take(ATTESTATION_FILE_READ_LIMIT)
82+
.read_to_string(&mut content)
7283
.map_err(|e| anyhow::anyhow!("reading {}: {}", path.display(), e))?;
7384

7485
let envelope: A2mlEnvelope = serde_json::from_str(&content)

0 commit comments

Comments
 (0)