Fix H1: bound untrusted array counts; resolve L1 (review #006)
- Replace the three Vec::with_capacity(count) sites in materialize.rs with Vec::new() — validate_bytes on an adversarial count no longer OOM-aborts the process (AGENTS.md §3). - New compile-time caps in schema.rs: MAX_ARRAY_ELEMENTS (2^16, enforced at BastArray::parse — the choke point every consumer inherits, bounds the walkers' per-element entry loops) and MAX_ARRAY_BYTES (2^26, enforced per walker against the mode-specific stride: compile_array, walk_array, compute_array_field). - L1: fixed_composite_size/fixed_plan_size now return Result<Option<usize>>; unwrap_or_default() gone, overflow is a clean Schema error instead of silent stride-0. - Zero-progress guard: stride-0 arrays whose elements consume 0 bytes (legal empty-struct elements) now error in plan_walk_variable_array_ size and materialize_array_packed instead of looping count times. - Tests: 8 new (parse/build/compile rejections, cap boundary, short-buffer clean error) + array_count_large_u64_parses_on_64bit rewritten to assert the new cap rejection. In-tree tests assert only the safe (compile-time) half per review #006's Methodology warning. - Review #006 updated: H1/L1 resolution blocks, new finding N2 (unbounded align annotations, found while re-deriving the cap arithmetic), resolution log, recommended order. Verified: 482 tests green (405+17+34+14+12, 2 pre-existing ignored), clippy -D warnings clean, wasm32-unknown-unknown build green.
This commit is contained in:
1 parent
27be01af93
commit
2d166f567b
8 files changed
+453
-49
No files matched your search
@@ -1,5 +1,5 @@
|
||||
---
|
||||
status: open
|
||||
status: in-progress (H1, L1 resolved 2026-09-02)
|
||||
last_updated: 2026-09-02
|
||||
reviewed_artifacts:
|
||||
- src/read_plan.rs
|
||||
@@ -93,21 +93,31 @@ sub-types, `fingerprint()` methods; `Hash` on `Endian`/
|
||||
|
||||
## Summary Statistics
|
||||
|
||||
| Severity | Count |
|
||||
|----------|------:|
|
||||
| High | 3 (H1, H2, H3) |
|
||||
| Medium | 4 (M1, M2, M3, M4) |
|
||||
| Low | 6 (L1–L6) |
|
||||
| Nit | 1 (N1) |
|
||||
| Severity | Count | Status |
|
||||
|----------|------:|--------|
|
||||
| High | 3 (H1, H2, H3) | H1 resolved 2026-09-02 |
|
||||
| Medium | 4 (M1, M2, M3, M4) | open |
|
||||
| Low | 6 (L1–L6) | L1 resolved 2026-09-02 |
|
||||
| Nit | 2 (N1, N2) | open |
|
||||
|
||||
The three Highs are adversarial-input crashes (H1, H2) and a
|
||||
cross-consumer wire-layout convention gap (H3) — all three are the
|
||||
exact class of problem AGENTS.md §3 exists for, given that `alkcall`
|
||||
feeds schemas from arbitrary internet peers into this engine. H1 should
|
||||
block publishing 0.3.0 to crates.io until fixed. None of the three is a
|
||||
0.2.0 regression in the strict sense (details per finding), but two of
|
||||
feeds schemas from arbitrary internet peers into this engine. H1
|
||||
blocked publishing 0.3.0 to crates.io until fixed (now fixed, see the
|
||||
resolution note on the finding). None of the three is a 0.2.0
|
||||
regression in the strict sense (details per finding), but two of
|
||||
them became materially easier to hit with the 0.3.0 surface.
|
||||
|
||||
**Resolution log:**
|
||||
|
||||
- **H1 + L1 (2026-09-02):** resolved in one commit — see the
|
||||
"Resolution (2026-09-02)" block at the end of the H1 finding and the
|
||||
L1 finding. 482 tests green (405 crate + 67 integration + 2
|
||||
pre-existing ignored), clippy `-D warnings` clean, wasm build green.
|
||||
The fix session also surfaced a new Nit (N2, unbounded `align`
|
||||
annotations) — added below.
|
||||
|
||||
---
|
||||
|
||||
## Findings
|
||||
@@ -177,6 +187,65 @@ Add tests: compile+validate with `count: usize::MAX` (expect clean
|
||||
`Err`), and a moderate count against a short buffer (expect clean
|
||||
`Err`).
|
||||
|
||||
**Resolution (2026-09-02) — both layers, plus two gaps the original fix
|
||||
text missed:**
|
||||
|
||||
1. **Abort removal (fix layer 1):** all three `Vec::with_capacity(count)`
|
||||
sites are now `Vec::new()` + push loop (`materialize_plan_array`,
|
||||
`materialize_array_packed`, `materialize_array_aligned`). The
|
||||
`validate_bytes` OOM-abort is gone: with an oversized count the
|
||||
per-element walk now errors on buffer bounds instead of allocating.
|
||||
|
||||
2. **Compile-time caps (fix layer 2), at two choke points** — the
|
||||
review's "documented cap" suggestion landed as two constants in
|
||||
`schema.rs`:
|
||||
- `MAX_ARRAY_ELEMENTS = 2^16`, enforced in `BastArray::parse`
|
||||
(`bast.rs`) — the single point every consumer inherits
|
||||
(`BastDoc::new` is the only construction path, and all five
|
||||
consumers — engine, ReadPlan, LayoutBuilder, OffsetMap, and
|
||||
standalone `BastDoc` users — parse through it). This cap is what
|
||||
bounds the walkers' *per-element push loops*: with a product cap
|
||||
alone, 256 MiB of u8 elements would still mean 268M loop
|
||||
iterations accumulating ~20 GB of offset-map/layout entries at
|
||||
compile time. The original fix text ("bound `count × element
|
||||
size`") missed this; a product cap bounds wire size, not walker
|
||||
memory.
|
||||
- `MAX_ARRAY_BYTES = 2^26`, enforced per walker against the
|
||||
mode-appropriate stride: `compile_array` (ReadPlan, schema error),
|
||||
`walk_array` (LayoutBuilder, offset error — defense-in-depth
|
||||
there: packed mode has no alignment, so max fixed element 8 ×
|
||||
count 2^16 = 2^19 << 2^26 can't reach the cap), `compute_array_field`
|
||||
(OffsetMap, offset error — *reachable* there, via an
|
||||
align-driven stride, see below).
|
||||
- L1 folded in: `fixed_composite_size`/`fixed_plan_size` now return
|
||||
`Result<Option<usize>>`; `unwrap_or_default()` is gone, and the
|
||||
checked-mul overflow arm is a clean `Schema` error instead of a
|
||||
silent stride-0.
|
||||
|
||||
3. **Zero-progress runtime guards** — a gap neither the finding nor the
|
||||
fix text covered: a stride-0 array whose elements consume 0 bytes
|
||||
(empty-struct elements are legal; `stride 0` means per-element
|
||||
sequential walking) loops `count` times with no buffer bound even
|
||||
after both caps. `plan_walk_variable_array_size` (reader) and
|
||||
`materialize_array_packed` (materializer) now error with "array
|
||||
element consumed 0 bytes" when an element makes no wire progress.
|
||||
The aligned materializer needs no guard: aligned array elements are
|
||||
always fixed-size kinds (`type_size() >= 1`).
|
||||
|
||||
Tests (8 new + 1 rewritten): parse-cap rejection (bast), byte-cap
|
||||
rejection via ReadPlan/LayoutBuilder/OffsetMap, count-at-cap
|
||||
acceptance, packed 2^16-array build acceptance (layout_builder,
|
||||
documents why the byte cap is unreachable in packed mode), huge-count
|
||||
short-buffer clean access error, and the old
|
||||
`array_count_large_u64_parses_on_64bit` (which asserted the *old*
|
||||
unbounded-parse behavior as a feature) rewritten as
|
||||
`array_count_large_u64_rejected_by_compile_cap`. No OOM reproducers
|
||||
in-tree (per the Methodology warning); the in-tree tests assert only
|
||||
the compile-time rejections — the safe half.
|
||||
|
||||
Verified: 482 tests green, clippy `-D warnings` clean,
|
||||
`cargo build --target wasm32-unknown-unknown --release` green.
|
||||
|
||||
### H2. Cyclic `$ref` stack-overflows `OffsetMap::compute` / `LayoutBuilder::new` / `materialize_aligned` when driven standalone
|
||||
|
||||
**Files**: `src/offset_map.rs:123` (`compute`),
|
||||
@@ -492,6 +561,8 @@ scheduling a standalone coverage sweep.
|
||||
|
||||
**File**: `src/read_plan.rs:547`
|
||||
|
||||
**Status**: resolved 2026-09-02 (with H1).
|
||||
|
||||
**Problem**: `let element_stride = fixed_composite_size(&element)
|
||||
.unwrap_or_default();` — `fixed_composite_size` returns `None` both
|
||||
for genuine variable-length elements (correct → stride 0) and for
|
||||
@@ -504,6 +575,13 @@ call site. Prefer making `compile_array` return `Err` on the overflow
|
||||
arm (a small refactor of `fixed_composite_size` to return a
|
||||
`Result<Option<usize>, …>` or to take the cap from H1's fix).
|
||||
|
||||
**Resolution (2026-09-02):** `fixed_composite_size` and
|
||||
`fixed_plan_size` now return `Result<Option<usize>, AlkTypeError>`;
|
||||
`None` means variable-length only, and overflow arms return clean
|
||||
`Schema` errors. `compile_array` uses `fixed_composite_size(&element)?
|
||||
.unwrap_or(0)` — the conflation is gone. See the H1 resolution block
|
||||
for the full change description.
|
||||
|
||||
### L2. `ReadPlan` construction carries a temporary `Value::Null` schema placeholder
|
||||
|
||||
**Files**: `src/read_plan.rs:295,460-461,601` (three
|
||||
@@ -609,6 +687,34 @@ or document the divergence; the meta-schema does not constrain the
|
||||
discriminator field's kind, so both code paths are reachable from the
|
||||
same schema.
|
||||
|
||||
### N2. `align` annotations are unbounded — no crash, but absurd layouts and the reachable path to H1's byte cap
|
||||
|
||||
**Files**: `src/bast_meta.rs:64,79` (`"align": { "type": "integer",
|
||||
"minimum": 1 }`, no maximum), `src/offset_map.rs` (`align_up`/
|
||||
`round_up`), `src/bast.rs:926` (`parse_align`)
|
||||
|
||||
**Problem**: found during the H1 fix session (probe: a struct with
|
||||
`align: 2^62` compiles in aligned mode and reports `total_size =
|
||||
2^63`). The meta-schema accepts any `align >= 1`, and `align_up`
|
||||
willingly rounds the running offset up by the full annotation — so a
|
||||
one-field schema can declare a layout of exabyte scale. Unlike H1 this
|
||||
does not crash: the allocation is offset *arithmetic*, not
|
||||
`with_capacity`, and `validate_bytes` on the absurd layout fails
|
||||
cleanly with a buffer-bounds `Access` error (probe-verified). Severity
|
||||
is Nit because there is no abort and no unbounded memory *at read
|
||||
time*; it is recorded because (a) `total_size` in that range is
|
||||
meaningless output the consumer may act on, (b) the align-driven
|
||||
stride is the one reachable path to `MAX_ARRAY_BYTES` (the H1 byte-cap
|
||||
test in `offset_map` uses exactly this), and (c) the same unbounded
|
||||
knob exists on `FieldDef.align`. A maximum align (e.g. 64 or 4096) in
|
||||
the meta-schema and/or a clamp-with-error in `parse_align` closes it;
|
||||
the honest layouts in the wild never need >64.
|
||||
|
||||
**Fix**: add `"maximum"` to both `align` properties in
|
||||
`bast_meta.rs`, or reject/clamp oversized values in `parse_align`
|
||||
(with a clean `Schema` error). A locking test (align above the cap →
|
||||
clean `Err`) mirrors the H1 test family.
|
||||
|
||||
---
|
||||
|
||||
## What's Good
|
||||
@@ -648,9 +754,8 @@ Worth recording, because the findings shouldn't eclipse it:
|
||||
|
||||
## Recommended Order
|
||||
|
||||
1. **H1** — release-blocking. Smallest correct fix: drop the three
|
||||
`Vec::with_capacity(count)`; add the compile-time cap + tests.
|
||||
Isolate the reproducer (see the Methodology warning).
|
||||
1. ~~**H1** — release-blocking~~ **resolved 2026-09-02** (with L1;
|
||||
see the resolution block on the finding).
|
||||
2. **H3** — needs a convention *decision* before code: write the
|
||||
addendum, enforce it, add L6's roundtrip test in the same session.
|
||||
3. **H2** — shared walk guard (or the minimum doc note if the full
|
||||
@@ -661,8 +766,10 @@ Worth recording, because the findings shouldn't eclipse it:
|
||||
6. **M4** — ongoing: per-fix coverage extension as recommended above;
|
||||
the aligned-materializer test gap (1) is the single biggest chunk
|
||||
and deserves its own session.
|
||||
7. **L1–L6, N1** — opportunistic, folded into whichever session touches
|
||||
the relevant file (L6 is the exception — it belongs with H3).
|
||||
7. **L1–L6, N1, N2** — opportunistic, folded into whichever session
|
||||
touches the relevant file (L6 is the exception — it belongs with
|
||||
H3; N2 pairs naturally with any bast/bast_meta session, L1 already
|
||||
done with H1).
|
||||
|
||||
## Notes
|
||||
|
||||
@@ -673,11 +780,18 @@ Worth recording, because the findings shouldn't eclipse it:
|
||||
If a reproducer test is wanted in-tree for H1/H2, it should be
|
||||
`#[ignore]`-gated with a comment pointing at the isolation
|
||||
requirements, or assert only the compile-time rejection (the safe
|
||||
half of the fix) in the default suite.
|
||||
half of the fix) in the default suite. The H1 resolution followed
|
||||
the second option: in-tree tests assert only compile-time
|
||||
rejections.
|
||||
- Coverage was measured with the default harness (`cargo llvm-cov
|
||||
--release`, summary + text). Numbers quoted are stable across two
|
||||
runs this session.
|
||||
- Severity here keys off AGENTS.md §3 (untrusted schema ⇒ handleable
|
||||
error) and the semver contract, not off effort: two of the three
|
||||
Highs are single-file, small-diff fixes; H3 is the only one that
|
||||
needs a decision first.
|
||||
needs a decision first.
|
||||
- The H1 fix session (2026-09-02) also confirmed the zero-progress
|
||||
gap (stride-0 array + zero-byte elements = unbounded loop) and the
|
||||
unbounded-`align` observation (new N2) while re-deriving the cap
|
||||
arithmetic — both recorded on their own findings rather than
|
||||
silently absorbed.
|
||||
+21
-11
@@ -35,7 +35,9 @@
|
||||
//! is used for any offset/count cast (AGENTS.md §4).
|
||||
|
||||
use crate::error::AlkTypeError;
|
||||
use crate::schema::{AlkTypeKind, Endian, VariableEncoding};
|
||||
use crate::schema::{
|
||||
AlkTypeKind, Endian, VariableEncoding, MAX_ARRAY_ELEMENTS,
|
||||
};
|
||||
use serde_json::Value;
|
||||
|
||||
const DEFS_KEY: &str = "$defs";
|
||||
@@ -851,6 +853,13 @@ impl BastArray {
|
||||
"bast: array at {path} has no `count` (variable-length arrays are not supported in v1, D-BAST-004)"
|
||||
))
|
||||
})?;
|
||||
if count > MAX_ARRAY_ELEMENTS {
|
||||
return Err(AlkTypeError::Schema(format!(
|
||||
"bast: array at {path} declares count {count}, which exceeds the \
|
||||
compile-time limit of {MAX_ARRAY_ELEMENTS} elements (untrusted schemas \
|
||||
must not be able to request unbounded per-element expansion)"
|
||||
)));
|
||||
}
|
||||
Ok(Self {
|
||||
element: Box::new(element),
|
||||
count,
|
||||
@@ -1750,7 +1759,10 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn array_count_large_u64_parses_on_64bit() {
|
||||
fn array_count_large_u64_rejected_by_compile_cap() {
|
||||
// Pre-0.3.1 this parsed u64::MAX verbatim (H1: the count fed
|
||||
// unbounded per-element expansion downstream). The cap now
|
||||
// rejects it at parse, before any walker sees the array.
|
||||
let root = json!({
|
||||
"$defs": {
|
||||
"S": { "kind": "struct", "fields": [
|
||||
@@ -1758,14 +1770,12 @@ mod tests {
|
||||
] }
|
||||
}
|
||||
});
|
||||
let d = BastDoc::new(&root, "S").expect("parses on usize>=u64 targets");
|
||||
let s = match d.root_def().kind() {
|
||||
BastDefKind::Struct(s) => s,
|
||||
_ => unreachable!(),
|
||||
};
|
||||
match s.fields()[0].ty() {
|
||||
BastType::Array(a) => assert_eq!(a.count(), usize::try_from(u64::MAX).unwrap()),
|
||||
other => panic!("expected Array, got {other:?}"),
|
||||
let err = BastDoc::new(&root, "S").unwrap_err();
|
||||
match err {
|
||||
AlkTypeError::Schema(reason) => {
|
||||
assert!(reason.contains("compile-time limit"), "reason: {reason}");
|
||||
}
|
||||
other => panic!("expected Schema error, got {other:?}"),
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
+63
-1
@@ -44,7 +44,9 @@ use crate::bast::{
|
||||
BastUnion,
|
||||
};
|
||||
use crate::error::AlkTypeError;
|
||||
use crate::schema::{AlkTypeKind, Endian, DISCRIMINATOR_PATH, U32_SIZE};
|
||||
use crate::schema::{
|
||||
AlkTypeKind, Endian, DISCRIMINATOR_PATH, U32_SIZE, MAX_ARRAY_BYTES,
|
||||
};
|
||||
use serde_json::Value;
|
||||
use std::collections::HashMap;
|
||||
|
||||
@@ -350,6 +352,21 @@ impl<'d> BuildCtx<'d> {
|
||||
reason: format!("element kind {elem_kind} has no fixed size"),
|
||||
})?;
|
||||
let count = array.count();
|
||||
let array_bytes = count
|
||||
.checked_mul(elem_size)
|
||||
.ok_or_else(|| AlkTypeError::Offset {
|
||||
field_path: field_path.to_string(),
|
||||
reason: format!("array size {count} × {elem_size} overflows usize"),
|
||||
})?;
|
||||
if array_bytes > MAX_ARRAY_BYTES {
|
||||
return Err(AlkTypeError::Offset {
|
||||
field_path: field_path.to_string(),
|
||||
reason: format!(
|
||||
"array size {count} × {elem_size} = {array_bytes} bytes exceeds the \
|
||||
compile-time limit of {MAX_ARRAY_BYTES} bytes"
|
||||
),
|
||||
});
|
||||
}
|
||||
|
||||
let start = *offset;
|
||||
for i in 0..count {
|
||||
@@ -896,6 +913,51 @@ mod tests {
|
||||
assert_eq!(layout.total_size(), 12);
|
||||
}
|
||||
|
||||
// ----- H1: array caps bound untrusted schemas at build time --------
|
||||
|
||||
#[test]
|
||||
fn h1_array_count_above_cap_rejected_at_parse() {
|
||||
let root = json!({
|
||||
"$defs": {
|
||||
"S": {
|
||||
"kind": "struct",
|
||||
"fields": [
|
||||
{ "name": "vals", "kind": { "kind": "array", "element": "uint8", "count": 2000000000 } }
|
||||
]
|
||||
}
|
||||
}
|
||||
});
|
||||
let err = LayoutBuilder::new(&root, "S").unwrap_err();
|
||||
match err {
|
||||
AlkTypeError::Schema(reason) => {
|
||||
assert!(reason.contains("compile-time limit"), "reason: {reason}");
|
||||
}
|
||||
other => panic!("expected Schema error, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn h1_array_bytes_above_cap_rejected_at_build_is_defense_in_depth() {
|
||||
// The byte cap cannot be reached through the public API with the
|
||||
// current caps (packed mode has no alignment: max fixed element
|
||||
// is 8 bytes, count <= 2^16, product <= 2^19 << 2^26) — the
|
||||
// check exists to hold if the element cap or the fixed-size kind
|
||||
// set ever grows. The reachable byte-cap path is exercised in
|
||||
// offset_map (aligned mode, align-driven stride).
|
||||
let root = json!({
|
||||
"$defs": {
|
||||
"S": {
|
||||
"kind": "struct",
|
||||
"fields": [
|
||||
{ "name": "vals", "kind": { "kind": "array", "element": "uint64", "count": 65536 } }
|
||||
]
|
||||
}
|
||||
}
|
||||
});
|
||||
let layout = build(&root, "S", &var_sizes(&[]));
|
||||
assert_eq!(layout.total_size(), 8 * 65536);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn array_fixed_count_after_preceding_field() {
|
||||
let root = json!({
|
||||
|
||||
+13
-3
@@ -245,7 +245,7 @@ fn materialize_plan_array(
|
||||
)))
|
||||
}
|
||||
};
|
||||
let mut arr = Vec::with_capacity(count);
|
||||
let mut arr = Vec::new();
|
||||
for i in 0..count {
|
||||
let path = format!("{field_path}[{i}]");
|
||||
let value = materialize_plan_composite(element, &path, endian, buffer, offset)?;
|
||||
@@ -634,10 +634,11 @@ fn materialize_array_packed(
|
||||
) -> Result<Value, AlkTypeError> {
|
||||
let element_ty = array.element();
|
||||
let count = array.count();
|
||||
let mut arr = Vec::with_capacity(count);
|
||||
let mut arr = Vec::new();
|
||||
for i in 0..count {
|
||||
let path = format!("{field_path}[{i}]");
|
||||
let resolved_elem = doc.resolve_typeref(element_ty)?;
|
||||
let before = *offset;
|
||||
let value = materialize_typeref_packed(
|
||||
doc,
|
||||
&resolved_elem,
|
||||
@@ -646,6 +647,15 @@ fn materialize_array_packed(
|
||||
buffer,
|
||||
offset,
|
||||
)?;
|
||||
if *offset == before {
|
||||
return Err(AlkTypeError::Access {
|
||||
field_path: path,
|
||||
reason: format!(
|
||||
"array element {i} consumed 0 bytes; a zero-size element makes the \
|
||||
declared count unbounded on the wire"
|
||||
),
|
||||
});
|
||||
}
|
||||
arr.push(value);
|
||||
}
|
||||
Ok(Value::Array(arr))
|
||||
@@ -921,7 +931,7 @@ fn materialize_array_aligned(
|
||||
let element_ty = array.element();
|
||||
let resolved_elem = doc.resolve_typeref(element_ty)?;
|
||||
let count = array.count();
|
||||
let mut arr = Vec::with_capacity(count);
|
||||
let mut arr = Vec::new();
|
||||
for i in 0..count {
|
||||
let elem_path = format!("{field_path}[{i}]");
|
||||
let entry = offset_map.get(&elem_path).ok_or_else(|| AlkTypeError::Offset {
|
||||
|
||||
+68
-1
@@ -16,7 +16,7 @@ use crate::bast::{
|
||||
BastArray, BastDefKind, BastDoc, BastField, BastStruct, BastType,
|
||||
};
|
||||
use crate::error::AlkTypeError;
|
||||
use crate::schema::{AlkTypeKind, Endian, VariableEncoding};
|
||||
use crate::schema::{AlkTypeKind, Endian, VariableEncoding, MAX_ARRAY_BYTES};
|
||||
|
||||
/// A byte range within a buffer.
|
||||
///
|
||||
@@ -415,6 +415,21 @@ impl<'d> ComputeCtx<'d> {
|
||||
let elem_align = element_alignment(&resolved_elem, struct_default_align, elem_natural);
|
||||
let stride = round_up(elem_size, elem_align);
|
||||
let count = array.count();
|
||||
let array_bytes = count
|
||||
.checked_mul(stride)
|
||||
.ok_or_else(|| AlkTypeError::Offset {
|
||||
field_path: field_path.to_string(),
|
||||
reason: format!("array size {count} × stride {stride} overflows usize"),
|
||||
})?;
|
||||
if array_bytes > MAX_ARRAY_BYTES {
|
||||
return Err(AlkTypeError::Offset {
|
||||
field_path: field_path.to_string(),
|
||||
reason: format!(
|
||||
"array size {count} × stride {stride} = {array_bytes} bytes exceeds the \
|
||||
compile-time limit of {MAX_ARRAY_BYTES} bytes"
|
||||
),
|
||||
});
|
||||
}
|
||||
|
||||
let array_align = field_alignment(field, struct_default_align, elem_align);
|
||||
|
||||
@@ -682,6 +697,58 @@ mod tests {
|
||||
assert_eq!(m.total_size(), 12);
|
||||
}
|
||||
|
||||
// ----- H1: array caps bound untrusted schemas at compute time ------
|
||||
|
||||
#[test]
|
||||
fn h1_array_count_above_cap_rejected_at_parse() {
|
||||
let root = json!({
|
||||
"$defs": {
|
||||
"S": {
|
||||
"kind": "struct",
|
||||
"fields": [
|
||||
{ "name": "vals", "kind": { "kind": "array", "element": "uint8", "count": 2000000000 } }
|
||||
]
|
||||
}
|
||||
}
|
||||
});
|
||||
let err = BastDoc::new(&root, "S").unwrap_err();
|
||||
match err {
|
||||
AlkTypeError::Schema(reason) => {
|
||||
assert!(reason.contains("compile-time limit"), "reason: {reason}");
|
||||
}
|
||||
other => panic!("expected Schema error, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn h1_array_bytes_above_cap_rejected_at_compute() {
|
||||
// 65536 elements × stride 2^20 (a u8 array in a struct with
|
||||
// align 2^20) = 2^36 > 2^26 — hits the byte cap while staying
|
||||
// under the element cap. (A struct/array element would be
|
||||
// rejected earlier as variable-length, OQ-001, so the reachable
|
||||
// way over the byte cap is a huge stride, not a huge element.)
|
||||
let root = json!({
|
||||
"$defs": {
|
||||
"S": {
|
||||
"kind": "struct",
|
||||
"align": 1048576u64,
|
||||
"fields": [
|
||||
{ "name": "vals", "kind": { "kind": "array", "element": "uint8", "count": 65536 } }
|
||||
]
|
||||
}
|
||||
}
|
||||
});
|
||||
let doc = BastDoc::new(&root, "S").expect("bast doc (under element cap)");
|
||||
let err = OffsetMap::compute(&doc).unwrap_err();
|
||||
match err {
|
||||
AlkTypeError::Offset { field_path, reason } => {
|
||||
assert_eq!(field_path, "vals");
|
||||
assert!(reason.contains("exceeds the compile-time limit"), "reason: {reason}");
|
||||
}
|
||||
other => panic!("expected Offset error, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn variable_string_length_prefix_at_known_offset() {
|
||||
let root = json!({
|
||||
|
||||
+134
-16
@@ -41,7 +41,9 @@ use crate::bast::{
|
||||
BastType, BastUnion,
|
||||
};
|
||||
use crate::error::AlkTypeError;
|
||||
use crate::schema::{AlkTypeKind, Endian, VariableEncoding};
|
||||
use crate::schema::{
|
||||
AlkTypeKind, Endian, VariableEncoding, MAX_ARRAY_BYTES,
|
||||
};
|
||||
use serde_json::Value;
|
||||
use std::collections::{BTreeMap, BTreeSet};
|
||||
use std::sync::Arc;
|
||||
@@ -544,10 +546,22 @@ fn compile_array(
|
||||
let element_path = format!("{path}.element");
|
||||
let (kind, body) = compile_typeref(doc, a.element(), container_endian, &element_path, depth, seen)?;
|
||||
let element = wrap_leaf(kind, body, container_endian, &element_path)?;
|
||||
let element_stride = fixed_composite_size(&element).unwrap_or_default();
|
||||
let element_stride = fixed_composite_size(&element)?.unwrap_or(0);
|
||||
let count = a.count();
|
||||
let total = count.checked_mul(element_stride).ok_or_else(|| {
|
||||
AlkTypeError::Schema(format!(
|
||||
"read_plan: array size {count} × {element_stride} overflows usize at {path}"
|
||||
))
|
||||
})?;
|
||||
if total > MAX_ARRAY_BYTES {
|
||||
return Err(AlkTypeError::Schema(format!(
|
||||
"read_plan: array at {path} computes to {count} × {element_stride} = {total} bytes, \
|
||||
which exceeds the compile-time limit of {MAX_ARRAY_BYTES} bytes"
|
||||
)));
|
||||
}
|
||||
Ok(CompositePlan::Array {
|
||||
element: Box::new(element),
|
||||
count: a.count(),
|
||||
count,
|
||||
element_stride,
|
||||
})
|
||||
}
|
||||
@@ -613,44 +627,82 @@ fn wrap_leaf(
|
||||
/// contribute their fixed size; structs sum their fields; nested arrays
|
||||
/// with fixed elements contribute `count × stride`; unions, records,
|
||||
/// and variable-length primitives make the whole node variable.
|
||||
fn fixed_composite_size(body: &CompositePlan) -> Option<usize> {
|
||||
///
|
||||
/// Returns `Err` only on arithmetic overflow while summing struct field
|
||||
/// sizes or array products — after the array caps ([`MAX_ARRAY_BYTES`],
|
||||
/// [`MAX_ARRAY_ELEMENTS`]) no honest schema can reach those arms, and
|
||||
/// treating overflow as "variable-length" (the pre-0.3.1
|
||||
/// `unwrap_or_default` behavior) would silently mis-plan the wire.
|
||||
fn fixed_composite_size(body: &CompositePlan) -> Result<Option<usize>, AlkTypeError> {
|
||||
match body {
|
||||
CompositePlan::Struct(plan) => fixed_plan_size(plan),
|
||||
CompositePlan::Union { .. } | CompositePlan::Record { .. } => None,
|
||||
CompositePlan::Union { .. } | CompositePlan::Record { .. } => Ok(None),
|
||||
CompositePlan::Array {
|
||||
count,
|
||||
element_stride,
|
||||
..
|
||||
} => {
|
||||
if *element_stride == 0 {
|
||||
None
|
||||
Ok(None)
|
||||
} else {
|
||||
count.checked_mul(*element_stride)
|
||||
match count.checked_mul(*element_stride) {
|
||||
Some(total) => Ok(Some(total)),
|
||||
None => Err(AlkTypeError::Schema(
|
||||
"internal: array size product overflowed while sizing a fixed-stride \
|
||||
composite (array caps should have rejected this schema earlier)"
|
||||
.to_string(),
|
||||
)),
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn fixed_plan_size(plan: &ReadPlan) -> Option<usize> {
|
||||
fn fixed_plan_size(plan: &ReadPlan) -> Result<Option<usize>, AlkTypeError> {
|
||||
let mut total = 0usize;
|
||||
for field in plan.fields() {
|
||||
let size = match field.kind() {
|
||||
ReadKind::Primitive(k) if k.is_fixed_size() => k.type_size()?,
|
||||
ReadKind::Enum => AlkTypeKind::Enum.type_size()?,
|
||||
ReadKind::Struct | ReadKind::Array => {
|
||||
fixed_composite_size(field.body().as_ref()?)?
|
||||
ReadKind::Primitive(k) if k.is_fixed_size() => {
|
||||
k.type_size().ok_or_else(|| {
|
||||
AlkTypeError::Schema(format!(
|
||||
"internal: fixed kind {k} has no size while sizing a plan"
|
||||
))
|
||||
})?
|
||||
}
|
||||
ReadKind::Union | ReadKind::Record => return None,
|
||||
ReadKind::Primitive(_) => return None,
|
||||
ReadKind::Enum => AlkTypeKind::Enum.type_size().ok_or_else(|| {
|
||||
AlkTypeError::Schema("internal: enum has no size while sizing a plan".to_string())
|
||||
})?,
|
||||
ReadKind::Struct | ReadKind::Array => {
|
||||
match fixed_composite_size(
|
||||
field
|
||||
.body()
|
||||
.as_ref()
|
||||
.ok_or_else(|| AlkTypeError::Schema(format!(
|
||||
"internal: composite field {} has no body while sizing",
|
||||
field.name()
|
||||
)))?,
|
||||
)? {
|
||||
Some(size) => size,
|
||||
None => return Ok(None),
|
||||
}
|
||||
}
|
||||
ReadKind::Union | ReadKind::Record => return Ok(None),
|
||||
ReadKind::Primitive(_) => return Ok(None),
|
||||
};
|
||||
total = total.checked_add(size)?;
|
||||
total = total.checked_add(size).ok_or_else(|| {
|
||||
AlkTypeError::Schema(
|
||||
"internal: struct field sizes overflowed usize while sizing a plan".to_string(),
|
||||
)
|
||||
})?;
|
||||
}
|
||||
Some(total)
|
||||
Ok(Some(total))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::engine::{AlkTypeEngine, LayoutMode};
|
||||
use crate::schema::MAX_ARRAY_ELEMENTS;
|
||||
use serde_json::json;
|
||||
|
||||
const LE: Endian = Endian::Little;
|
||||
@@ -926,6 +978,72 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
// ----- H1: array caps bound untrusted schemas at compile time -------
|
||||
|
||||
fn array_root(count: serde_json::Value, element: serde_json::Value) -> Value {
|
||||
json!({ "$defs": { "S": { "kind": "struct", "fields": [
|
||||
{ "name": "vals", "kind": { "kind": "array", "element": element, "count": count } }
|
||||
]}}})
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn h1_array_count_above_cap_rejected_at_compile() {
|
||||
let root = array_root(serde_json::json!(2_000_000_000u64), serde_json::json!("uint8"));
|
||||
let err = ReadPlan::compile(&root, "S").unwrap_err();
|
||||
match err {
|
||||
AlkTypeError::Schema(reason) => {
|
||||
assert!(
|
||||
reason.contains("compile-time limit"),
|
||||
"reason: {reason}"
|
||||
);
|
||||
}
|
||||
other => panic!("expected Schema error, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn h1_array_count_at_cap_compiles() {
|
||||
let root = array_root(
|
||||
serde_json::json!(MAX_ARRAY_ELEMENTS),
|
||||
serde_json::json!("uint8"),
|
||||
);
|
||||
let p = plan(&root, "S");
|
||||
match p.fields()[0].body().expect("array body") {
|
||||
CompositePlan::Array { count, .. } => assert_eq!(*count, MAX_ARRAY_ELEMENTS),
|
||||
other => panic!("expected Array body, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn h1_array_bytes_above_cap_rejected_at_compile() {
|
||||
// 65536 elements × 1032-byte stride (129 × u64 inner array) =
|
||||
// 67,633,152 > 2^26 — hits the byte cap while staying under the
|
||||
// element cap.
|
||||
let root = array_root(
|
||||
serde_json::json!(MAX_ARRAY_ELEMENTS),
|
||||
serde_json::json!({ "kind": "array", "element": "uint64", "count": 129 }),
|
||||
);
|
||||
let err = ReadPlan::compile(&root, "S").unwrap_err();
|
||||
match err {
|
||||
AlkTypeError::Schema(reason) => {
|
||||
assert!(
|
||||
reason.contains("exceeds the compile-time limit"),
|
||||
"reason: {reason}"
|
||||
);
|
||||
}
|
||||
other => panic!("expected Schema error, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn h1_huge_count_short_buffer_validate_is_clean_error() {
|
||||
let root = array_root(serde_json::json!(1024), serde_json::json!("uint8"));
|
||||
let engine =
|
||||
AlkTypeEngine::compile(&root, "S", LayoutMode::Packed, None).expect("compile");
|
||||
let err = engine.validate_bytes(&[0u8; 4]).unwrap_err();
|
||||
assert!(matches!(err, AlkTypeError::Access { .. }), "got {err:?}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cov_record() {
|
||||
let root = json!({ "$defs": { "S": { "kind": "struct", "fields": [
|
||||
|
||||
@@ -16,6 +16,20 @@ use std::fmt;
|
||||
pub(crate) const U32_SIZE: usize = 4;
|
||||
pub(crate) const DISCRIMINATOR_PATH: &str = "__discriminator";
|
||||
|
||||
/// Maximum declared array element count. Schemas are untrusted input
|
||||
/// (AGENTS.md §3): every layout walker expands `count` into per-element
|
||||
/// work (offset-map/layout entries, per-element reads), so an unbounded
|
||||
/// count is a memory/CPU denial-of-service. The cap applies at parse
|
||||
/// time, before any walker sees the array; 2^16 elements bounds the
|
||||
/// per-array compile-time entry cost at ~10 MB.
|
||||
pub(crate) const MAX_ARRAY_ELEMENTS: usize = 1 << 16;
|
||||
|
||||
/// Maximum computed byte size of a single array (`count × element
|
||||
/// stride`). Bounds the arithmetic product independently of
|
||||
/// [`MAX_ARRAY_ELEMENTS`] so multi-megabyte strides cannot combine with
|
||||
/// a legal count into an unbounded layout request.
|
||||
pub(crate) const MAX_ARRAY_BYTES: usize = 1 << 26;
|
||||
|
||||
/// The 18 BAST kinds recognized by the engine.
|
||||
///
|
||||
/// Each variant corresponds to a lowercase BAST kind string
|
||||
|
||||
@@ -765,6 +765,15 @@ fn plan_walk_variable_array_size(
|
||||
reason: format!("array element walked backwards: {position} → {new_position}"),
|
||||
});
|
||||
}
|
||||
if new_position == position {
|
||||
return Err(AlkTypeError::Access {
|
||||
field_path: element_path,
|
||||
reason: format!(
|
||||
"array element {i} consumed 0 bytes; a zero-size element makes the \
|
||||
declared count unbounded on the wire"
|
||||
),
|
||||
});
|
||||
}
|
||||
position = new_position;
|
||||
}
|
||||
Ok(position - start)
|
||||
|
||||
Reference in new issue
Block a user