From ac61953f60b52340e765b0cd7f1e9bac717d3bdc Mon Sep 17 00:00:00 2001 From: bettercallcaleb <121191527+bettercallcaleb@users.noreply.github.com> Date: Wed, 4 Nov 2026 14:11:20 -0500 Subject: [PATCH] fix(gguf): handle malformed metadata without panicking Signed-off-by: bettercallcaleb <121191527+bettercallcaleb@users.noreply.github.com> --- src/gguf.rs | 17 ++++++++++------- tests/gguf.rs | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 7 deletions(-) diff --git a/src/gguf.rs b/src/gguf.rs index 586c061d..431d4afb 100644 --- a/src/gguf.rs +++ b/src/gguf.rs @@ -44,14 +44,15 @@ struct Cur<'a> { b: &'a [u8], p: usize } impl<'a> Cur<'a> { fn take(&mut self, n: usize) -> anyhow::Result<&'a [u8]> { - let s = self.b.get(self.p..self.p + n).context("GGUF truncated")?; - self.p += n; + let end = self.p.checked_add(n).context("GGUF truncated")?; + let s = self.b.get(self.p..end).context("GGUF truncated")?; + self.p = end; Ok(s) } fn u32(&mut self) -> anyhow::Result { Ok(u32::from_le_bytes(self.take(4)?.try_into()?)) } fn u64(&mut self) -> anyhow::Result { Ok(u64::from_le_bytes(self.take(8)?.try_into()?)) } fn str(&mut self) -> anyhow::Result { - let n = self.u64()? as usize; + let n = usize::try_from(self.u64()?).context("GGUF string length out of range")?; Ok(String::from_utf8_lossy(self.take(n)?).into_owned()) } fn val(&mut self, t: u32) -> anyhow::Result { @@ -66,8 +67,9 @@ impl<'a> Cur<'a> { 7 => Value::Bool(self.take(1)?[0] != 0), 8 => Value::Str(self.str()?), 9 => { - let (et, n) = (self.u32()?, self.u64()? as usize); - let mut a = Vec::with_capacity(n); + let (et, n) = (self.u32()?, usize::try_from(self.u64()?).context("GGUF array length out of range")?); + if n > self.b.len().saturating_sub(self.p) { bail!("GGUF array length exceeds remaining data"); } + let mut a = Vec::with_capacity(n.min(1024)); for _ in 0..n { a.push(self.val(et)?); } Value::Arr(a) } @@ -102,8 +104,9 @@ impl Gguf { let (ty, offset) = (c.u32()?, c.u64()?); tensors.insert(name.clone(), TensorInfo { name, dims, ty, offset }); } - let align = meta.get("general.alignment").and_then(Value::as_u64).unwrap_or(32) as usize; - let data_offset = c.p.div_ceil(align) * align; + let align = usize::try_from(meta.get("general.alignment").and_then(Value::as_u64).unwrap_or(32)) + .context("GGUF alignment out of range")?; + let data_offset = c.p.checked_next_multiple_of(align).context("invalid GGUF alignment")?; // A cut-off download keeps a valid header; catch it here, not as a slice panic in `bytes`. for t in tensors.values() { let Ok(ty) = GgmlType::from_u32(t.ty) else { continue }; // Weights::validate reports it diff --git a/tests/gguf.rs b/tests/gguf.rs index 0968dc8a..22f776a3 100644 --- a/tests/gguf.rs +++ b/tests/gguf.rs @@ -151,3 +151,50 @@ fn rejects_bad_ssm_dims() { let bad = qwen3next_syn("ssm-ba", &kv(2), &[emb, ("blk.0.ssm_ba.weight", [8, 6])]); assert!(Weights::new(&bad).is_err()); } + +#[test] +fn malformed_metadata_must_not_panic() { + let mut fixtures = Vec::new(); + let mut zero_alignment = Vec::new(); + zero_alignment.extend(b"GGUF"); + zero_alignment.extend(3u32.to_le_bytes()); + zero_alignment.extend(0u64.to_le_bytes()); + zero_alignment.extend(1u64.to_le_bytes()); + w_str(&mut zero_alignment, "general.alignment"); + zero_alignment.extend(4u32.to_le_bytes()); + zero_alignment.extend(0u32.to_le_bytes()); + fixtures.push(("zero-alignment", zero_alignment)); + + let mut giant_key = Vec::new(); + giant_key.extend(b"GGUF"); + giant_key.extend(3u32.to_le_bytes()); + giant_key.extend(0u64.to_le_bytes()); + giant_key.extend(1u64.to_le_bytes()); + giant_key.extend(u64::MAX.to_le_bytes()); + fixtures.push(("oversized-key", giant_key)); + + let mut giant_array = Vec::new(); + giant_array.extend(b"GGUF"); + giant_array.extend(3u32.to_le_bytes()); + giant_array.extend(0u64.to_le_bytes()); + giant_array.extend(1u64.to_le_bytes()); + w_str(&mut giant_array, "array"); + giant_array.extend(9u32.to_le_bytes()); + giant_array.extend(0u32.to_le_bytes()); + giant_array.extend(u64::MAX.to_le_bytes()); + fixtures.push(("oversized-array", giant_array)); + + let mut failures = Vec::new(); + for (label, bytes) in fixtures { + let path = std::env::temp_dir().join(format!("rustingstrata-{}-{label}.gguf", std::process::id())); + std::fs::write(&path, bytes).unwrap(); + let outcome = std::panic::catch_unwind(|| Gguf::open(&path)); + std::fs::remove_file(&path).ok(); + match outcome { + Err(_) => failures.push(format!("parser panicked on {label}")), + Ok(Ok(_)) => failures.push(format!("parser accepted malformed {label}")), + Ok(Err(_)) => {} + } + } + assert!(failures.is_empty(), "{}", failures.join("; ")); +}