Skip to content

Commit fe86444

Browse files
hyperpolymathclaude
andcommitted
fix(parser): T3 totality — no panic on malformed array-size / region-size arithmetic
The .twasm parser had three panic-on-malformed-input sites (the T3 gap in the codegen→verified-wasm assurance ladder): - parse_array_size_expr: out-of-bounds index when the operator byte was missing after an operand; and *,+,-,/ that overflowed, underflowed, or divided by zero on a crafted size expression. - compute_region_byte_size: size += field_size * cardinality overflowed u32. - resolve_field: offset += size * cardinality overflowed u32. All three now degrade gracefully: the array-size expression and region size return a parse error (the latter promoted to Result<u32,String>), and resolve_field returns None (caller falls back to the stub). Checked arithmetic throughout; u32::try_from instead of a silent `as u32` truncation. Tests (src/parser.rs totality_tests): div-by-zero, u64/u32 overflow, underflow, truncated-operator, region-size overflow, and offset overflow each rejected without panicking, each paired with a well-formed control. No regression. (The remaining clippy collapsible_match in typed-wasm-verify's MemorySection arm is pre-existing and unrelated — for the separate lint chore PR.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 9988d8b commit fe86444

1 file changed

Lines changed: 135 additions & 17 deletions

File tree

crates/typed-wasm-codegen/src/parser.rs

Lines changed: 135 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ impl<'a> Parser<'a> {
256256
// For now, we don't calculate byte_size - we'll need to compute it
257257
// based on field types. For the paint-type schemas, we can use
258258
// the hardcoded values.
259-
let byte_size = self.compute_region_byte_size(&fields);
259+
let byte_size = self.compute_region_byte_size(&fields)?;
260260

261261
let region_index = self.regions.len();
262262
self.region_map.insert(name.clone(), region_index);
@@ -270,18 +270,25 @@ impl<'a> Parser<'a> {
270270
Ok(())
271271
}
272272

273-
fn compute_region_byte_size(&self, fields: &[Field]) -> u32 {
273+
fn compute_region_byte_size(&self, fields: &[Field]) -> Result<u32, String> {
274274
let mut size = 0u32;
275275
for field in fields {
276276
let field_size = match field.ty {
277277
FieldTy::Scalar(s) => scalar_byte_size(&s),
278278
FieldTy::Ptr { .. } => 4, // Pointer is 4 bytes in wasm
279279
};
280-
size += field_size * field.cardinality;
280+
// Checked arithmetic: a pathological schema (a field, or running
281+
// total, exceeding u32 bytes) is a parse error, never a panic.
282+
let contribution = field_size
283+
.checked_mul(field.cardinality)
284+
.ok_or_else(|| format!("field '{}' size overflows u32", field.name))?;
285+
size = size
286+
.checked_add(contribution)
287+
.ok_or_else(|| "region byte size overflows u32".to_string())?;
281288
}
282289
// Add padding if needed for alignment
283290
// For simplicity, we'll let the caller handle alignment
284-
size
291+
Ok(size)
285292
}
286293

287294
fn parse_field_type(&mut self) -> Result<(FieldTy, u32), String> {
@@ -453,26 +460,40 @@ impl<'a> Parser<'a> {
453460
let left: u64 = self.parse_number()?;
454461
self.skip_whitespace();
455462

456-
// Parse operator
457-
let op = self.src.as_bytes()[self.pos];
463+
// Parse operator. Panic-safe index: truncated input after the left
464+
// operand yields an Err, never an out-of-bounds index panic.
465+
let Some(&op) = self.src.as_bytes().get(self.pos) else {
466+
return Err(
467+
"Expected operator in array size expression, found end of input".to_string(),
468+
);
469+
};
458470
if op != b'*' && op != b'+' && op != b'-' && op != b'/' {
459471
return Err(format!("Expected operator, found '{}'", op as char));
460472
}
461473
self.pos += 1;
462-
474+
463475
self.skip_whitespace();
464476
let right: u64 = self.parse_number()?;
465-
466-
// Evaluate the expression
467-
let result = match op {
468-
b'*' => left * right,
469-
b'+' => left + right,
470-
b'-' => left - right,
471-
b'/' => left / right,
477+
478+
// Evaluate with checked arithmetic: a malformed expression (overflow,
479+
// division by zero, underflow) is a parse error, never a panic.
480+
let result: u64 = match op {
481+
b'*' => left
482+
.checked_mul(right)
483+
.ok_or_else(|| format!("array size expression overflows: {left} * {right}"))?,
484+
b'+' => left
485+
.checked_add(right)
486+
.ok_or_else(|| format!("array size expression overflows: {left} + {right}"))?,
487+
b'-' => left
488+
.checked_sub(right)
489+
.ok_or_else(|| format!("array size expression underflows: {left} - {right}"))?,
490+
b'/' => left
491+
.checked_div(right)
492+
.ok_or_else(|| format!("array size expression divides by zero: {left} / {right}"))?,
472493
_ => return Err(format!("Unknown operator: {}", op as char)),
473494
};
474-
475-
Ok(result as u32)
495+
496+
u32::try_from(result).map_err(|_| format!("array size {result} does not fit in u32"))
476497
}
477498

478499
fn peek_char(&mut self, c: char) -> bool {
@@ -985,7 +1006,10 @@ impl<'a> Parser<'a> {
9851006
FieldTy::Scalar(s) => scalar_byte_size(&s),
9861007
FieldTy::Ptr { .. } => 4,
9871008
};
988-
offset += size * f.cardinality;
1009+
// Checked: a region whose fields overrun u32 cannot yield a sane
1010+
// offset, so treat it as unresolvable (caller falls back to the
1011+
// stub) rather than panicking on overflow.
1012+
offset = offset.checked_add(size.checked_mul(f.cardinality)?)?;
9891013
}
9901014
None
9911015
}
@@ -1266,3 +1290,97 @@ fn scalar_store_op(s: &Scalar, offset: u64) -> crate::Op {
12661290
Scalar::F64 => F64Store { offset },
12671291
}
12681292
}
1293+
1294+
#[cfg(test)]
1295+
mod totality_tests {
1296+
use super::*;
1297+
1298+
// T3 — parser totality: the three previously-panicking arithmetic sites
1299+
// (array-size expression, region byte size, field offset) now degrade to
1300+
// an error / `None` on malformed or pathological input, never a panic.
1301+
// Each negative test is paired with a well-formed control so it is the
1302+
// fault being rejected, not the path being broken.
1303+
1304+
#[test]
1305+
fn array_size_div_by_zero_is_err_not_panic() {
1306+
let mut p = Parser::new("4 / 0");
1307+
assert!(p.parse_array_size_expr().is_err());
1308+
}
1309+
1310+
#[test]
1311+
fn array_size_overflow_is_err_not_panic() {
1312+
// 1e10 * 1e10 = 1e20 overflows u64 -> checked_mul None -> Err.
1313+
let mut p = Parser::new("10000000000 * 10000000000");
1314+
assert!(p.parse_array_size_expr().is_err());
1315+
}
1316+
1317+
#[test]
1318+
fn array_size_u32_overflow_is_err_not_panic() {
1319+
// Fits in u64 but not u32 -> try_from Err, not a silent truncation.
1320+
let mut p = Parser::new("100000 * 100000");
1321+
assert!(p.parse_array_size_expr().is_err());
1322+
}
1323+
1324+
#[test]
1325+
fn array_size_underflow_is_err_not_panic() {
1326+
let mut p = Parser::new("1 - 2");
1327+
assert!(p.parse_array_size_expr().is_err());
1328+
}
1329+
1330+
#[test]
1331+
fn array_size_truncated_after_operand_is_err_not_panic() {
1332+
// No operator byte after the operand: was an out-of-bounds index panic.
1333+
let mut p = Parser::new("5");
1334+
assert!(p.parse_array_size_expr().is_err());
1335+
}
1336+
1337+
#[test]
1338+
fn array_size_well_formed_still_evaluates() {
1339+
let mut p = Parser::new("64 * 64");
1340+
assert_eq!(p.parse_array_size_expr(), Ok(4096));
1341+
}
1342+
1343+
#[test]
1344+
fn region_byte_size_overflow_is_err_not_panic() {
1345+
let p = Parser::new("");
1346+
let fields = vec![
1347+
Field::array("a", Scalar::U8, 3_000_000_000),
1348+
Field::array("b", Scalar::U8, 3_000_000_000), // sum 6e9 > u32::MAX
1349+
];
1350+
assert!(p.compute_region_byte_size(&fields).is_err());
1351+
}
1352+
1353+
#[test]
1354+
fn region_byte_size_normal_is_ok() {
1355+
let p = Parser::new("");
1356+
let fields = vec![Field::scalar("a", Scalar::I32), Field::scalar("b", Scalar::U8)];
1357+
assert_eq!(p.compute_region_byte_size(&fields), Ok(5));
1358+
}
1359+
1360+
#[test]
1361+
fn resolve_field_offset_overflow_is_none_not_panic() {
1362+
let mut p = Parser::new("");
1363+
p.regions.push(Region {
1364+
name: "R".into(),
1365+
fields: vec![
1366+
Field::array("pad", Scalar::U8, 4_000_000_000),
1367+
Field::array("pad2", Scalar::U8, 4_000_000_000), // offset 8e9 > u32::MAX
1368+
Field::scalar("target", Scalar::I32),
1369+
],
1370+
byte_size: 0,
1371+
});
1372+
assert!(p.resolve_field(0, "target").is_none());
1373+
}
1374+
1375+
#[test]
1376+
fn resolve_field_normal_offsets_resolve() {
1377+
let mut p = Parser::new("");
1378+
p.regions.push(Region {
1379+
name: "R".into(),
1380+
fields: vec![Field::scalar("a", Scalar::I32), Field::scalar("b", Scalar::U8)],
1381+
byte_size: 5,
1382+
});
1383+
// 'b' sits at offset 4 (after the i32).
1384+
assert!(matches!(p.resolve_field(0, "b"), Some((1, 4, Scalar::U8))));
1385+
}
1386+
}

0 commit comments

Comments
 (0)