Skip to content

Commit e9742bd

Browse files
fix(security): bound 8 unbounded file reads — panic-attack Critical findings (Refs #104) (#145)
## Summary Closes the **UnboundedAllocation** slice of the [panic-attack Track-C triage (#104)](#104) — 8 Critical findings, all sharing one root cause. ## Root cause Raw \`std::fs::read_to_string(path)\` with no size cap. A pathological file (\`/dev/zero\`, a corrupted \`.agda\` corpus entry, a maliciously-large \`synonyms.toml\`) could OOM the process. ## Fix Add \`bounded_read_corpus_file\` (sync) to \`src/rust/provers/io.rs\`, mirroring the existing async \`bounded_read_proof_file\`: - Same 64 MiB cap (\`MAX_PROOF_BYTES\`) - Same \`Read::take(N+1)\` discipline (no TOCTOU race vs \`metadata().len()\`) - Same error-not-truncate semantics - 3 new unit tests (small / oversized / missing-file) Swap the 8 call sites to route through the helper: | File | Original line | What it reads | |---|---|---| | \`src/rust/corpus/agda.rs\` | 55 | per-\`*.agda\` corpus entry | | \`src/rust/corpus/coq.rs\` | 49 | per-\`*.v\` corpus entry | | \`src/rust/corpus/mod.rs\` | 225 | \`Corpus::load_json\` snapshot | | \`src/rust/suggest/synonyms.rs\` | 56 | synonym-table TOML | | \`src/rust/suggest/extractor.rs\` | 66 | lemma-extraction probe | | \`src/rust/diagnostics/corpus_monitor.rs\` | 89 | periodic premises.jsonl | | \`src/rust/diagnostics/gnn_training.rs\` | 28 | GNN metrics JSON | ## False-positive documented \`src/rust/corpus/octad.rs:361\` — \`String::with_capacity(bytes.len() * 2)\` is statically bounded. Every caller passes \`Sha256::finalize()\` output (32 bytes ⇒ 64-char hex string). Annotated inline so future static analysis can treat it as proven-bounded; no code change. ## Test plan - [x] \`cargo test --lib provers::io\` — 5/5 pass (2 async + 3 new sync) - [x] \`cargo test --lib corpus\` — 53/53 pass (no regression) - [x] \`cargo test --lib suggest\` — 49/49 pass (no regression) - [x] \`cargo test --lib diagnostics\` — 36/36 pass (no regression) - [x] \`cargo clippy --lib\` — 0 warnings ## Out of scope - PA001/PA007 UnsafeCode/UnsafeFFI findings — separate panic-attack rule families - Findings already suppressed in \`audits/assail-classifications.a2ml\` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ba065e2 commit e9742bd

10 files changed

Lines changed: 73 additions & 21 deletions

File tree

src/rust/corpus/agda.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,7 @@ pub fn ingest(root: &Path) -> Result<Corpus> {
5151
let mut all_names: HashSet<String> = HashSet::new();
5252
for path in &files {
5353
let rel = path.strip_prefix(root).unwrap_or(path).to_path_buf();
54-
let raw =
55-
std::fs::read_to_string(path).with_context(|| format!("read {}", path.display()))?;
54+
let raw = crate::provers::bounded_read_corpus_file(path)?;
5655
let parsed = parse_agda_file(&raw);
5756

5857
let module_idx = corpus.modules.len();

src/rust/corpus/coq.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,7 @@ pub fn ingest(root: &Path) -> Result<Corpus> {
4545
let mut all_names: HashSet<String> = HashSet::new();
4646
for path in &files {
4747
let rel = path.strip_prefix(root).unwrap_or(path).to_path_buf();
48-
let raw =
49-
std::fs::read_to_string(path).with_context(|| format!("read {}", path.display()))?;
48+
let raw = crate::provers::bounded_read_corpus_file(path)?;
5049
let parsed = parse_coq_file(&raw, &rel);
5150

5251
let module_idx = corpus.modules.len();

src/rust/corpus/mod.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,8 +221,7 @@ impl Corpus {
221221

222222
/// Load from JSON.
223223
pub fn load_json(path: &Path) -> Result<Self> {
224-
let s =
225-
std::fs::read_to_string(path).with_context(|| format!("read {}", path.display()))?;
224+
let s = crate::provers::bounded_read_corpus_file(path)?;
226225
let c: Corpus =
227226
serde_json::from_str(&s).with_context(|| format!("parse {}", path.display()))?;
228227
Ok(c)

src/rust/corpus/octad.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,12 @@ fn axiom_usage_from_tensor(t: &DeclTensor) -> super::AxiomUsage {
355355
}
356356
}
357357

358+
/// Hex-encode a byte slice.
359+
///
360+
/// `with_capacity(bytes.len() * 2)` is statically bounded: every caller in
361+
/// this module passes `Sha256::finalize()` (32 bytes ⇒ 64 chars). The
362+
/// argument is `impl AsRef<[u8]>` only for ergonomics, not to admit
363+
/// caller-controlled lengths.
358364
pub(crate) fn to_hex(bytes: impl AsRef<[u8]>) -> String {
359365
use std::fmt::Write;
360366
let bytes = bytes.as_ref();

src/rust/diagnostics/corpus_monitor.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ impl CorpusMonitor {
8686
total_size_bytes += metadata.len();
8787

8888
// Count lines (proofs) in the file
89-
if let Ok(content) = fs::read_to_string(&path) {
89+
if let Ok(content) = crate::provers::bounded_read_corpus_file(&path) {
9090
let line_count = content.lines().count();
9191
total_proofs += line_count;
9292
// Estimate premises as 2-3 per proof on average

src/rust/diagnostics/gnn_training.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
use chrono::{DateTime, Utc};
55
use serde::{Deserialize, Serialize};
6-
use std::fs;
76
use std::path::Path;
87

98
/// Metrics exported from GNN training pipeline.
@@ -25,7 +24,7 @@ pub struct GnnTrainingMetrics {
2524
/// Load GNN training metrics from JSON file
2625
/// Returns None if file doesn't exist or can't be parsed
2726
pub fn load_training_metrics(metrics_path: &Path) -> Option<GnnTrainingMetrics> {
28-
match fs::read_to_string(metrics_path) {
27+
match crate::provers::bounded_read_corpus_file(metrics_path) {
2928
Ok(json_content) => match serde_json::from_str::<GnnTrainingMetrics>(&json_content) {
3029
Ok(metrics) => Some(metrics),
3130
Err(e) => {

src/rust/provers/io.rs

Lines changed: 59 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,24 @@
11
// SPDX-License-Identifier: MPL-2.0
22
// Copyright (c) 2026 Jonathan D.A. Jewell (hyperpolymath) <j.d.a.jewell@open.ac.uk>
33

4-
//! Shared IO helpers for prover backends.
4+
//! Shared IO helpers for prover backends and corpus loaders.
55
//!
6-
//! All prover backends read user-supplied proof files from disk. Bare
7-
//! `tokio::fs::read_to_string(&path)` has no upper bound — a malicious
8-
//! or pathological caller could pass `/dev/zero` or a multi-GB file
9-
//! and OOM the process. This module exposes
10-
//! [`bounded_read_proof_file`] which caps the read at
11-
//! [`MAX_PROOF_BYTES`] and reports an error rather than truncating.
6+
//! All prover backends, corpus loaders, suggest extractors, and the
7+
//! diagnostics monitors read user-supplied / on-disk files. Bare
8+
//! `tokio::fs::read_to_string(&path)` (or its `std::fs` sync sibling)
9+
//! has no upper bound — a malicious or pathological caller could pass
10+
//! `/dev/zero` or a multi-GB file and OOM the process. This module
11+
//! exposes [`bounded_read_proof_file`] (async) and
12+
//! [`bounded_read_corpus_file`] (sync), which cap the read at
13+
//! [`MAX_PROOF_BYTES`] and report an error rather than truncating.
1214
//!
1315
//! Cap chosen at 64 MiB: large real-world proof corpora (Mizar MML
1416
//! per-article files, Coq `.v` libraries, Isabelle theory chains)
1517
//! all sit well below this limit with margin; anything larger is
1618
//! almost certainly pathological input.
1719
1820
use anyhow::{Context, Result};
21+
use std::io::Read;
1922
use std::path::Path;
2023
use tokio::io::AsyncReadExt;
2124

@@ -47,6 +50,30 @@ pub async fn bounded_read_proof_file<P: AsRef<Path>>(path: P) -> Result<String>
4750
Ok(buf)
4851
}
4952

53+
/// Read a UTF-8 corpus / config / diagnostics file synchronously,
54+
/// bounded at [`MAX_PROOF_BYTES`].
55+
///
56+
/// Same `take(N+1)` discipline as [`bounded_read_proof_file`]; differs
57+
/// only in that callers (corpus loaders, suggest extractors, diagnostic
58+
/// monitors) live on sync code paths and cannot await a tokio future.
59+
pub fn bounded_read_corpus_file<P: AsRef<Path>>(path: P) -> Result<String> {
60+
let path = path.as_ref();
61+
let file = std::fs::File::open(path)
62+
.with_context(|| format!("opening corpus file {}", path.display()))?;
63+
let mut buf = String::new();
64+
file.take(MAX_PROOF_BYTES + 1)
65+
.read_to_string(&mut buf)
66+
.with_context(|| format!("reading corpus file {}", path.display()))?;
67+
if buf.len() as u64 > MAX_PROOF_BYTES {
68+
return Err(anyhow::anyhow!(
69+
"corpus file {} exceeds {} byte cap",
70+
path.display(),
71+
MAX_PROOF_BYTES
72+
));
73+
}
74+
Ok(buf)
75+
}
76+
5077
#[cfg(test)]
5178
mod tests {
5279
use super::*;
@@ -71,4 +98,29 @@ mod tests {
7198
let err = bounded_read_proof_file(&path).await.unwrap_err();
7299
assert!(err.to_string().contains("exceeds"));
73100
}
101+
102+
#[test]
103+
fn sync_small_file_reads_fully() {
104+
let dir = tempfile::tempdir().unwrap();
105+
let path = dir.path().join("small.corpus");
106+
std::fs::write(&path, b"corpus entry: lemma_foo\n").unwrap();
107+
let s = bounded_read_corpus_file(&path).unwrap();
108+
assert!(s.contains("lemma_foo"));
109+
}
110+
111+
#[test]
112+
fn sync_oversized_file_errors() {
113+
let dir = tempfile::tempdir().unwrap();
114+
let path = dir.path().join("big.corpus");
115+
let big = vec![b'x'; (MAX_PROOF_BYTES + 1024) as usize];
116+
std::fs::write(&path, &big).unwrap();
117+
let err = bounded_read_corpus_file(&path).unwrap_err();
118+
assert!(err.to_string().contains("exceeds"));
119+
}
120+
121+
#[test]
122+
fn sync_missing_file_errors_clean() {
123+
let err = bounded_read_corpus_file("/nonexistent/path/that/does/not/exist").unwrap_err();
124+
assert!(err.to_string().contains("opening corpus file"));
125+
}
74126
}

src/rust/provers/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ pub mod outcome;
1717
pub use outcome::{classify_anyhow_error, ProverOutcome};
1818

1919
pub mod io;
20-
pub use io::{bounded_read_proof_file, MAX_PROOF_BYTES};
20+
pub use io::{bounded_read_corpus_file, bounded_read_proof_file, MAX_PROOF_BYTES};
2121

2222
pub mod abc;
2323
pub mod abella;

src/rust/suggest/extractor.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,7 @@ impl Probe {
6262

6363
/// Extract named lemma from `path` for the given prover.
6464
pub fn extract(prover: ProverKind, path: &Path, lemma_name: &str) -> Result<Probe> {
65-
let content =
66-
std::fs::read_to_string(path).with_context(|| format!("Cannot read {}", path.display()))?;
65+
let content = crate::provers::bounded_read_corpus_file(path)?;
6766
match prover {
6867
ProverKind::Isabelle => extract_isabelle(&content, lemma_name),
6968
ProverKind::Coq => extract_coq(&content, lemma_name),

src/rust/suggest/synonyms.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,7 @@ impl SynonymTable {
5353
if !path.exists() {
5454
return Ok(Self::default());
5555
}
56-
let raw = std::fs::read_to_string(&path)
57-
.with_context(|| format!("Failed to read synonym table {}", path.display()))?;
56+
let raw = crate::provers::bounded_read_corpus_file(&path)?;
5857
let parsed: RawTable =
5958
toml::from_str(&raw).with_context(|| format!("Failed to parse {}", path.display()))?;
6059
Ok(Self::from_entries(parsed.synonyms))

0 commit comments

Comments
 (0)