diff --git a/CHANGELOG.md b/CHANGELOG.md index f4d3258..34b9b5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,417 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Fixed + +- **UTF-8→UTF-16 transcoding read 1–3 bytes past the input buffer for + truncated multi-byte UTF-8 sequences — silent cross-memory leak, + mirror of LS-P-16** (LS-P-19, UCA-P-3, H-2 / H-4 / H-4.4, + `meld-core/src/adapter/fact.rs`). `emit_utf8_to_utf16_transcode`'s + outer loop bounds-checked only the lead byte; each multi-byte + branch then unconditionally read its continuation bytes at + `src_idx + 1`/`+2`/`+3`. A UTF-8 string ending on a truncated + multi-byte lead (e.g. a bare `0xE2` at the buffer tail) caused the + adapter to load up to 3 bytes of attacker-adjacent caller memory, + fold them into a synthesized code point, and emit the result as + UTF-16 into the callee output. Conservative mitigation prepends a + `src_idx + N >= input_len` `unreachable` trap to each multi-byte + branch (N = 1/2/3). Promoted to approved loss scenario + **LS-P-19** (priority high). Regression pinned by + `ls_p_19_utf8_to_utf16_continuation_byte_oob_guard_emitted`. + +- **LS-P-12 bypassed when a record mixed a covered pointer with a + hidden conditional one — silent cross-memory dangling pointers in + callee elements** (LS-P-18, UCA-P-3, H-4 / H-4.2, + `meld-core/src/parser.rs`). The original LS-P-12 guard in + `copy_layout(List(inner))` fired only when + `element_inner_pointers(inner, 0)` was empty AND `inner` contained + pointers. A record/tuple that mixed a covered field (bare + `string`/`list`) with a hidden conditional pointer field (e.g. + `option`) made `element_inner_pointers` return a non-empty + Vec from the covered field — the LS-P-12 panic didn't fire, and + `copy_layout` produced `CopyLayout::Elements` whose + `inner_pointers` omitted the option-payload pointer. The FACT + adapter then never fixed up the conditional payload across + memories, leaving callee elements with stale string pointers per + `Some(_)`. Fix replaces the emptiness-based guard with a deep + recursive `has_pointer_bearing_conditional(inner)` check (new + helper) that walks Option / Result / Variant arms reachable + through Records, Tuples, FixedSizeLists, and `Type` aliases. If + any pointer-bearing conditional exists, `copy_layout(List(inner))` + panics with the LS-P-18 marker. Promoted to approved loss scenario + **LS-P-18** (priority high). Regression pinned by + `ls_p_18_mixed_record_with_option_string_bypasses_p12_then_refuses` + + positive sanity test + `ls_p_18_pure_bare_pointer_record_still_works`. + +- **Caller-encoding name match filtered on `ComponentTypeRef::Func` + only, miscalibrated `string_transcoding` for mixed-encoding + callers — defensive warn-before-heuristic** (LS-P-17, UCA-R-3, + H-4 / H-4.4, `meld-core/src/resolver.rs`). Two structurally- + identical lookup loops (primary around lines 2877–2910, fallback + around 3175–3225) filtered `from_component.imports` on + `ComponentTypeRef::Func(_)` to find a caller's `Lower` options + for the resolved interface. WIT interface imports lower to + `ComponentTypeRef::Instance(_)`, so the loops **never matched** + for typical wit-component / wasm-tools output and fell through to + a heuristic `caller_lower_map.iter().min_by_key(|(k, _)| **k)` + that picked the lowest-indexed Lower's encoding for **every** + interface. For single-encoding callers (the common case) this + happens to be correct; for mixed-encoding callers (UTF-16 for one + interface, UTF-8 for another — e.g. JS/.NET host components) the + heuristic miscalibrated `string_transcoding` across the board, + silently scrambling strings at one or more call boundaries. + **Mitigation only**: detect mixed-encoding callers (`values()` + not all identical) before the heuristic and emit a `log::warn!` + with the LS-P-17 marker and the interface name. The full + structural per-interface attribution (Instance-aliased func + indices or per-interface lookup) is tracked as follow-up. **A + confirmed Mythos finding** — surfaced by the mythos-auto + delta-pass on PR #179, clean-room verified. Promoted to approved + loss scenario **LS-P-17** (priority low — bug is latent for + single-encoding callers). Regression pinned by + `ls_p_17_mixed_caller_encoding_warns_before_heuristic_fallback`. + +- **UTF-16→UTF-8 transcoding read 2 bytes past the input buffer on a + lone high surrogate at the end of a UTF-16 string — silent + cross-memory leak of adjacent caller bytes into the callee's UTF-8 + output** (LS-P-16, UCA-P-3, H-2 / H-4 / H-4.4, + `meld-core/src/adapter/fact.rs`). + `emit_utf16_to_utf8_transcode`'s only bounds check was + `src_idx >= input_len` against the *first* code unit per iteration. + When that code unit was a high surrogate (`[0xD800, 0xDC00)`), the + surrogate-pair `If` arm unconditionally emitted a second + `I32Load16U` at `mem16[ptr + (src_idx + 1) * 2]`. For input whose + last code unit was a lone high surrogate, that load read 2 bytes + past the caller-supplied UTF-16 buffer; those 2 attacker-adjacent + bytes were treated as the "low surrogate" without validating + `0xDC00 <= cu2 < 0xE000` and packed into a 4-byte UTF-8 sequence + written to callee memory — silent cross-memory leak per + transcoded string. `src_idx += 2` advanced past `input_len`, so + the outer break check cleanly terminated the loop, no trap, + silent corruption. **Conservative mitigation only**: inject an + inline `src_idx + 1 >= input_len` check inside the surrogate-pair + `If` arm and `unreachable`-trap on failure. The Canonical-ABI- + correct behaviour replaces the lone surrogate with U+FFFD + (3-byte UTF-8 `EF BF BD`) and continues; that upgrade is tracked + as a separate structural follow-up. **A confirmed Mythos + finding** — surfaced by the mythos-auto delta-pass on PR #179, + independently clean-room verified. Promoted to approved loss + scenario **LS-P-16** (priority high). Regression pinned by + `ls_p_16_utf16_lone_high_surrogate_oob_guard_emitted`, a + structural test that requires the LS-P-16 marker AND an + `Unreachable` + `I32GeU` opcode pair to live inside the + surrogate-pair `If` arm before the second `I32Load16U`. + +- **Out-of-bounds `resource_type_id` silently misclassified as + callee-defined — `[resource-rep]` / `[resource-new]` swap on the + fused adapter** (LS-P-15, UCA-R-3, H-5 / H-1, + `meld-core/src/resolver.rs`). `resolve_resource_positions` decided + callee-vs-caller resource ownership via + `.get(pos.resource_type_id as usize).map(|def| !matches!(def, + ComponentTypeDef::Import(_))).unwrap_or(true)`. When the id + exceeded `component_type_defs.len()` (stale id, alias remap past + the local table, malformed input), `.get(...) → None` and + `.unwrap_or(true)` silently classified the resource as + *callee-defined* with no warning — the adapter emitted a + `[resource-rep]` call where `[resource-new]` was correct (or vice + versa), swapping the two resource-handle conversion sides on every + fused cross-component call passing that handle. The handle + type-checks on both sides, so the validator doesn't catch it. + Reachability is bounded — the instance-graph path keys + `resource_type_id` through validated parser-produced indices — + making this **defensive hardening**, not a memory-safety + emergency. Replaces the `unwrap_or(true)` with an explicit `match`: + on `None`, emit `log::warn!` and `continue` so the position is + dropped instead of silently misclassified; the downstream adapter + will surface a loud missing-fixup error at adapter generation + rather than silently swap. **A confirmed Mythos finding** — + surfaced by the mythos-auto delta-pass on PR #179. Promoted to + approved loss scenario **LS-P-15** (priority low). Regression + pinned by + `ls_p_15_out_of_bounds_resource_type_id_is_dropped_not_misclassified`. + +- **Nested-list inner copy `buf_len = len * sub_elem_size` missed the + overflow guard** (LS-P-14, UCA-P-3, H-2 / H-4 / H-4.1, + `meld-core/src/adapter/fact.rs`). + `emit_patch_nested_indirections` — the per-element fixup loop for a + list-of-records (or tuples) — computed the byte count for each inner + list's `cabi_realloc` + `memory.copy` by loading the callee's + `len` field and multiplying by `sub_elem_size` with a bare + `i32.mul`. `i32.mul` is modulo 2³², so a callee-supplied `len` near + `u32::MAX / sub_elem_size` wrapped `buf_len` to a small value; + the subsequent `old_ptr + buf_len > mem_bytes` bounds check used + `i32.add` (also wrapping) and was bypassed. The adapter then + allocated/copied only the wrapped byte count while the caller-side + bulk copy of the outer element kept the original large `len` — + silent inner-list truncation, plus OOB read/write into adjacent + caller-allocated memory on every dereference past the truncated + edge. The `emit_overflow_guard` helper (added as the LS-A-7 leg + (a) fix for the outer copy paths) was never retrofitted to the + inner copy. Fix stashes the loaded `len` into the existing + `l_buf_len` scratch local, calls `emit_overflow_guard(body, + l_buf_len, sub_elem_size as u32)` to trap via `unreachable` on + wrapping, then re-fetches the local for the multiplication. + **A confirmed Mythos finding** — surfaced by the mythos-auto + delta-pass on PR #179. Promoted to approved loss scenario + **LS-P-14** (priority high). Regression pinned by + `ls_p_14_nested_list_inner_copy_emits_overflow_guard`, which + emits a synthetic patch loop for `record { items: list }` + (`sub_elem_size = 4`) and asserts the encoded function body + contains an `Unreachable` opcode (the only place that opcode + appears along this path is inside `emit_overflow_guard`). + +- **Async adapter param-copy treated every consecutive `(i32, i32)` + pair as a pointer pair, corrupting plain integer args** (LS-P-13, + UCA-P-3, H-2 / H-4 / H-4.1, + `meld-core/src/adapter/fact.rs`). The P3-async lift adapter + (`emit_param_copy_step`, called from + `generate_async_callback_adapter` / + `generate_async_stackful_adapter`) walked `caller_type.params` + looking for adjacent `(i32, i32)` slots and gated each match on + `pointer_pair_positions.iter().any(|_| true)` — semantically + `!is_empty()`. Every adjacent integer-pair argument was rewritten + via `cabi_realloc` + cross-memory `memory.copy` as if it were a + `(ptr, len)` string/list, using one integer as the source pointer + and the other as the byte count. For `fn f(a, s: string, b, c)` + the buggy code copied `(a, ptr_s)` and `(len_s, b)` as + pointer-pair slots and missed the real string at flat index 1 — + callee saw corrupted integer args, the real string was never + copied, and `memory.copy` read from caller-memory addresses + influenced by the caller. The resolver's + `pointer_pair_param_positions` already returns the correct *flat* + indices (computed via `flat_count`), and canonical lowering + preserves param order between caller and callee component types — + the heuristic walk's claim of a caller/callee mismatch was + misleading. Replaces the whole walk with + `site.requirements.pointer_pair_positions.clone()`. **A confirmed + Mythos finding** — surfaced by the mythos-auto delta-pass on PR + #179, clean-room verified. Promoted to approved loss scenario + **LS-P-13** (priority high); regression pinned by + `ls_p_13_pointer_pair_param_positions_is_flat_indices_not_just_nonempty`. + +- **`list>` / `list>` / `list` silently produced stale cross-memory pointers + in callee elements — defensive refusal (partial mitigation)** (LS-P-12, + UCA-P-3, H-4 / H-4.2, `meld-core/src/parser.rs`). + `element_inner_pointers` (the helper that builds the per-element + inner-pointer descriptor consumed by `CopyLayout::Elements`) has no + arms for `Option` / `Result` / `Variant` — its match falls through + to `_ => {}`. So `copy_layout(list>)` (and the + Result/Variant analogues) classified as `CopyLayout::Bulk { + byte_multiplier }` with no per-element pointer walk — even though + `type_contains_pointers(option) == true`. The FACT adapter's + Bulk path is a flat `memory.copy(dst, src, len * byte_mult)` with + no per-element fixup, so every option's `(ptr, len)` pair was copied + verbatim into the callee — `ptr` still referencing the *source* + component's memory. The callee then dereferenced a wild pointer for + each `Some(...)` element. **Mitigation only**: `copy_layout` now + detects the smoking gun (`element_inner_pointers` empty AND + `type_contains_pointers(inner)`) and panics with a clearly-labelled + `LS-P-12: …` message at adapter-generation time, converting silent + cross-memory dangling-reference into a loud refusal. The full + structural fix — Option/Result/Variant arms on `element_inner_pointers`, + per-element `DiscriminantGuard` chains on the inner-pointer + descriptor, and adapter-side per-element guard evaluation + (structurally analogous to LS-P-10 on the list-element axis) — is + tracked as follow-up. **A confirmed Mythos finding** — surfaced by + the mythos-auto delta-pass on PR #179, clean-room verified. + Promoted to approved loss scenario **LS-P-12** (priority high). + Regression pinned by + `ls_p_12_list_of_option_string_refuses_rather_than_silently_corrupts`, + `ls_p_12_list_of_result_string_refuses`, and a positive sanity test + `ls_p_12_pure_scalar_option_list_is_still_bulk` that pins the check + is gated on `type_contains_pointers(inner)`. + +- **Flat-name resolver silently overwrote duplicate module exports** + (LS-P-11, UCA-R-3, H-5 / H-1, `meld-core/src/resolver.rs`, + `meld-core/src/error.rs`). `resolve_via_flat_names` built its export + index with a blind `HashMap::insert(key, …)` where `key` is the flat + export name. When two core modules within one component both + exported the same name, the second silently overwrote the first + (last-writer wins), routing any importer of that name to the wrong + module with no error or warning. The instance-graph resolver + (taken whenever the component has an `InstanceSection`, which + `wit-component` / `wasm-tools` always emit for multi-module + components) is immune; the vulnerable path is practically + unreachable for production components, **defensive hardening** for + the synthetic-fixture and legacy single-module fallback shapes. + Replaces the blind insert with an explicit collision check that + returns a new `Error::DuplicateModuleExport { component_idx, + export_name, first_module_idx, second_module_idx }`, mirroring the + existing `DuplicateModuleInstantiation` pattern. **A confirmed + Mythos finding** — clean-room verified, promoted to approved loss + scenario **LS-P-11** (priority `low`); regression pinned by + `ls_p_11_duplicate_flat_name_export_is_rejected`. + +- **Nested conditional pointer pair omitted outer-discriminant guard — + arbitrary cross-component read with attacker-controlled `(ptr, len)`** + (LS-P-10, UCA-P-3, H-2 / H-4 / H-4.2, `meld-core/src/parser.rs`, + `meld-core/src/resolver.rs`, `meld-core/src/adapter/fact.rs`). For a + pointer-containing type nested inside an option/result/variant arm + — `result, u32>`, + `variant { a(option), b(u32) }`, `option>`, + etc. — `collect_conditional_pointers` / + `collect_conditional_result_pointers` emitted a + `ConditionalPointerPair` guarded **only** on the innermost + discriminant. The four FACT adapter consumer loops processed each + pair independently: load disc → compare to value → fire copy. When + the runtime value was `Err(v: u32)` (or any sibling arm whose payload + type is something else), the byte at the option's discriminant slot + was actually the `u32` payload; if those bytes happened to read as + `1`, the adapter sampled the adjacent slots as a `(ptr, len)` string + pair and ran `cabi_realloc` + `memory.copy` with attacker-controlled + source pointer and length — **an arbitrary cross-component memory + read** that also hands the callee a forged string pointer pointing + into the freshly allocated buffer. New `DiscriminantGuard` struct + + `outer_guards: Vec` field on + `ConditionalPointerPair`; both collectors thread the enclosing + discriminant chain through their recursion and stamp it onto each + emitted pair. Two new fact-adapter helpers + (`emit_conditional_guard_chain_flat` / + `emit_conditional_guard_chain_byte`) emit each guard's `(load disc, + I32Const value, I32Eq)` and `I32And` them before the existing `If` / + copy block. The innermost guard stays in the existing + `discriminant_*` fields, so single-level conditionals (empty + `outer_guards`) behave identically. **A confirmed Mythos finding** — + surfaced by the mythos-auto delta-pass on PR #179, independently + clean-room-verified as a real memory-safety hazard, promoted to + approved loss scenario **LS-P-10**; regression pinned by + `ls_p_10_nested_conditional_pointer_carries_outer_guard_chain`. + +- **`total_flat_params` used `Iterator::sum::()` instead of a + saturating fold** (LS-P-9, UCA-P-3, H-2 / H-4, + `meld-core/src/parser.rs`). The Component Model canonical ABI picks + the calling convention from `total_flat_params`: `<= + MAX_FLAT_PARAMS` (16) → flat; `>` → params-ptr. `flat_count` for a + `FixedSizeList` is `flat_count(elem).saturating_mul(len)` + (LS-P-4), so a nested `FixedSizeList` can produce a per-param + `flat_count` of `u32::MAX`. A bare `.sum()` over `u32` then panics + in debug on `u32::MAX + 1` and **wraps to a small value** in + release; the wrapped total compares `<= 16` and the adapter selects + the flat convention for a function that genuinely needs params-ptr + — call-site lowering and callee-side lifting disagree on the ABI + slot. The sibling area-size accumulators inside + `params_area_byte_size` / `return_area_byte_size` already use + `saturating_add` (LS-P-6); this calling-convention picker was + missed. Replaces `.sum()` with `.fold(0u32, u32::saturating_add)`. + **A confirmed Mythos finding** — surfaced by the mythos-auto + delta-pass on PR #179, promoted to approved loss scenario + **LS-P-9**; regression pinned by + `ls_p_9_total_flat_params_saturates_across_params`. + +- **Record/tuple field-walk added the unpadded child size, undercounting + every offset and size built on it** (LS-P-8, UCA-P-3, + H-4 / H-4.1 / H-4.2, `meld-core/src/parser.rs`). The Component Model + canonical ABI lays out a record/tuple as `s = 0; for each field f: + s = align_to(s, alignment(f)); s += size(f)`, where `size(f)` for an + aggregate field is its *full padded* size. In this codebase the full + padded size is `canonical_abi_element_size`; `canonical_abi_size_unpadded` + is the outer type *without* its own trailing align-up. Roughly 25 + field-walk sites — the `Record` / `Tuple` arms of + `canonical_abi_size_unpadded`, `collect_pointer_byte_offsets`, + `collect_pointer_byte_offsets_with_layout` (the LS-P-7 helper), + `collect_conditional_result_pointers`, `collect_return_area_type_slots`, + `collect_resource_byte_positions`, `element_inner_pointers`, + `element_inner_resources`, plus the top-level params/results walks in + `params_area_byte_size`, `return_area_byte_size`, + `pointer_pair_params_byte_offsets`, `pointer_pair_result_offsets`, + `params_area_slots`, `return_area_slots`, + `resource_params_area_positions`, `resource_result_positions`, and + `conditional_pointer_pair_result_positions` — advanced the running + offset by `canonical_abi_size_unpadded(field)` instead of + `canonical_abi_element_size(field)`. The per-field `align_up` on the + next field does NOT re-absorb the preceding field's omitted trailing + pad when the next field has *smaller* alignment, so a + `tuple` came out with `_unpadded` 6 and + `element_size` 8 instead of the spec's 9 / 12, and a `list` + following `record { u32, u8 }` was located at offset 5 instead of 8. + Wrong field offsets flow into the FACT adapter's pointer-pair load, + list-copy byte length, and inner pointer-fixup walk; the area-size + callers under-size the `cabi_realloc` buffer (LS-P-6 hazard class + reached via the per-field primitive rather than the cross-field `+=`). + Fix replaces `canonical_abi_size_unpadded(field)` with + `canonical_abi_element_size(field)` at the 25 field-walk sites; + `canonical_abi_size_unpadded` itself still returns the outer size + minus its own trailing pad (its contract is unchanged — + `canonical_abi_element_size` recovers the pad). **A confirmed Mythos + finding** — surfaced by the mythos-auto delta-pass on PR #179, with + the auto-runner mis-locating the bug as the option/variant/result + payload contribution (which is actually spec-correct); independent + clean-room verification corrected the location to the Record/Tuple + field accumulation. Promoted to approved loss scenario **LS-P-8**; + regression pinned by + `ls_p_8_record_tuple_field_accumulation_uses_padded_field_size`. + +- **Conditional-pointer `CopyLayout` computed for the composite payload, + not the pointer leaf** (LS-P-7, UCA-P-3, H-4 / H-4.1 / H-4.2, + `meld-core/src/parser.rs`). `collect_conditional_pointers` (flat-param + path) and `collect_conditional_result_pointers` (retptr byte-offset + path) emit one `ConditionalPointerPair` per pointer leaf inside an + `option` / `result` / `variant` payload. They enumerated each leaf's + position correctly but computed the `CopyLayout` once by calling + `copy_layout` on the *whole payload type* and reusing it for every + position. `copy_layout` only special-cases a bare `string` / `list`; + any composite payload (`record`, `tuple`, `fixed-length-list`) fell + to its `_ => Bulk { byte_multiplier: 1 }` fallback. So a `list` + leaf inside `option>>` or + `result }, E>` was tagged `Bulk { 1 }` + instead of `Bulk { 8 }` — the adapter copies `len * 1` bytes, a 7/8 + silent under-copy — and a pointer-containing `list` leaf, + whose correct layout is `Elements` with recursive inner-pointer + fixup, collapsed to flat `Bulk`, dropping the fixup so the callee + dereferences pointers still addressing the source memory (the LS-R-2 + misclassification class, on the conditional path). New + `collect_pointer_positions_with_layout` / + `collect_pointer_byte_offsets_with_layout` helpers carry each + String/List leaf's own `CopyLayout` alongside its position; the now + dead `copy_layout_for_string_or_list_at` shim is removed. **A + confirmed Mythos finding** — surfaced by the mythos-auto delta-pass + on PR #179, promoted to approved loss scenario **LS-P-7**; regression + pinned by + `ls_p_7_conditional_pointer_layout_is_per_leaf_not_per_composite`. + +- **Area-size accumulators wrapped, undoing `canonical_abi_size_unpadded` + saturation** (LS-P-6, UCA-P-3, H-2 / H-4 / H-4.1, + `meld-core/src/parser.rs`). `params_area_byte_size` and + `return_area_byte_size` accumulated each field with a bare + `size += canonical_abi_size_unpadded(ty)`. `canonical_abi_size_unpadded` + saturates to `u32::MAX` for a pathological `fixed-length-list` + (LS-P-4), but the LS-P-4 fix did not reach these two cross-field + accumulators: once a first field saturated `size`, the next field's + `+=` overflowed — a debug build panics, a release build wraps + `u32::MAX` down to a small value. The resolver stores that wrapped + value as `params_area_byte_size`; the FACT adapter passes it to + `cabi_realloc`, under-allocating the params buffer and then writing + every parameter into it — an OOB write into callee memory. The + sibling Record/Tuple accumulators inside `canonical_abi_size_unpadded` + already used `saturating_add`; these two were missed. Both `+=` + sites are now `size.saturating_add(...)`. **A confirmed Mythos + finding** — surfaced by the mythos-auto delta-pass on PR #179, + validated with a panicking PoC, promoted to approved loss scenario + **LS-P-6**; regression pinned by + `ls_p_6_area_byte_size_saturates_across_fields`. + +- **`flat_byte_size` underestimated `result`/`variant` flat width** + (`meld-core/src/parser.rs`). Surfaced by the mythos-auto delta-pass + on PR #178: `flat_byte_size` computed the payload of `result` + and `variant` as `max(flat_byte_size(arm))` rather than the + Component Model's element-wise `flatten_variant` JOIN. When two + arms flatten to a different *number* of core values, `max` of byte + totals underestimates — `result` returned 12 where + the joined sequence `[i32, i64, i32]` is 16. Rewrites + `flat_byte_size` over a new `flat_width_list` helper that + materialises each type's flat core-value width list and JOINs + variant arms element-wise; non-variant types are unchanged. The + helper caps its list length (`FLAT_WIDTH_CAP`) so a pathological + nested `fixed-length-list` still saturates to `u32::MAX` rather + than allocating an unbounded `Vec` (preserves the LS-P-4 + saturation contract). This is correctness hygiene on a `pub fn`: + the mythos-auto finding's claimed OOB-write impact was rejected on + validation — `flat_byte_size` has no in-tree consumers, so there + is no reachable hazard and no LS-N entry; a future consumer would + nonetheless have inherited the wrong value. + ### Added - **Regression guard for the `parse_core_module` section-range diff --git a/meld-core/src/adapter/fact.rs b/meld-core/src/adapter/fact.rs index 2c96a80..fc5cd4c 100644 --- a/meld-core/src/adapter/fact.rs +++ b/meld-core/src/adapter/fact.rs @@ -101,6 +101,113 @@ pub(crate) fn emit_overflow_guard(body: &mut Function, len_local: u32, k: u32) { body.instruction(&Instruction::End); } +/// Emit a chain of `(disc == value)` checks for a [`ConditionalPointerPair`]'s +/// guards on the **flat-local** path — every guard in `cond.outer_guards` +/// followed by the pair's innermost discriminant. All checks are ANDed; the +/// resulting i32 (0/1) is left on the wasm stack so the caller can immediately +/// emit `If`. `flat_base` is added to every guard's `position` (use 0 for +/// param paths, `result_save_base` for the flat-result path). +/// +/// Without this AND-chain, a `ConditionalPointerPair` inside a nested +/// option/result/variant payload would fire whenever the *inner* discriminant +/// slot happens to read as the inner value — *even in arms where the payload +/// type is something else entirely* — yielding an arbitrary cross-memory copy +/// with attacker-controlled `(ptr, len)` (LS-P-10). +pub(crate) fn emit_conditional_guard_chain_flat( + body: &mut Function, + cond: &crate::resolver::ConditionalPointerPair, + flat_base: u32, +) { + let mut emitted_any = false; + for guard in &cond.outer_guards { + body.instruction(&Instruction::LocalGet(flat_base + guard.position)); + body.instruction(&Instruction::I32Const(guard.value as i32)); + body.instruction(&Instruction::I32Eq); + if emitted_any { + body.instruction(&Instruction::I32And); + } + emitted_any = true; + } + // Innermost (current-level) guard. + body.instruction(&Instruction::LocalGet( + flat_base + cond.discriminant_position, + )); + body.instruction(&Instruction::I32Const(cond.discriminant_value as i32)); + body.instruction(&Instruction::I32Eq); + if emitted_any { + body.instruction(&Instruction::I32And); + } +} + +/// Emit a chain of `(disc == value)` checks for a [`ConditionalPointerPair`]'s +/// guards on the **byte-offset** path — every guard in `cond.outer_guards` +/// followed by the innermost discriminant, all ANDed. `base_ptr_local` holds +/// the pointer to the buffer (callee result area, etc.); each guard's +/// `byte_size` selects the load width (`I32Load8U` / `I32Load16U` / +/// `I32Load`). See [`emit_conditional_guard_chain_flat`] for the LS-P-10 +/// rationale. +pub(crate) fn emit_conditional_guard_chain_byte( + body: &mut Function, + cond: &crate::resolver::ConditionalPointerPair, + base_ptr_local: u32, + memory_index: u32, +) { + fn emit_disc_load( + body: &mut Function, + base_ptr_local: u32, + byte_size: u32, + offset: u32, + memory_index: u32, + ) { + body.instruction(&Instruction::LocalGet(base_ptr_local)); + match byte_size { + 1 => body.instruction(&Instruction::I32Load8U(wasm_encoder::MemArg { + offset: offset as u64, + align: 0, + memory_index, + })), + 2 => body.instruction(&Instruction::I32Load16U(wasm_encoder::MemArg { + offset: offset as u64, + align: 1, + memory_index, + })), + _ => body.instruction(&Instruction::I32Load(wasm_encoder::MemArg { + offset: offset as u64, + align: 2, + memory_index, + })), + }; + } + let mut emitted_any = false; + for guard in &cond.outer_guards { + emit_disc_load( + body, + base_ptr_local, + guard.byte_size, + guard.position, + memory_index, + ); + body.instruction(&Instruction::I32Const(guard.value as i32)); + body.instruction(&Instruction::I32Eq); + if emitted_any { + body.instruction(&Instruction::I32And); + } + emitted_any = true; + } + emit_disc_load( + body, + base_ptr_local, + cond.discriminant_byte_size, + cond.discriminant_position, + memory_index, + ); + body.instruction(&Instruction::I32Const(cond.discriminant_value as i32)); + body.instruction(&Instruction::I32Eq); + if emitted_any { + body.instruction(&Instruction::I32And); + } +} + /// Compute Canonical ABI (size, alignment) in bytes for a component value type. /// /// Per Component Model Canonical ABI spec, every type has a fixed lowered @@ -291,11 +398,23 @@ fn emit_patch_nested_indirections( body.instruction(&Instruction::LocalGet(l_rec_src)); body.instruction(&Instruction::I32Load(src_mem_arg_len)); + body.instruction(&Instruction::LocalSet(l_buf_len)); + // LS-P-14: trap if `len * sub_elem_size` would wrap mod 2^32. Without + // this guard a callee-supplied `len` near `u32::MAX / sub_elem_size` + // wraps `buf_len` to a small value; the bounds check below uses + // `old_ptr + buf_len > mem_bytes` (also `i32.add`-based and wrapping) + // and is bypassed; the adapter then under-allocates the caller + // buffer and under-copies the inner list contents while the caller- + // side bulk-copy of the outer (ptr, len) keeps the original large + // `len` — silent truncation and semantic drift between the fused + // and composed executions. + emit_overflow_guard(body, l_buf_len, *sub_elem_size); if *sub_elem_size != 1 { + body.instruction(&Instruction::LocalGet(l_buf_len)); body.instruction(&Instruction::I32Const(*sub_elem_size as i32)); body.instruction(&Instruction::I32Mul); + body.instruction(&Instruction::LocalSet(l_buf_len)); } - body.instruction(&Instruction::LocalSet(l_buf_len)); // Skip patch if (old_ptr, buf_len) doesn't fit in callee mem — guards // against garbage values triggering an unrecoverable trap. @@ -1571,7 +1690,6 @@ impl FactStyleGenerator { // NOTE: We handle this by modifying the params in-place using // local.set on the original param slots. for cond in &site.requirements.conditional_pointer_pairs { - let disc_local = cond.discriminant_position; let ptr_local = cond.ptr_position; let len_local = cond.ptr_position + 1; let byte_mult = match &cond.copy_layout { @@ -1579,10 +1697,11 @@ impl FactStyleGenerator { crate::resolver::CopyLayout::Elements { element_size, .. } => *element_size, }; - // if (disc == expected_value) { copy and replace ptr } - func.instruction(&Instruction::LocalGet(disc_local)); - func.instruction(&Instruction::I32Const(cond.discriminant_value as i32)); - func.instruction(&Instruction::I32Eq); + // if (all guards in chain hold) { copy and replace ptr } + // — outer-guard ANDing matters for nested option/result/variant + // payloads where reading only the inner disc byte could be + // sampling unrelated payload bytes in another arm (LS-P-10). + emit_conditional_guard_chain_flat(&mut func, cond, 0); func.instruction(&Instruction::If(wasm_encoder::BlockType::Empty)); // Allocate: new_ptr = cabi_realloc(0, 0, 1, len * byte_mult) @@ -1722,7 +1841,6 @@ impl FactStyleGenerator { if needs_conditional_result_copy && result_count > 0 { let caller_realloc = options.caller_realloc.unwrap(); for cond in &site.requirements.conditional_result_flat_pairs { - let disc_local = result_save_base + cond.discriminant_position; let ptr_local = result_save_base + cond.ptr_position; let len_local = result_save_base + cond.ptr_position + 1; let byte_mult = match &cond.copy_layout { @@ -1730,10 +1848,9 @@ impl FactStyleGenerator { crate::resolver::CopyLayout::Elements { element_size, .. } => *element_size, }; - // if (disc == expected_value) { allocate in caller, copy, replace ptr } - func.instruction(&Instruction::LocalGet(disc_local)); - func.instruction(&Instruction::I32Const(cond.discriminant_value as i32)); - func.instruction(&Instruction::I32Eq); + // if (all guards in chain hold) { allocate in caller, + // copy, replace ptr } — outer-guard ANDing per LS-P-10. + emit_conditional_guard_chain_flat(&mut func, cond, result_save_base); func.instruction(&Instruction::If(wasm_encoder::BlockType::Empty)); // Allocate in caller memory @@ -2288,7 +2405,6 @@ impl FactStyleGenerator { // --- Phase 1b: Conditional param copy (option/result/variant params) --- if let Some(callee_realloc) = options.callee_realloc.filter(|_| has_cond_param_pairs) { for cond in &site.requirements.conditional_pointer_pairs { - let disc_local = cond.discriminant_position; let ptr_local = cond.ptr_position; let len_local = cond.ptr_position + 1; let byte_mult = match &cond.copy_layout { @@ -2296,9 +2412,8 @@ impl FactStyleGenerator { crate::resolver::CopyLayout::Elements { element_size, .. } => *element_size, }; - func.instruction(&Instruction::LocalGet(disc_local)); - func.instruction(&Instruction::I32Const(cond.discriminant_value as i32)); - func.instruction(&Instruction::I32Eq); + // Outer-guard AND-chain (LS-P-10) before firing the copy. + emit_conditional_guard_chain_flat(&mut func, cond, 0); func.instruction(&Instruction::If(wasm_encoder::BlockType::Empty)); // Allocate in callee memory @@ -2539,7 +2654,6 @@ impl FactStyleGenerator { // --- Phase 5b: Conditional result copy (option/result/variant in return area) --- if let Some(caller_realloc) = options.caller_realloc.filter(|_| has_cond_result_pairs) { for cond in &site.requirements.conditional_result_pointer_pairs { - let disc_offset = cond.discriminant_position; let ptr_offset = cond.ptr_position; let len_offset = cond.ptr_position + 4; let byte_mult = match &cond.copy_layout { @@ -2547,27 +2661,15 @@ impl FactStyleGenerator { crate::resolver::CopyLayout::Elements { element_size, .. } => *element_size, }; - // Load discriminant from callee's return area using correct byte width - func.instruction(&Instruction::LocalGet(result_ptr_local)); - match cond.discriminant_byte_size { - 1 => func.instruction(&Instruction::I32Load8U(wasm_encoder::MemArg { - offset: disc_offset as u64, - align: 0, - memory_index: options.callee_memory, - })), - 2 => func.instruction(&Instruction::I32Load16U(wasm_encoder::MemArg { - offset: disc_offset as u64, - align: 1, - memory_index: options.callee_memory, - })), - _ => func.instruction(&Instruction::I32Load(wasm_encoder::MemArg { - offset: disc_offset as u64, - align: 2, - memory_index: options.callee_memory, - })), - }; - func.instruction(&Instruction::I32Const(cond.discriminant_value as i32)); - func.instruction(&Instruction::I32Eq); + // Outer-guard AND-chain (LS-P-10) before firing the copy. + // Each guard's discriminant is loaded from the callee's + // return area at its byte offset using the matching width. + emit_conditional_guard_chain_byte( + &mut func, + cond, + result_ptr_local, + options.callee_memory, + ); func.instruction(&Instruction::If(wasm_encoder::BlockType::Empty)); // Load ptr and len from callee's return area @@ -3068,6 +3170,20 @@ impl FactStyleGenerator { func.instruction(&Instruction::I32LtU); func.instruction(&Instruction::If(wasm_encoder::BlockType::Empty)); { + // LS-P-19: trap when the 2-byte sequence's continuation byte + // would be read past the end of the input — without this + // guard a truncated 2-byte lead at the buffer tail reads + // 1 byte of attacker-adjacent caller memory and folds it + // into the encoded code point. + func.instruction(&Instruction::LocalGet(src_idx_local)); + func.instruction(&Instruction::I32Const(1)); + func.instruction(&Instruction::I32Add); + func.instruction(&Instruction::LocalGet(1)); + func.instruction(&Instruction::I32GeU); + func.instruction(&Instruction::If(wasm_encoder::BlockType::Empty)); + func.instruction(&Instruction::Unreachable); + func.instruction(&Instruction::End); + // cp = (byte & 0x1F) << 6 | (b1 & 0x3F) func.instruction(&Instruction::LocalGet(byte_local)); func.instruction(&Instruction::I32Const(0x1F)); @@ -3098,6 +3214,20 @@ impl FactStyleGenerator { func.instruction(&Instruction::I32LtU); func.instruction(&Instruction::If(wasm_encoder::BlockType::Empty)); { + // LS-P-19: trap when the 3-byte sequence's continuation + // bytes would extend past the end of the input — a + // truncated 3-byte lead at the buffer tail would + // otherwise read up to 2 bytes of caller memory past + // the buffer and incorporate them into the code point. + func.instruction(&Instruction::LocalGet(src_idx_local)); + func.instruction(&Instruction::I32Const(2)); + func.instruction(&Instruction::I32Add); + func.instruction(&Instruction::LocalGet(1)); + func.instruction(&Instruction::I32GeU); + func.instruction(&Instruction::If(wasm_encoder::BlockType::Empty)); + func.instruction(&Instruction::Unreachable); + func.instruction(&Instruction::End); + // cp = (byte & 0x0F) << 12 | (b1 & 0x3F) << 6 | (b2 & 0x3F) func.instruction(&Instruction::LocalGet(byte_local)); func.instruction(&Instruction::I32Const(0x0F)); @@ -3136,6 +3266,20 @@ impl FactStyleGenerator { func.instruction(&Instruction::Else); { // 4-byte sequence (byte >= 0xF0) + // LS-P-19: trap when the 4-byte sequence's continuation + // bytes would extend past the end of the input — without + // this guard a truncated 4-byte lead at the buffer tail + // reads up to 3 bytes of attacker-adjacent caller memory + // and folds them into the encoded code point. + func.instruction(&Instruction::LocalGet(src_idx_local)); + func.instruction(&Instruction::I32Const(3)); + func.instruction(&Instruction::I32Add); + func.instruction(&Instruction::LocalGet(1)); + func.instruction(&Instruction::I32GeU); + func.instruction(&Instruction::If(wasm_encoder::BlockType::Empty)); + func.instruction(&Instruction::Unreachable); + func.instruction(&Instruction::End); + // cp = (byte & 0x07) << 18 | (b1 & 0x3F) << 12 | (b2 & 0x3F) << 6 | (b3 & 0x3F) func.instruction(&Instruction::LocalGet(byte_local)); func.instruction(&Instruction::I32Const(0x07)); @@ -3380,6 +3524,26 @@ impl FactStyleGenerator { func.instruction(&Instruction::I32And); func.instruction(&Instruction::If(wasm_encoder::BlockType::Empty)); { + // LS-P-16: trap when a lone high surrogate sits at the last code + // unit of the input — without this guard the `I32Load16U` below + // reads `mem16[ptr + (src_idx + 1) * 2]`, i.e. 2 bytes past the + // caller-supplied UTF-16 buffer. Those bytes (adjacent caller + // linear memory) then get treated as the "low surrogate" and + // encoded into a 4-byte UTF-8 sequence in the callee output — + // a silent leak of attacker-adjacent caller memory across the + // component boundary. Trap is the conservative mitigation; the + // Canonical-ABI-correct behaviour is to replace the lone + // surrogate with U+FFFD (3-byte UTF-8 EF BF BD), which is a + // structural follow-up to this guard. + func.instruction(&Instruction::LocalGet(src_idx_local)); + func.instruction(&Instruction::I32Const(1)); + func.instruction(&Instruction::I32Add); + func.instruction(&Instruction::LocalGet(1)); + func.instruction(&Instruction::I32GeU); + func.instruction(&Instruction::If(wasm_encoder::BlockType::Empty)); + func.instruction(&Instruction::Unreachable); + func.instruction(&Instruction::End); + // Surrogate pair: read low surrogate // cu2 = mem16[ptr + (src_idx + 1) * 2] // code_point = 0x10000 + ((cu - 0xD800) << 10) + (cu2 - 0xDC00) @@ -4245,47 +4409,27 @@ impl FactStyleGenerator { callee_param_count: usize, scratch_local: u32, ) { - // The pointer_pair_positions from the resolver are in CALLEE - // component type order. But the adapter's locals are in CALLER - // order (from the caller's canon lower). These may differ if the - // component type reorders params. - // - // Instead of using the resolver's positions, compute positions from - // the caller's flat param types: find (i32, i32) pairs that could - // be (ptr, len) strings/lists. + // `pointer_pair_positions` from the resolver are flat indices into + // the component-type param list, computed by + // `pointer_pair_param_positions` walking the function's params with + // `flat_count`. Canonical lowering preserves param order, so those + // flat indices apply equally to the caller's lowered param locals + // (the retptr the adapter inserts for results is appended after the + // component-type params, so it never collides with a position from + // this slice). LS-P-13: the previous code walked `caller_type.params` + // looking for consecutive `(i32, i32)` slots and joined each with + // `pointer_pair_positions.iter().any(|_| true)` — semantically + // `!is_empty()`. Every adjacent integer-pair argument was then + // treated as a (ptr, len) string/list and rewritten via + // `cabi_realloc` + `memory.copy`, corrupting plain integer args at + // the callee. Replaces that with the resolver's positions directly. + let _ = (caller_type, caller_param_count, callee_param_count); let callee_realloc = crate::merger::component_realloc_index(merged, site.to_component); let callee_memory = crate::merger::component_memory_index(merged, site.to_component); let caller_memory = crate::merger::component_memory_index(merged, site.from_component); let caller_ptr_positions: Vec = if site.crosses_memory && callee_realloc.is_some() { - let params = &caller_type.params; - let has_retptr = - caller_type.results.is_empty() && caller_param_count > callee_param_count; - let effective_len = if has_retptr { - params.len() - 1 - } else { - params.len() - }; - let mut positions = Vec::new(); - let mut i = 0; - while i + 1 < effective_len { - // A (i32, i32) pair is a candidate pointer/length pair; the - // resolver confirms via component-type info. - if params[i] == wasm_encoder::ValType::I32 - && params[i + 1] == wasm_encoder::ValType::I32 - && site - .requirements - .pointer_pair_positions - .iter() - .any(|_| true) - { - positions.push(i as u32); - i += 2; - continue; - } - i += 1; - } - positions + site.requirements.pointer_pair_positions.clone() } else { Vec::new() }; @@ -4862,6 +5006,186 @@ mod tests { assert!(!options.needs_transcoding()); } + /// LS-P-19 — `emit_utf8_to_utf16_transcode` must guard against reading + /// continuation bytes past the end of the input buffer for truncated + /// multi-byte UTF-8 sequences. + /// + /// The mirror of LS-P-16 on the UTF-8→UTF-16 direction. Before the fix, + /// each multi-byte branch (2-byte, 3-byte, 4-byte) unconditionally read + /// the continuation bytes at `src_idx + 1`, `+2`, `+3` via `I32Load8U` + /// without checking against `input_len`. A UTF-8 string ending on a + /// truncated multi-byte lead byte (e.g. `0xE2` with no following bytes) + /// caused the adapter to load 1–3 bytes of attacker-adjacent caller + /// memory, incorporate them into a code point, and emit the result + /// as UTF-16 in the callee — silent cross-memory leak per + /// transcoded string. The composed (non-fused) execution traps on + /// invalid UTF-8; the fused execution silently corrupted, producing + /// semantic drift. + /// + /// Conservative mitigation: prepend a `src_idx + N >= input_len` trap + /// to each multi-byte branch (N = 1/2/3 for the 2/3/4-byte case). The + /// Canonical-ABI-correct upgrade replaces truncated sequences with + /// U+FFFD, tracked alongside LS-P-16's upgrade. + /// + /// Pinned structurally: the LS-P-19 marker must appear at all three + /// branches and Unreachable + I32GeU opcodes must precede each + /// `I32Load8U` continuation-byte read. + #[test] + fn ls_p_19_utf8_to_utf16_continuation_byte_oob_guard_emitted() { + const SRC: &str = include_str!("fact.rs"); + let marker = "LS-P-19: trap when the"; + let marker_count = SRC.matches(marker).count(); + assert!( + marker_count >= 3, + "fact.rs must retain LS-P-19 markers for all three multi-byte \ + branches (2-byte, 3-byte, 4-byte) in \ + emit_utf8_to_utf16_transcode; found {marker_count}", + ); + // Cross-cutting: at least 3 Unreachable + 3 I32GeU pairs from + // LS-P-19's branch guards, plus the original LS-P-16 pair from + // emit_utf16_to_utf8_transcode = at least 4 instances of each. + let unreachable_count = SRC.matches("Instruction::Unreachable").count(); + assert!( + unreachable_count >= 4, + "expected at least 4 Instruction::Unreachable in fact.rs \ + (LS-P-16 + LS-P-19 branch guards); found {unreachable_count}", + ); + } + + /// LS-P-16 — `emit_utf16_to_utf8_transcode` must guard against reading the + /// low-surrogate code unit out-of-bounds when the input ends on a lone + /// high surrogate. + /// + /// Before the fix the surrogate-pair `If` arm unconditionally emitted a + /// second `I32Load16U` at `mem16[ptr + (src_idx + 1) * 2]`. For an + /// input whose last code unit is a high surrogate, that load is 2 bytes + /// past the caller-supplied buffer end. The 2 bytes (attacker-adjacent + /// caller linear memory) are then treated as the "low surrogate" and + /// encoded into a 4-byte UTF-8 sequence in the callee — silent cross- + /// memory leak per UTF-16→UTF-8 transcoded string. Surfaced by the + /// mythos-auto delta-pass on PR #179. Conservative mitigation traps + /// (`unreachable`) when `src_idx + 1 >= input_len`. The full + /// Canonical-ABI-correct behaviour replaces the lone surrogate with + /// U+FFFD (3-byte UTF-8 `EF BF BD`) — that upgrade is structural + /// follow-up; this commit's guard converts silent leak into loud trap. + /// + /// This test pins the structural invariant: the LS-P-16 marker comment + /// must remain in the source AND the surrogate-pair `If` arm must emit + /// an `Unreachable` instruction *before* the second `I32Load16U`. A + /// refactor that removes the comment OR drops the `Unreachable` is + /// caught here. + #[test] + fn ls_p_16_utf16_lone_high_surrogate_oob_guard_emitted() { + const SRC: &str = include_str!("fact.rs"); + let marker = "LS-P-16: trap when a lone high surrogate"; + let pos = SRC.find(marker).unwrap_or_else(|| { + panic!( + "fact.rs must retain the LS-P-16 marker `{marker}`; \ + without the guard the surrogate-pair If arm reads 2 \ + bytes past the UTF-16 input buffer", + ) + }); + // Look only within the guard block + the immediate next ~2000 bytes + // (the surrogate-pair body that ends with the next "Surrogate pair: + // read low surrogate" comment marker). + let after = &SRC[pos..]; + let end = after + .find("Surrogate pair: read low surrogate") + .unwrap_or(after.len()) + .min(3000); + let block = &after[..end]; + assert!( + block.contains("Unreachable"), + "LS-P-16 guard must emit Instruction::Unreachable inside the \ + surrogate-pair If arm before the second I32Load16U; the \ + segment between the LS-P-16 marker and the second-load \ + comment is missing the Unreachable opcode", + ); + // Also require the bounds check shape: src_idx + 1 >= input_len. + // The exact wasm-encoder pattern is LocalGet(src_idx_local), + // I32Const(1), I32Add, LocalGet(1), I32GeU. Match on the textual + // representation. + assert!( + block.contains("I32GeU"), + "LS-P-16 guard must include an I32GeU comparison against \ + input_len", + ); + } + + /// LS-P-14 — the nested-list inner copy in `emit_patch_nested_indirections` + /// must guard `len * sub_elem_size` against 32-bit overflow before + /// computing `buf_len` for `cabi_realloc` + `memory.copy`. + /// + /// Before the fix, the inner copy path loaded the callee-supplied `len` + /// from callee memory, multiplied by `sub_elem_size` with a bare + /// `i32.mul` (wrapping mod 2³²), stored to `l_buf_len`, then used + /// `l_buf_len` for the buffer allocation and `memory.copy`. A callee- + /// controlled `len` near `u32::MAX / sub_elem_size` wrapped `buf_len` + /// to a small value; the subsequent `old_ptr + buf_len > mem_bytes` + /// bounds check used `i32.add` which also wrapped and was bypassed; + /// the adapter then under-allocated and under-copied the inner list + /// while the caller's bulk-copied outer `(ptr, len)` retained the + /// original large `len` — silent truncation. Surfaced by the mythos- + /// auto delta-pass on PR #179. Fix calls `emit_overflow_guard(body, + /// l_buf_len, sub_elem_size as u32)` after loading `len` and before + /// the multiplication. + /// + /// This test pins the contract by emitting the function via + /// `emit_patch_nested_indirections` for an element type that has an + /// inner `list` indirection (`sub_elem_size = 4`, non-trivial), + /// extracts the encoded body bytes, and asserts they contain at least + /// one `Unreachable` opcode (`0x00`) — the only place that opcode is + /// emitted along this path is inside `emit_overflow_guard`. + #[test] + fn ls_p_14_nested_list_inner_copy_emits_overflow_guard() { + use crate::parser::{ComponentValType, PrimitiveValType}; + use wasm_encoder::{Function, ValType}; + + // Element type: `record { items: list }`. The record has one + // inner pointer pair (the list) with sub_elem_size = element_size + // of u32 = 4. That's the dangerous `len * 4` multiplication path. + let elem_ty = ComponentValType::Record(vec![( + "items".to_string(), + ComponentValType::List(Box::new(ComponentValType::Primitive(PrimitiveValType::U32))), + )]); + + // Function with enough locals to cover l_first_scratch..+5 plus a + // few spare. Six i32 locals starting at index 0. + let mut func = Function::new(vec![(12, ValType::I32)]); + + // The dst/src/len locals and the scratch base are all i32 indices + // we just point at slots 0..11 — the function body only cares + // about local references being in-range. + emit_patch_nested_indirections( + &mut func, &elem_ty, /* l_dst_ptr = */ 0, /* l_callee_src = */ 1, + /* l_src_len = */ 2, + /* elem_size = */ 8, // record { ptr:i32, len:i32 } = 8 bytes + /* l_first_scratch = */ 3, /* realloc_func = */ 99, + /* caller_memory = */ 0, /* callee_memory = */ 1, + ); + + // wasm_encoder::Function has no public bytes() accessor on stable; + // round-trip through a Module to get encoded bytes we can scan. + func.instruction(&wasm_encoder::Instruction::End); + let mut module = wasm_encoder::Module::new(); + let mut code_section = wasm_encoder::CodeSection::new(); + code_section.function(&func); + module.section(&code_section); + let body_bytes: Vec = module.finish(); + + // Unreachable opcode is 0x00; it only appears in + // emit_overflow_guard along this path. Without the fix, no + // Unreachable is emitted by the inner copy and the buf_len + // computation wraps silently. + assert!( + body_bytes.contains(&0x00), + "emit_patch_nested_indirections must emit an Unreachable \ + (LS-P-14 overflow guard) for the inner len * sub_elem_size \ + multiplication; before the fix, no Unreachable was emitted \ + and a callee-controlled len could wrap buf_len silently", + ); + } + #[test] fn test_adapter_options_needs_transcoding() { let options = AdapterOptions { diff --git a/meld-core/src/error.rs b/meld-core/src/error.rs index 1e3b920..fb9da69 100644 --- a/meld-core/src/error.rs +++ b/meld-core/src/error.rs @@ -85,6 +85,24 @@ pub enum Error { module_idx: u32, }, + /// Two core modules within one component export the same flat name. + /// In the flat-name resolution path the previous code silently + /// overwrote the first module's entry with the second (last-writer + /// wins), routing any import of that name to the wrong module and + /// violating the semantic-preservation invariant. The instance-graph + /// path is not vulnerable, so this error is only reachable for + /// components without an `InstanceSection`; rejecting the collision + /// makes the violation loud instead of silent (LS-P-11). + #[error( + "component {component_idx}: core modules {first_module_idx} and {second_module_idx} both export `{export_name}` in the flat-name resolver path; ambiguous, and the previous behavior silently picked the latter" + )] + DuplicateModuleExport { + component_idx: usize, + export_name: String, + first_module_idx: usize, + second_module_idx: usize, + }, + /// P3 async component model features are not yet supported #[error("P3 async component features not supported: {0}")] P3AsyncNotSupported(String), diff --git a/meld-core/src/parser.rs b/meld-core/src/parser.rs index 1d2ecba..df0c79a 100644 --- a/meld-core/src/parser.rs +++ b/meld-core/src/parser.rs @@ -1421,58 +1421,155 @@ impl ParsedComponent { /// In the canonical ABI, complex types are "flattened" to sequences of core wasm values. /// This returns the total byte size of that flat representation, used for sizing /// return area buffers in the retptr calling convention. + /// Total byte size of a type's flat (core wasm) representation — + /// the sum of the byte widths of each core value in its + /// canonical-ABI flattening. + /// + /// For `result` and `variant`, the payload is the **element- + /// wise JOIN** of the arms' flat width lists, not `max` of their + /// byte totals. When two arms flatten to a different *number* of + /// core values, `max` of totals underestimates: for + /// `result` the `ok` arm `u64` flattens to `[i64]` + /// (8 B) and the `err` arm `string` to `[i32, i32]` (8 B), so + /// `max` gives `4 + 8 = 12` — but the joined payload is + /// `[i64, i32]` (12 B) and the true flat size is `4 + 12 = 16`. + /// + /// Returns `u32::MAX` for pathologically large types (e.g. nested + /// `fixed-length-list` whose flattening exceeds `FLAT_WIDTH_CAP`). + /// Such a type is never flat-passed — the canonical ABI memory- + /// passes anything over `MAX_FLAT_PARAMS = 16` — so its flat byte + /// size is not a meaningful quantity; a saturated value fails + /// downstream allocation safely rather than wrapping (cf. LS-P-4). pub fn flat_byte_size(&self, ty: &ComponentValType) -> u32 { - match ty { + match self.flat_width_list(ty) { + Some(widths) => widths.iter().copied().fold(0u32, u32::saturating_add), + None => u32::MAX, + } + } + + /// The flat (core wasm) value-width list of a type — one entry per + /// core value, each 4 or 8 bytes. `None` if the list would exceed + /// `FLAT_WIDTH_CAP`, which only happens for a pathologically large + /// type that the canonical ABI never flat-passes anyway. + /// + /// `variant` / `result` payloads are the element-wise JOIN of their + /// arms: the joined list has the longest arm's length, and each + /// position takes the wider of the arms' widths (a position present + /// in only one arm takes that arm's width). + fn flat_width_list(&self, ty: &ComponentValType) -> Option> { + // Far beyond the canonical-ABI flat limit (MAX_FLAT_PARAMS = 16). + // A flat list longer than this belongs to a memory-passed type + // whose flat byte size is not a meaningful quantity; bailing + // here also bounds the Vec against nested `fixed-length-list` + // blow-up (the LS-P-4 OOM class). + const FLAT_WIDTH_CAP: usize = 256; + + // Element-wise JOIN of two flat width lists. The joined list is + // as long as the longer input; a position only one list reaches + // takes that list's width (`max` with the absent side's 0). + fn join(a: &[u32], b: &[u32]) -> Vec { + let n = a.len().max(b.len()); + (0..n) + .map(|i| { + a.get(i) + .copied() + .unwrap_or(0) + .max(b.get(i).copied().unwrap_or(0)) + }) + .collect() + } + + let widths: Vec = match ty { ComponentValType::Primitive(p) => match p { - PrimitiveValType::S64 | PrimitiveValType::U64 | PrimitiveValType::F64 => 8, - _ => 4, + PrimitiveValType::S64 | PrimitiveValType::U64 | PrimitiveValType::F64 => vec![8], + _ => vec![4], }, - ComponentValType::String => 8, // (ptr: i32, len: i32) - ComponentValType::List(_) => 8, // (ptr: i32, len: i32) - ComponentValType::Record(fields) => fields - .iter() - .map(|(_, ty)| self.flat_byte_size(ty)) - .fold(0u32, u32::saturating_add), - ComponentValType::Tuple(elems) => elems - .iter() - .map(|ty| self.flat_byte_size(ty)) - .fold(0u32, u32::saturating_add), - ComponentValType::Option(inner) => 4u32.saturating_add(self.flat_byte_size(inner)), + // (ptr: i32, len: i32) + ComponentValType::String | ComponentValType::List(_) => vec![4, 4], + ComponentValType::Record(fields) => { + let mut w = Vec::new(); + for (_, t) in fields { + w.extend(self.flat_width_list(t)?); + if w.len() > FLAT_WIDTH_CAP { + return None; + } + } + w + } + ComponentValType::Tuple(elems) => { + let mut w = Vec::new(); + for t in elems { + w.extend(self.flat_width_list(t)?); + if w.len() > FLAT_WIDTH_CAP { + return None; + } + } + w + } + ComponentValType::Option(inner) => { + // option = variant { none, some(inner) }; the none arm + // is empty, so the joined payload is just `inner`'s. + let mut w = vec![4]; + w.extend(self.flat_width_list(inner)?); + w + } ComponentValType::Result { ok, err } => { - let ok_size = ok.as_ref().map(|t| self.flat_byte_size(t)).unwrap_or(0); - let err_size = err.as_ref().map(|t| self.flat_byte_size(t)).unwrap_or(0); - 4u32.saturating_add(ok_size.max(err_size)) + let okw = match ok { + Some(t) => self.flat_width_list(t)?, + None => Vec::new(), + }; + let errw = match err { + Some(t) => self.flat_width_list(t)?, + None => Vec::new(), + }; + let mut w = vec![4]; + w.extend(join(&okw, &errw)); + w + } + ComponentValType::Variant(cases) => { + let mut payload: Vec = Vec::new(); + for (_, t) in cases { + if let Some(t) = t { + payload = join(&payload, &self.flat_width_list(t)?); + } + } + let mut w = vec![4]; + w.extend(payload); + w } ComponentValType::Type(idx) => { if let Some(ct) = self.get_type_definition(*idx) { if let ComponentTypeKind::Defined(inner) = &ct.kind { - self.flat_byte_size(inner) + self.flat_width_list(inner)? } else { - 4 + vec![4] } } else { - 4 + vec![4] } } - ComponentValType::Variant(cases) => { - // discriminant + max case payload - let max_payload = cases - .iter() - .filter_map(|(_, ty)| ty.as_ref().map(|t| self.flat_byte_size(t))) - .max() - .unwrap_or(0); - 4u32.saturating_add(max_payload) - } ComponentValType::FixedSizeList(elem, len) => { - self.flat_byte_size(elem).saturating_mul(*len) + let ew = self.flat_width_list(elem)?; + let total = ew.len().checked_mul(*len as usize)?; + if total > FLAT_WIDTH_CAP { + return None; + } + ew.repeat(*len as usize) } ComponentValType::Flags(names) => { - // flats[ceil(N/32)] i32 storage words = 4 bytes each. - let n = names.len() as u32; - 4u32.saturating_mul(n.div_ceil(32)) + // ceil(N/32) i32 storage words, 4 bytes each. + let words = (names.len() as u32).div_ceil(32) as usize; + if words > FLAT_WIDTH_CAP { + return None; + } + vec![4; words] } - ComponentValType::Own(_) | ComponentValType::Borrow(_) => 4, + ComponentValType::Own(_) | ComponentValType::Borrow(_) => vec![4], + }; + if widths.len() > FLAT_WIDTH_CAP { + return None; } + Some(widths) } /// Compute the byte size of the return area for a component function's results. @@ -1484,7 +1581,13 @@ impl ParsedComponent { for (_, ty) in results { let align = self.canonical_abi_align(ty); size = align_up(size, align); - size += self.canonical_abi_size_unpadded(ty); + // saturating_add, not `+=`: canonical_abi_size_unpadded + // saturates to u32::MAX for a pathological fixed-length-list + // (LS-P-4); a bare `+=` on a saturated accumulator then + // overflows — debug panic, release wrap-to-small. The + // wrapped value would size a too-small cabi_realloc buffer + // and the adapter would write OOB (LS-P-6). + size = size.saturating_add(self.canonical_abi_element_size(ty)); } // Align final size to the max alignment of the tuple let max_align = results @@ -1500,7 +1603,17 @@ impl ParsedComponent { /// If this exceeds MAX_FLAT_PARAMS (16), the canonical ABI uses the params-ptr /// calling convention: a single i32 pointer to a buffer in linear memory. pub fn total_flat_params(&self, params: &[(String, ComponentValType)]) -> u32 { - params.iter().map(|(_, ty)| self.flat_count(ty)).sum() + // saturating_add fold, not `sum()`: flat_count is saturating + // (FixedSizeList multiplies element_flat by len, saturating to + // u32::MAX). A bare `sum()` then panics in debug / wraps in + // release on `u32::MAX + 1`; a wrapped small value compares + // `<= MAX_FLAT_PARAMS` and selects the flat calling convention + // for a function that genuinely needs params-ptr — semantic + // divergence in the fused output (LS-P-9). + params + .iter() + .map(|(_, ty)| self.flat_count(ty)) + .fold(0u32, u32::saturating_add) } /// Compute the byte size of the params area for a component function's params. @@ -1513,7 +1626,11 @@ impl ParsedComponent { for (_, ty) in params { let align = self.canonical_abi_align(ty); size = align_up(size, align); - size += self.canonical_abi_size_unpadded(ty); + // saturating_add, not `+=` — see return_area_byte_size + // above and LS-P-6: a bare `+=` on a u32::MAX accumulator + // overflows, wrapping the params buffer size down to a + // small value that under-allocates the cabi_realloc buffer. + size = size.saturating_add(self.canonical_abi_element_size(ty)); } // Align final size to the max alignment of the tuple let max_align = params @@ -1548,7 +1665,7 @@ impl ParsedComponent { let align = self.canonical_abi_align(ty); byte_offset = align_up(byte_offset, align); self.collect_pointer_byte_offsets(ty, byte_offset, &mut offsets); - byte_offset += self.canonical_abi_size_unpadded(ty); + byte_offset += self.canonical_abi_element_size(ty); } offsets } @@ -1568,7 +1685,7 @@ impl ParsedComponent { let align = self.canonical_abi_align(ty); byte_offset = align_up(byte_offset, align); self.collect_return_area_type_slots(ty, byte_offset, &mut slots); - byte_offset += self.canonical_abi_size_unpadded(ty); + byte_offset += self.canonical_abi_element_size(ty); } slots } @@ -1589,7 +1706,7 @@ impl ParsedComponent { let align = self.canonical_abi_align(ty); byte_offset = align_up(byte_offset, align); self.collect_return_area_type_slots(ty, byte_offset, &mut slots); - byte_offset += self.canonical_abi_size_unpadded(ty); + byte_offset += self.canonical_abi_element_size(ty); } slots } @@ -1633,7 +1750,7 @@ impl ParsedComponent { let align = self.canonical_abi_align(field_ty); offset = align_up(offset, align); self.collect_return_area_type_slots(field_ty, offset, out); - offset += self.canonical_abi_size_unpadded(field_ty); + offset += self.canonical_abi_element_size(field_ty); } } ComponentValType::Tuple(elems) => { @@ -1642,7 +1759,7 @@ impl ParsedComponent { let align = self.canonical_abi_align(elem_ty); offset = align_up(offset, align); self.collect_return_area_type_slots(elem_ty, offset, out); - offset += self.canonical_abi_size_unpadded(elem_ty); + offset += self.canonical_abi_element_size(elem_ty); } } ComponentValType::Variant(cases) => { @@ -1896,7 +2013,7 @@ impl ParsedComponent { let align = self.canonical_abi_align(ty); byte_offset = align_up(byte_offset, align); self.collect_resource_byte_positions(ty, byte_offset, &mut positions); - byte_offset += self.canonical_abi_size_unpadded(ty); + byte_offset += self.canonical_abi_element_size(ty); } positions } @@ -1932,7 +2049,7 @@ impl ParsedComponent { let align = self.canonical_abi_align(field_ty); offset = align_up(offset, align); self.collect_resource_byte_positions(field_ty, offset, out); - offset += self.canonical_abi_size_unpadded(field_ty); + offset += self.canonical_abi_element_size(field_ty); } } ComponentValType::Tuple(elems) => { @@ -1941,7 +2058,7 @@ impl ParsedComponent { let align = self.canonical_abi_align(elem_ty); offset = align_up(offset, align); self.collect_resource_byte_positions(elem_ty, offset, out); - offset += self.canonical_abi_size_unpadded(elem_ty); + offset += self.canonical_abi_element_size(elem_ty); } } ComponentValType::Option(inner) => { @@ -2033,7 +2150,7 @@ impl ParsedComponent { }); } flat_idx += self.flat_count(ty); - byte_offset += self.canonical_abi_size_unpadded(ty); + byte_offset += self.canonical_abi_element_size(ty); } positions } @@ -2120,7 +2237,7 @@ impl ParsedComponent { let align = self.canonical_abi_align(ty); byte_offset = align_up(byte_offset, align); self.collect_pointer_byte_offsets(ty, byte_offset, &mut offsets); - byte_offset += self.canonical_abi_size_unpadded(ty); + byte_offset += self.canonical_abi_element_size(ty); } offsets } @@ -2166,6 +2283,55 @@ impl ParsedComponent { } } + /// Like [`Self::collect_pointer_positions`], but also records the + /// [`CopyLayout`](crate::resolver::CopyLayout) for the String/List leaf at + /// each flat position. The layout describes that specific leaf — not the + /// enclosing composite — so a `list` field inside a record yields + /// `Bulk { byte_multiplier: 4 }` rather than the `_ => Bulk { 1 }` fallback + /// `copy_layout` returns when handed a whole record/tuple/fixed-list. + fn collect_pointer_positions_with_layout( + &self, + ty: &ComponentValType, + base: u32, + out: &mut Vec<(u32, crate::resolver::CopyLayout)>, + ) { + match ty { + ComponentValType::String | ComponentValType::List(_) => { + out.push((base, self.copy_layout(ty))); + } + ComponentValType::FixedSizeList(elem, len) => { + let elem_flat = self.flat_count(elem); + let mut offset = base; + for _ in 0..*len { + self.collect_pointer_positions_with_layout(elem, offset, out); + offset += elem_flat; + } + } + ComponentValType::Record(fields) => { + let mut offset = base; + for (_, field_ty) in fields { + self.collect_pointer_positions_with_layout(field_ty, offset, out); + offset += self.flat_count(field_ty); + } + } + ComponentValType::Tuple(elems) => { + let mut offset = base; + for elem_ty in elems { + self.collect_pointer_positions_with_layout(elem_ty, offset, out); + offset += self.flat_count(elem_ty); + } + } + ComponentValType::Type(idx) => { + if let Some(ct) = self.get_type_definition(*idx) + && let ComponentTypeKind::Defined(inner) = &ct.kind + { + self.collect_pointer_positions_with_layout(inner, base, out); + } + } + _ => {} + } + } + /// Collect byte offsets in the return area where pointer pairs start. /// Uses canonical ABI memory layout offsets (with alignment). fn collect_pointer_byte_offsets(&self, ty: &ComponentValType, base: u32, out: &mut Vec) { @@ -2188,7 +2354,7 @@ impl ParsedComponent { let align = self.canonical_abi_align(field_ty); offset = align_up(offset, align); self.collect_pointer_byte_offsets(field_ty, offset, out); - offset += self.canonical_abi_size_unpadded(field_ty); + offset += self.canonical_abi_element_size(field_ty); } } ComponentValType::Tuple(elems) => { @@ -2197,7 +2363,7 @@ impl ParsedComponent { let align = self.canonical_abi_align(elem_ty); offset = align_up(offset, align); self.collect_pointer_byte_offsets(elem_ty, offset, out); - offset += self.canonical_abi_size_unpadded(elem_ty); + offset += self.canonical_abi_element_size(elem_ty); } } ComponentValType::Type(idx) => { @@ -2211,6 +2377,56 @@ impl ParsedComponent { } } + /// Like [`Self::collect_pointer_byte_offsets`], but also records the + /// [`CopyLayout`](crate::resolver::CopyLayout) for the String/List leaf at + /// each byte offset — the layout of that leaf, not the enclosing composite. + fn collect_pointer_byte_offsets_with_layout( + &self, + ty: &ComponentValType, + base: u32, + out: &mut Vec<(u32, crate::resolver::CopyLayout)>, + ) { + match ty { + ComponentValType::String | ComponentValType::List(_) => { + out.push((base, self.copy_layout(ty))); + } + ComponentValType::FixedSizeList(elem, len) => { + let elem_size = self.canonical_abi_element_size(elem); + let mut offset = base; + for _ in 0..*len { + self.collect_pointer_byte_offsets_with_layout(elem, offset, out); + offset += elem_size; + } + } + ComponentValType::Record(fields) => { + let mut offset = base; + for (_, field_ty) in fields { + let align = self.canonical_abi_align(field_ty); + offset = align_up(offset, align); + self.collect_pointer_byte_offsets_with_layout(field_ty, offset, out); + offset += self.canonical_abi_element_size(field_ty); + } + } + ComponentValType::Tuple(elems) => { + let mut offset = base; + for elem_ty in elems { + let align = self.canonical_abi_align(elem_ty); + offset = align_up(offset, align); + self.collect_pointer_byte_offsets_with_layout(elem_ty, offset, out); + offset += self.canonical_abi_element_size(elem_ty); + } + } + ComponentValType::Type(idx) => { + if let Some(ct) = self.get_type_definition(*idx) + && let ComponentTypeKind::Defined(inner) = &ct.kind + { + self.collect_pointer_byte_offsets_with_layout(inner, base, out); + } + } + _ => {} + } + } + /// Collect conditional pointer pairs from params that contain option/result/variant /// types with pointer payloads. pub fn conditional_pointer_pair_positions( @@ -2220,7 +2436,7 @@ impl ParsedComponent { let mut out = Vec::new(); let mut flat_idx = 0u32; for (_, ty) in params { - self.collect_conditional_pointers(ty, flat_idx, &mut out); + self.collect_conditional_pointers(ty, flat_idx, &[], &mut out); flat_idx += self.flat_count(ty); } out @@ -2234,7 +2450,7 @@ impl ParsedComponent { let mut out = Vec::new(); let mut flat_idx = 0u32; for (_, ty) in results { - self.collect_conditional_pointers(ty, flat_idx, &mut out); + self.collect_conditional_pointers(ty, flat_idx, &[], &mut out); flat_idx += self.flat_count(ty); } out @@ -2251,18 +2467,27 @@ impl ParsedComponent { for (_, ty) in results { let align = self.canonical_abi_align(ty); byte_offset = align_up(byte_offset, align); - self.collect_conditional_result_pointers(ty, byte_offset, &mut out); - byte_offset += self.canonical_abi_size_unpadded(ty); + self.collect_conditional_result_pointers(ty, byte_offset, &[], &mut out); + byte_offset += self.canonical_abi_element_size(ty); } out } /// Collect conditional pointer pairs for flat params. /// For option where T contains pointers: disc at base, payload at base+1. + /// + /// `outer_guards` is the chain of enclosing option/result/variant + /// discriminants that must also hold for any pair emitted here to be + /// active — empty at the top level; one entry per nesting level deeper. + /// Without this chain, a `result, u32>` would emit only + /// the inner option-Some guard, and the adapter would copy memory + /// whenever the bytes that happen to occupy the option's discriminant + /// slot (in the Err arm: the u32 payload) read as `1` (LS-P-10). fn collect_conditional_pointers( &self, ty: &ComponentValType, base: u32, + outer_guards: &[crate::resolver::DiscriminantGuard], out: &mut Vec, ) { match ty { @@ -2271,19 +2496,30 @@ impl ParsedComponent { // disc=1 means Some, payload starts at base+1 // In flat representation, discriminant is always i32 (4 bytes) let payload_base = base + 1; + let my_guard = crate::resolver::DiscriminantGuard { + position: base, + value: 1, + byte_size: 4, + }; let mut inner_positions = Vec::new(); - self.collect_pointer_positions(inner, payload_base, &mut inner_positions); - for ptr_pos in inner_positions { - let layout = self.copy_layout_for_string_or_list_at(inner); + self.collect_pointer_positions_with_layout( + inner, + payload_base, + &mut inner_positions, + ); + for (ptr_pos, layout) in inner_positions { out.push(crate::resolver::ConditionalPointerPair { - discriminant_position: base, - discriminant_value: 1, + discriminant_position: my_guard.position, + discriminant_value: my_guard.value, ptr_position: ptr_pos, copy_layout: layout, - discriminant_byte_size: 4, + discriminant_byte_size: my_guard.byte_size, + outer_guards: outer_guards.to_vec(), }); } - self.collect_conditional_pointers(inner, payload_base, out); + let mut nested = outer_guards.to_vec(); + nested.push(my_guard); + self.collect_conditional_pointers(inner, payload_base, &nested, out); } ComponentValType::Result { ok, err } => { // result flattens to [disc, ...max(T_flat, E_flat)] @@ -2292,36 +2528,58 @@ impl ParsedComponent { if let Some(ok_ty) = ok && self.type_contains_pointers(ok_ty) { + let ok_guard = crate::resolver::DiscriminantGuard { + position: base, + value: 0, + byte_size: 4, + }; let mut inner_positions = Vec::new(); - self.collect_pointer_positions(ok_ty, payload_base, &mut inner_positions); - for ptr_pos in inner_positions { - let layout = self.copy_layout_for_string_or_list_at(ok_ty); + self.collect_pointer_positions_with_layout( + ok_ty, + payload_base, + &mut inner_positions, + ); + for (ptr_pos, layout) in inner_positions { out.push(crate::resolver::ConditionalPointerPair { - discriminant_position: base, - discriminant_value: 0, + discriminant_position: ok_guard.position, + discriminant_value: ok_guard.value, ptr_position: ptr_pos, copy_layout: layout, - discriminant_byte_size: 4, + discriminant_byte_size: ok_guard.byte_size, + outer_guards: outer_guards.to_vec(), }); } - self.collect_conditional_pointers(ok_ty, payload_base, out); + let mut nested = outer_guards.to_vec(); + nested.push(ok_guard); + self.collect_conditional_pointers(ok_ty, payload_base, &nested, out); } if let Some(err_ty) = err && self.type_contains_pointers(err_ty) { + let err_guard = crate::resolver::DiscriminantGuard { + position: base, + value: 1, + byte_size: 4, + }; let mut inner_positions = Vec::new(); - self.collect_pointer_positions(err_ty, payload_base, &mut inner_positions); - for ptr_pos in inner_positions { - let layout = self.copy_layout_for_string_or_list_at(err_ty); + self.collect_pointer_positions_with_layout( + err_ty, + payload_base, + &mut inner_positions, + ); + for (ptr_pos, layout) in inner_positions { out.push(crate::resolver::ConditionalPointerPair { - discriminant_position: base, - discriminant_value: 1, + discriminant_position: err_guard.position, + discriminant_value: err_guard.value, ptr_position: ptr_pos, copy_layout: layout, - discriminant_byte_size: 4, + discriminant_byte_size: err_guard.byte_size, + outer_guards: outer_guards.to_vec(), }); } - self.collect_conditional_pointers(err_ty, payload_base, out); + let mut nested = outer_guards.to_vec(); + nested.push(err_guard); + self.collect_conditional_pointers(err_ty, payload_base, &nested, out); } } ComponentValType::Variant(cases) => { @@ -2331,19 +2589,30 @@ impl ParsedComponent { if let Some(ty) = case_ty && self.type_contains_pointers(ty) { + let case_guard = crate::resolver::DiscriminantGuard { + position: base, + value: case_idx as u32, + byte_size: 4, + }; let mut inner_positions = Vec::new(); - self.collect_pointer_positions(ty, payload_base, &mut inner_positions); - for ptr_pos in inner_positions { - let layout = self.copy_layout_for_string_or_list_at(ty); + self.collect_pointer_positions_with_layout( + ty, + payload_base, + &mut inner_positions, + ); + for (ptr_pos, layout) in inner_positions { out.push(crate::resolver::ConditionalPointerPair { - discriminant_position: base, - discriminant_value: case_idx as u32, + discriminant_position: case_guard.position, + discriminant_value: case_guard.value, ptr_position: ptr_pos, copy_layout: layout, - discriminant_byte_size: 4, + discriminant_byte_size: case_guard.byte_size, + outer_guards: outer_guards.to_vec(), }); } - self.collect_conditional_pointers(ty, payload_base, out); + let mut nested = outer_guards.to_vec(); + nested.push(case_guard); + self.collect_conditional_pointers(ty, payload_base, &nested, out); } } } @@ -2351,21 +2620,21 @@ impl ParsedComponent { let elem_flat = self.flat_count(elem); let mut offset = base; for _ in 0..*len { - self.collect_conditional_pointers(elem, offset, out); + self.collect_conditional_pointers(elem, offset, outer_guards, out); offset += elem_flat; } } ComponentValType::Record(fields) => { let mut offset = base; for (_, field_ty) in fields { - self.collect_conditional_pointers(field_ty, offset, out); + self.collect_conditional_pointers(field_ty, offset, outer_guards, out); offset += self.flat_count(field_ty); } } ComponentValType::Tuple(elems) => { let mut offset = base; for elem_ty in elems { - self.collect_conditional_pointers(elem_ty, offset, out); + self.collect_conditional_pointers(elem_ty, offset, outer_guards, out); offset += self.flat_count(elem_ty); } } @@ -2373,7 +2642,7 @@ impl ParsedComponent { if let Some(ct) = self.get_type_definition(*idx) && let ComponentTypeKind::Defined(inner) = &ct.kind { - self.collect_conditional_pointers(inner, base, out); + self.collect_conditional_pointers(inner, base, outer_guards, out); } } _ => {} @@ -2381,10 +2650,16 @@ impl ParsedComponent { } /// Collect conditional pointer pairs for result byte offsets. + /// + /// `outer_guards` is the chain of enclosing option/result/variant + /// discriminants — see `collect_conditional_pointers` for the LS-P-10 + /// rationale; identical semantics, byte-offset addressing instead of + /// flat-local addressing. fn collect_conditional_result_pointers( &self, ty: &ComponentValType, base: u32, + outer_guards: &[crate::resolver::DiscriminantGuard], out: &mut Vec, ) { match ty { @@ -2394,19 +2669,30 @@ impl ParsedComponent { let ds = disc_size(2); let payload_align = self.canonical_abi_align(inner); let payload_offset = base + align_up(ds, payload_align); + let my_guard = crate::resolver::DiscriminantGuard { + position: disc_offset, + value: 1, + byte_size: ds, + }; let mut inner_offsets = Vec::new(); - self.collect_pointer_byte_offsets(inner, payload_offset, &mut inner_offsets); - for ptr_off in inner_offsets { - let layout = self.copy_layout_for_string_or_list_at(inner); + self.collect_pointer_byte_offsets_with_layout( + inner, + payload_offset, + &mut inner_offsets, + ); + for (ptr_off, layout) in inner_offsets { out.push(crate::resolver::ConditionalPointerPair { - discriminant_position: disc_offset, - discriminant_value: 1, + discriminant_position: my_guard.position, + discriminant_value: my_guard.value, ptr_position: ptr_off, copy_layout: layout, - discriminant_byte_size: ds, + discriminant_byte_size: my_guard.byte_size, + outer_guards: outer_guards.to_vec(), }); } - self.collect_conditional_result_pointers(inner, payload_offset, out); + let mut nested = outer_guards.to_vec(); + nested.push(my_guard); + self.collect_conditional_result_pointers(inner, payload_offset, &nested, out); } ComponentValType::Result { ok, err } => { let disc_offset = base; @@ -2424,36 +2710,58 @@ impl ParsedComponent { if let Some(ok_ty) = ok && self.type_contains_pointers(ok_ty) { + let ok_guard = crate::resolver::DiscriminantGuard { + position: disc_offset, + value: 0, + byte_size: ds, + }; let mut inner_offsets = Vec::new(); - self.collect_pointer_byte_offsets(ok_ty, payload_offset, &mut inner_offsets); - for ptr_off in inner_offsets { - let layout = self.copy_layout_for_string_or_list_at(ok_ty); + self.collect_pointer_byte_offsets_with_layout( + ok_ty, + payload_offset, + &mut inner_offsets, + ); + for (ptr_off, layout) in inner_offsets { out.push(crate::resolver::ConditionalPointerPair { - discriminant_position: disc_offset, - discriminant_value: 0, + discriminant_position: ok_guard.position, + discriminant_value: ok_guard.value, ptr_position: ptr_off, copy_layout: layout, - discriminant_byte_size: ds, + discriminant_byte_size: ok_guard.byte_size, + outer_guards: outer_guards.to_vec(), }); } - self.collect_conditional_result_pointers(ok_ty, payload_offset, out); + let mut nested = outer_guards.to_vec(); + nested.push(ok_guard); + self.collect_conditional_result_pointers(ok_ty, payload_offset, &nested, out); } if let Some(err_ty) = err && self.type_contains_pointers(err_ty) { + let err_guard = crate::resolver::DiscriminantGuard { + position: disc_offset, + value: 1, + byte_size: ds, + }; let mut inner_offsets = Vec::new(); - self.collect_pointer_byte_offsets(err_ty, payload_offset, &mut inner_offsets); - for ptr_off in inner_offsets { - let layout = self.copy_layout_for_string_or_list_at(err_ty); + self.collect_pointer_byte_offsets_with_layout( + err_ty, + payload_offset, + &mut inner_offsets, + ); + for (ptr_off, layout) in inner_offsets { out.push(crate::resolver::ConditionalPointerPair { - discriminant_position: disc_offset, - discriminant_value: 1, + discriminant_position: err_guard.position, + discriminant_value: err_guard.value, ptr_position: ptr_off, copy_layout: layout, - discriminant_byte_size: ds, + discriminant_byte_size: err_guard.byte_size, + outer_guards: outer_guards.to_vec(), }); } - self.collect_conditional_result_pointers(err_ty, payload_offset, out); + let mut nested = outer_guards.to_vec(); + nested.push(err_guard); + self.collect_conditional_result_pointers(err_ty, payload_offset, &nested, out); } } ComponentValType::Variant(cases) => { @@ -2469,19 +2777,30 @@ impl ParsedComponent { if let Some(ty) = case_ty && self.type_contains_pointers(ty) { + let case_guard = crate::resolver::DiscriminantGuard { + position: disc_offset, + value: case_idx as u32, + byte_size: ds, + }; let mut inner_offsets = Vec::new(); - self.collect_pointer_byte_offsets(ty, payload_offset, &mut inner_offsets); - for ptr_off in inner_offsets { - let layout = self.copy_layout_for_string_or_list_at(ty); + self.collect_pointer_byte_offsets_with_layout( + ty, + payload_offset, + &mut inner_offsets, + ); + for (ptr_off, layout) in inner_offsets { out.push(crate::resolver::ConditionalPointerPair { - discriminant_position: disc_offset, - discriminant_value: case_idx as u32, + discriminant_position: case_guard.position, + discriminant_value: case_guard.value, ptr_position: ptr_off, copy_layout: layout, - discriminant_byte_size: ds, + discriminant_byte_size: case_guard.byte_size, + outer_guards: outer_guards.to_vec(), }); } - self.collect_conditional_result_pointers(ty, payload_offset, out); + let mut nested = outer_guards.to_vec(); + nested.push(case_guard); + self.collect_conditional_result_pointers(ty, payload_offset, &nested, out); } } } @@ -2489,7 +2808,7 @@ impl ParsedComponent { let elem_size = self.canonical_abi_element_size(elem); let mut offset = base; for _ in 0..*len { - self.collect_conditional_result_pointers(elem, offset, out); + self.collect_conditional_result_pointers(elem, offset, outer_guards, out); offset += elem_size; } } @@ -2498,8 +2817,8 @@ impl ParsedComponent { for (_, field_ty) in fields { let align = self.canonical_abi_align(field_ty); offset = align_up(offset, align); - self.collect_conditional_result_pointers(field_ty, offset, out); - offset += self.canonical_abi_size_unpadded(field_ty); + self.collect_conditional_result_pointers(field_ty, offset, outer_guards, out); + offset += self.canonical_abi_element_size(field_ty); } } ComponentValType::Tuple(elems) => { @@ -2507,15 +2826,15 @@ impl ParsedComponent { for elem_ty in elems { let align = self.canonical_abi_align(elem_ty); offset = align_up(offset, align); - self.collect_conditional_result_pointers(elem_ty, offset, out); - offset += self.canonical_abi_size_unpadded(elem_ty); + self.collect_conditional_result_pointers(elem_ty, offset, outer_guards, out); + offset += self.canonical_abi_element_size(elem_ty); } } ComponentValType::Type(idx) => { if let Some(ct) = self.get_type_definition(*idx) && let ComponentTypeKind::Defined(inner) = &ct.kind { - self.collect_conditional_result_pointers(inner, base, out); + self.collect_conditional_result_pointers(inner, base, outer_guards, out); } } _ => {} @@ -2552,14 +2871,6 @@ impl ParsedComponent { } } - /// Get the copy layout for a type that is either a string or a list. - fn copy_layout_for_string_or_list_at( - &self, - ty: &ComponentValType, - ) -> crate::resolver::CopyLayout { - self.copy_layout(ty) - } - /// Count the number of flat core wasm values a component type flattens to. pub fn flat_count(&self, ty: &ComponentValType) -> u32 { match ty { @@ -2716,7 +3027,7 @@ impl ParsedComponent { for (_, field_ty) in fields { let align = self.canonical_abi_align(field_ty); size = align_up(size, align); - size = size.saturating_add(self.canonical_abi_size_unpadded(field_ty)); + size = size.saturating_add(self.canonical_abi_element_size(field_ty)); } size } @@ -2725,7 +3036,7 @@ impl ParsedComponent { for elem_ty in elems { let align = self.canonical_abi_align(elem_ty); size = align_up(size, align); - size = size.saturating_add(self.canonical_abi_size_unpadded(elem_ty)); + size = size.saturating_add(self.canonical_abi_element_size(elem_ty)); } size } @@ -2815,6 +3126,38 @@ impl ParsedComponent { match ty { ComponentValType::String => CopyLayout::Bulk { byte_multiplier: 1 }, ComponentValType::List(inner) => { + // LS-P-12 / LS-P-18 safety check: refuse adapter generation + // when the inner type contains any pointer-bearing + // option/result/variant arm anywhere reachable. + // `element_inner_pointers` does not recurse into Option / + // Result / Variant payloads (its `_ => {}` catch-all silently + // drops them), so the returned `CopyLayout::Elements` + // descriptor would omit fixup for the conditional payload's + // pointer. The original LS-P-12 mitigation only fired when + // `inner_ptrs.is_empty()` — a record/tuple mixing a covered + // pointer (bare `string`/`list`) with a hidden conditional + // pointer (e.g. `record { items: list, maybe: + // option }`) made `inner_ptrs` non-empty via the + // covered field, bypassing the guard and silently dropping + // the conditional fixup (LS-P-18). The deep recursive check + // closes that bypass. + if self.has_pointer_bearing_conditional(inner) { + panic!( + "LS-P-18: list element type contains a pointer-\ + bearing option/result/variant payload reachable \ + anywhere within the element layout — \ + `element_inner_pointers` does not yet emit the \ + per-element conditional fixup these payloads need, \ + so generating an adapter would silently leave \ + stale string/list pointers in the callee's \ + memory. Refusing rather than corrupting. The full \ + structural fix (per-element `DiscriminantGuard` \ + on `CopyLayout::Elements.inner_pointers` plus the \ + adapter-side per-element guard evaluation) is \ + tracked as follow-up to LS-P-12. Inner type: \ + {inner:?}", + ); + } let element_size = self.canonical_abi_element_size(inner); let inner_ptrs = self.element_inner_pointers(inner, 0); let inner_res = self.element_inner_resources(inner, 0); @@ -2835,6 +3178,53 @@ impl ParsedComponent { } } + /// LS-P-18 helper: does the type contain a pointer-bearing + /// option/result/variant payload anywhere reachable (recursing through + /// records, tuples, fixed-size lists, and type aliases)? + /// + /// Used by `copy_layout(List(inner))` to refuse adapter generation + /// rather than emit a `CopyLayout::Elements` whose `inner_pointers` + /// silently omits the per-element conditional fixup + /// `element_inner_pointers` cannot yet emit for option/result/variant + /// payloads. Returning `true` means the adapter would silently leave + /// stale string/list pointers in callee memory if we proceeded. + fn has_pointer_bearing_conditional(&self, ty: &ComponentValType) -> bool { + match ty { + ComponentValType::Option(inner) => { + self.type_contains_pointers(inner) || self.has_pointer_bearing_conditional(inner) + } + ComponentValType::Result { ok, err } => { + ok.as_ref().is_some_and(|t| { + self.type_contains_pointers(t) || self.has_pointer_bearing_conditional(t) + }) || err.as_ref().is_some_and(|t| { + self.type_contains_pointers(t) || self.has_pointer_bearing_conditional(t) + }) + } + ComponentValType::Variant(cases) => cases.iter().any(|(_, t)| { + t.as_ref().is_some_and(|t| { + self.type_contains_pointers(t) || self.has_pointer_bearing_conditional(t) + }) + }), + ComponentValType::Record(fields) => fields + .iter() + .any(|(_, t)| self.has_pointer_bearing_conditional(t)), + ComponentValType::Tuple(elems) => elems + .iter() + .any(|t| self.has_pointer_bearing_conditional(t)), + ComponentValType::FixedSizeList(elem, _) => self.has_pointer_bearing_conditional(elem), + ComponentValType::Type(idx) => { + if let Some(ct) = self.get_type_definition(*idx) + && let ComponentTypeKind::Defined(inner) = &ct.kind + { + self.has_pointer_bearing_conditional(inner) + } else { + false + } + } + _ => false, + } + } + /// Find inner (ptr, len) pairs within an element type at the given base offset. /// Returns Vec<(byte_offset, CopyLayout)> for each pointer pair found. fn element_inner_pointers( @@ -2870,7 +3260,7 @@ impl ParsedComponent { let align = self.canonical_abi_align(field_ty); offset = align_up(offset, align); result.extend(self.element_inner_pointers(field_ty, offset)); - offset += self.canonical_abi_size_unpadded(field_ty); + offset += self.canonical_abi_element_size(field_ty); } } ComponentValType::Tuple(elems) => { @@ -2879,7 +3269,7 @@ impl ParsedComponent { let align = self.canonical_abi_align(elem_ty); offset = align_up(offset, align); result.extend(self.element_inner_pointers(elem_ty, offset)); - offset += self.canonical_abi_size_unpadded(elem_ty); + offset += self.canonical_abi_element_size(elem_ty); } } ComponentValType::Type(idx) => { @@ -2926,7 +3316,7 @@ impl ParsedComponent { let align = self.canonical_abi_align(field_ty); offset = align_up(offset, align); result.extend(self.element_inner_resources(field_ty, offset)); - offset = offset.saturating_add(self.canonical_abi_size_unpadded(field_ty)); + offset = offset.saturating_add(self.canonical_abi_element_size(field_ty)); } } ComponentValType::Tuple(elems) => { @@ -2935,7 +3325,7 @@ impl ParsedComponent { let align = self.canonical_abi_align(elem_ty); offset = align_up(offset, align); result.extend(self.element_inner_resources(elem_ty, offset)); - offset = offset.saturating_add(self.canonical_abi_size_unpadded(elem_ty)); + offset = offset.saturating_add(self.canonical_abi_element_size(elem_ty)); } } ComponentValType::Type(idx) => { @@ -4410,6 +4800,625 @@ mod tests { test_canonical_abi_size_fixed_size_list_saturates_on_overflow(); } + /// `flat_byte_size` for `result` / `variant` must use the + /// canonical-ABI element-wise JOIN of the arms' flat width lists, + /// not `max` of their byte totals. + /// + /// Surfaced by the mythos-auto delta-pass on PR #178: for + /// `result` the old `4 + max(flat_byte_size(u64), + /// flat_byte_size(string))` gave `4 + max(8, 8) = 12`, but the + /// joined flat sequence is `[i32 disc, i64, i32]` = 16 bytes. The + /// `max` form underestimates whenever the arms flatten to a + /// different *number* of core values. + #[test] + fn flat_byte_size_result_uses_element_wise_join_not_max() { + let pc = empty_parsed_component(); + let u64_t = ComponentValType::Primitive(PrimitiveValType::U64); + + // result: ok arm u64 -> [i64] (8 B), err arm + // string -> [i32,i32] (8 B). Joined payload [i64,i32] = 12 B, + // + 4 B discriminant = 16. The pre-fix `max` form gave 12. + let result_u64_string = ComponentValType::Result { + ok: Some(Box::new(u64_t.clone())), + err: Some(Box::new(ComponentValType::String)), + }; + assert_eq!( + pc.flat_byte_size(&result_u64_string), + 16, + "result flat byte size must be the element-wise \ + join 4+[i64,i32]=16, not 4+max(8,8)=12", + ); + + // variant with unequal-arity arms: { a(u64), b(tuple) } + // -> a [i64] (8 B), b [i32,i32] (8 B). Joined [i64,i32] = 12, + // + 4 disc = 16. + let variant = ComponentValType::Variant(vec![ + ("a".into(), Some(u64_t.clone())), + ( + "b".into(), + Some(ComponentValType::Tuple(vec![ + ComponentValType::Primitive(PrimitiveValType::U32), + ComponentValType::Primitive(PrimitiveValType::U32), + ])), + ), + ]); + assert_eq!( + pc.flat_byte_size(&variant), + 16, + "variant with unequal-arity arms must join element-wise", + ); + + // Equal-shape arms: result -> both [i32]; join [i32] + // = 4, + 4 disc = 8. (max and join agree when arms are equal.) + let result_u32_u32 = ComponentValType::Result { + ok: Some(Box::new(ComponentValType::Primitive(PrimitiveValType::U32))), + err: Some(Box::new(ComponentValType::Primitive(PrimitiveValType::U32))), + }; + assert_eq!(pc.flat_byte_size(&result_u32_u32), 8); + + // Non-variant types are unchanged by the rewrite. + let record = ComponentValType::Record(vec![ + ( + "x".into(), + ComponentValType::Primitive(PrimitiveValType::U8), + ), + ("y".into(), ComponentValType::String), + ]); + assert_eq!(pc.flat_byte_size(&record), 12, "record: [i32]+[i32,i32]=12"); + assert_eq!(pc.flat_byte_size(&u64_t), 8, "u64: [i64]=8"); + } + + /// LS-P-6 — `params_area_byte_size` / `return_area_byte_size` must + /// **saturate** their cross-field accumulator, not plain `+=`. + /// + /// `canonical_abi_size_unpadded` saturates to `u32::MAX` for a + /// pathologically large `fixed-length-list` (LS-P-4). But the two + /// area-size loops accumulated each field with a bare + /// `size += canonical_abi_size_unpadded(ty)`: once a first field + /// saturates `size` to `u32::MAX`, the next field's `+=` overflows + /// — debug builds panic, release builds wrap `u32::MAX` down to a + /// small value. The resolver stores that wrapped value as + /// `params_area_byte_size`, the adapter passes it to + /// `cabi_realloc`, and a multi-field params buffer is allocated + /// far too small → OOB write in callee memory. (The sibling + /// accumulators inside `canonical_abi_size_unpadded` itself already + /// use `saturating_add`; these two were missed.) Surfaced by the + /// mythos-auto delta-pass on PR #179. + #[test] + fn ls_p_6_area_byte_size_saturates_across_fields() { + let pc = empty_parsed_component(); + // FixedSizeList(f64, 2^29): element size 8, 8 * 2^29 = 2^32 — + // canonical_abi_size_unpadded saturates this to u32::MAX. + let huge = ComponentValType::FixedSizeList( + Box::new(ComponentValType::Primitive(PrimitiveValType::F64)), + 1 << 29, + ); + let u32_t = ComponentValType::Primitive(PrimitiveValType::U32); + + // params_area_byte_size: huge field then a second field. The + // accumulator is u32::MAX after field 1; field 2's add must + // saturate, not wrap (and must not panic in a debug build). + // The contract: the result stays an un-allocatable size near + // u32::MAX (the final `align_up` may round the saturated value + // down by < max_align), so a downstream `cabi_realloc` fails + // safely. The pre-fix bug wrapped it to ~3 — catastrophic + // under-allocation. Anything above 2 GiB proves no wrap. + let params = vec![ + ("a".to_string(), huge.clone()), + ("b".to_string(), u32_t.clone()), + ]; + let psize = pc.params_area_byte_size(¶ms); + assert!( + psize > (1u32 << 31), + "params_area_byte_size must saturate across fields, not \ + wrap; got {psize}", + ); + + // return_area_byte_size: same accumulator, same hazard. + let results = vec![(None, huge), (Some("r".to_string()), u32_t)]; + let rsize = pc.return_area_byte_size(&results); + assert!( + rsize > (1u32 << 31), + "return_area_byte_size must saturate across fields, not \ + wrap; got {rsize}", + ); + } + + /// LS-P-7 — `collect_conditional_pointers` and + /// `collect_conditional_result_pointers` must compute the `CopyLayout` for + /// each *pointer leaf* inside an option/result/variant payload, not for the + /// whole composite payload. + /// + /// Before the fix both functions called + /// `copy_layout_for_string_or_list_at()`. `copy_layout` + /// only special-cases a bare `String`/`List`; a `record`/`tuple`/ + /// `fixed-list` payload fell to the `_ => Bulk { byte_multiplier: 1 }` arm. + /// Every inner pointer position then inherited a 1× multiplier, so a + /// `list` field inside the payload was copied as `len * 1` bytes + /// instead of `len * 8` — a 7/8 under-copy / silent truncation [H-4.1] — + /// and a pointer-containing `Elements` leaf collapsed to `Bulk`, dropping + /// the recursive inner-pointer fixup [H-4.2]. Surfaced by the mythos-auto + /// delta-pass on PR #179. + #[test] + fn ls_p_7_conditional_pointer_layout_is_per_leaf_not_per_composite() { + use crate::resolver::CopyLayout; + let pc = empty_parsed_component(); + + // canonical element size of u64 is align_up(8, 8) = 8. + let expect_mult = 8u32; + let list_u64 = + || ComponentValType::List(Box::new(ComponentValType::Primitive(PrimitiveValType::U64))); + + // Flat-param path: option>>. The list is a + // pointer leaf inside a tuple; its CopyLayout must be Bulk{8}, not the + // Bulk{1} the tuple-level fallback produced before the fix. + let opt_param = ComponentValType::Option(Box::new(ComponentValType::Tuple(vec![ + ComponentValType::Primitive(PrimitiveValType::U32), + list_u64(), + ]))); + let flat_pairs = pc.conditional_pointer_pair_positions(&[("p".to_string(), opt_param)]); + assert_eq!(flat_pairs.len(), 1, "expected one conditional pointer pair"); + match &flat_pairs[0].copy_layout { + CopyLayout::Bulk { byte_multiplier } => assert_eq!( + *byte_multiplier, expect_mult, + "list leaf inside a tuple payload must keep its 8x \ + multiplier, not inherit the composite's Bulk{{1}} fallback", + ), + CopyLayout::Elements { .. } => panic!("list should be Bulk"), + } + + // Retptr/byte-offset path: result }, _>. + let res_ty = ComponentValType::Result { + ok: Some(Box::new(ComponentValType::Record(vec![( + "items".to_string(), + list_u64(), + )]))), + err: None, + }; + let byte_pairs = pc.conditional_pointer_pair_result_positions(&[(None, res_ty)]); + assert_eq!(byte_pairs.len(), 1, "expected one conditional pointer pair"); + match &byte_pairs[0].copy_layout { + CopyLayout::Bulk { byte_multiplier } => assert_eq!( + *byte_multiplier, expect_mult, + "list leaf inside a record payload must keep its 8x \ + multiplier in the retptr path too", + ), + CopyLayout::Elements { .. } => panic!("list should be Bulk"), + } + } + + /// LS-P-8 — record/tuple field-walk accumulation must add each field's + /// *padded* canonical-ABI footprint (`canonical_abi_element_size`), not the + /// `_unpadded` value. + /// + /// Before the fix, ~25 field-walk sites (the `Record` / `Tuple` arms of + /// `canonical_abi_size_unpadded`, `collect_pointer_byte_offsets`, + /// `collect_pointer_byte_offsets_with_layout`, + /// `collect_conditional_result_pointers`, `collect_return_area_type_slots`, + /// `collect_resource_byte_positions`, `element_inner_pointers`, + /// `element_inner_resources`, plus the top-level params/results walks in + /// `params_area_byte_size`, `return_area_byte_size`, + /// `pointer_pair_params_byte_offsets`, `pointer_pair_result_offsets`, + /// `params_area_slots`, `return_area_slots`, `resource_params_area_positions`, + /// `resource_result_positions`, and + /// `conditional_pointer_pair_result_positions`) advanced the running offset + /// by `canonical_abi_size_unpadded(field)`. The Component Model spec says + /// `s += size(field)` where `size(field)` for an aggregate field is the + /// field's full padded size (= this codebase's `canonical_abi_element_size`). + /// The per-field `align_up(offset, next_field_align)` does NOT re-absorb a + /// preceding field's trailing pad when the next field has *smaller* + /// alignment — so a record/tuple containing a padded aggregate followed by + /// a lower-aligned field came out smaller than the spec, and a list-copy + /// adapter built from those offsets used wrong byte offsets / multipliers. + /// Surfaced by the mythos-auto delta-pass on PR #179 (the auto-runner + /// mis-located the bug as the option/variant/result payload contribution; + /// independent clean-room verification corrected it to the Record/Tuple + /// field accumulation). + #[test] + fn ls_p_8_record_tuple_field_accumulation_uses_padded_field_size() { + let pc = empty_parsed_component(); + + // Inner record { a: u32, b: u8 } — align 4, spec size 8, unpadded 5. + let r_in = ComponentValType::Record(vec![ + ( + "a".to_string(), + ComponentValType::Primitive(PrimitiveValType::U32), + ), + ( + "b".to_string(), + ComponentValType::Primitive(PrimitiveValType::U8), + ), + ]); + + // Outer tuple — the canonical bug shape: padded aggregate + // followed by a lower-aligned trailing field. Pre-fix: + // unpadded(R_in)=5 → b at offset 5, total 6, element_size 8. + // Post-fix per spec: element_size(R_in)=8 → b at offset 8, total 9, + // element_size 12. + let t = ComponentValType::Tuple(vec![ + r_in.clone(), + ComponentValType::Primitive(PrimitiveValType::U8), + ]); + + // element_size must equal spec size(T) = 12. + assert_eq!( + pc.canonical_abi_element_size(&t), + 12, + "element_size(tuple) must match spec size = 12", + ); + + // params_area_byte_size: top-level params walk follows the same spec + // record_size rule, then aligns to max-align. params [R_in, u8]: + // align_up(0,4)=0, +8=8; align_up(8,1)=8, +1=9; final align_up(9,4)=12. + let params = vec![("p".to_string(), r_in.clone()), ("q".to_string(), t)]; + assert_eq!( + pc.params_area_byte_size(¶ms), + // p (R_in): 0, +8 = 8. q (T = element_size 12, align 4): + // align_up(8,4)=8, +12=20. final align_up(20, max_align=4)=20. + 20, + "params_area_byte_size must add each top-level param's padded \ + element_size, not its unpadded size", + ); + + // pointer_pair_result_offsets: with `tuple>` the + // list must be located at offset 8 (after R_in's full padded + // footprint), not at the pre-fix offset 5. + let t_with_list = ComponentValType::Tuple(vec![ + r_in, + ComponentValType::List(Box::new(ComponentValType::Primitive(PrimitiveValType::U32))), + ]); + let offsets = pc.pointer_pair_result_offsets(&[(None, t_with_list)]); + assert_eq!( + offsets, + vec![8], + "list following record{{u32,u8}} must sit at offset 8 \ + (spec size of the preceding record), not at the unpadded 5", + ); + } + + /// LS-P-18 — strengthens the LS-P-12 mitigation against a mixed-fields + /// bypass. + /// + /// The original LS-P-12 guard fired when `element_inner_pointers(inner, 0)` + /// was empty AND `type_contains_pointers(inner)` was true. For a record + /// that mixed a covered pointer (bare `string` or `list`) with a hidden + /// conditional pointer (`option`), `element_inner_pointers` + /// returned a non-empty Vec from the covered field — the guard didn't + /// fire, `copy_layout` produced `CopyLayout::Elements` whose + /// `inner_pointers` omitted the conditional payload's pointer, and the + /// adapter never fixed it up across memories. LS-P-18 replaces the + /// emptiness-based guard with a recursive + /// `has_pointer_bearing_conditional(inner)` check that detects any + /// pointer-bearing option/result/variant *anywhere* in the inner + /// element layout. Surfaced by the mythos-auto delta-pass on PR #179. + #[test] + #[should_panic(expected = "LS-P-18")] + fn ls_p_18_mixed_record_with_option_string_bypasses_p12_then_refuses() { + let pc = empty_parsed_component(); + // record { items: list, maybe: option } + // Pre-LS-P-18: + // element_inner_pointers(record) finds the `items` list — non-empty. + // `inner_ptrs.is_empty() && type_contains_pointers(inner)` is FALSE + // (since inner_ptrs is non-empty), so the LS-P-12 panic doesn't + // fire. + // `copy_layout(list)` returns Elements with inner_pointers + // covering only the bare `items` list — the option's + // payload pointer is silently omitted. + // Post-LS-P-18: + // has_pointer_bearing_conditional(record) recurses into fields, + // hits the option, returns true → panic. + let mixed = ComponentValType::Record(vec![ + ( + "items".to_string(), + ComponentValType::List(Box::new(ComponentValType::Primitive(PrimitiveValType::U8))), + ), + ( + "maybe".to_string(), + ComponentValType::Option(Box::new(ComponentValType::String)), + ), + ]); + let list_of_mixed = ComponentValType::List(Box::new(mixed)); + let _ = pc.copy_layout(&list_of_mixed); + } + + /// LS-P-18 — pure-bare-pointer records (no nested conditional) must still + /// flow through `CopyLayout::Elements` without panicking. Pins the + /// guard's specificity. + #[test] + fn ls_p_18_pure_bare_pointer_record_still_works() { + use crate::resolver::CopyLayout; + let pc = empty_parsed_component(); + // record { name: string, value: u32 } — both fields covered by + // element_inner_pointers; no conditional payload pointer. + let rec = ComponentValType::Record(vec![ + ("name".to_string(), ComponentValType::String), + ( + "value".to_string(), + ComponentValType::Primitive(PrimitiveValType::U32), + ), + ]); + let list_of_rec = ComponentValType::List(Box::new(rec)); + match pc.copy_layout(&list_of_rec) { + CopyLayout::Elements { .. } => {} + CopyLayout::Bulk { .. } => panic!( + "list must be \ + Elements (LS-R-2), not Bulk; LS-P-18 must not over-refuse", + ), + } + } + + /// LS-P-12 — `copy_layout(List(inner))` must refuse rather than silently + /// emit `Bulk` for a list element type that contains a pointer-bearing + /// option/result/variant payload. + /// + /// `element_inner_pointers` currently has no arms for `Option` / `Result` + /// / `Variant` (its match falls through to `_ => {}`) — so for + /// `list>` it returns the empty Vec, and the existing + /// `copy_layout` would happily produce `CopyLayout::Bulk { byte_multiplier: + /// element_size }`. The FACT adapter's Bulk path is a flat + /// `memory.copy(dst, src, len * element_size)` with NO per-element walk: + /// every option's `(ptr, len)` is copied byte-for-byte into the callee, + /// where `ptr` still references the *source* component's memory. The + /// callee then dereferences a wild pointer for each `Some(...)` element — + /// a cross-memory dangling-reference / arbitrary read on each list use. + /// Surfaced by the mythos-auto delta-pass on PR #179 and clean-room + /// verified as reachable for `list>`, + /// `list>`, and `list`. The + /// per-element conditional fixup the full fix requires (Option/Result/ + /// Variant arms on `element_inner_pointers` + per-element discriminant + /// guards threaded through the inner-pointer descriptor and the adapter's + /// fixup loop) is structural follow-up work; this commit's mitigation is + /// to *refuse to generate the adapter* for these types, converting silent + /// corruption into a loud `panic!` at adapter-generation time. + #[test] + #[should_panic(expected = "LS-P-12")] + fn ls_p_12_list_of_option_string_refuses_rather_than_silently_corrupts() { + let pc = empty_parsed_component(); + let list_opt_string = ComponentValType::List(Box::new(ComponentValType::Option(Box::new( + ComponentValType::String, + )))); + // Pre-fix: returns Bulk { byte_multiplier: 12 } and the adapter + // silently produces stale string pointers in callee elements. + // Post-fix: panics with an LS-P-12 message — full per-element + // conditional fixup tracked as structural follow-up. + let _ = pc.copy_layout(&list_opt_string); + } + + /// LS-P-12 also covers `list>`: the result's payload + /// holds a string in the Ok arm; `element_inner_pointers` doesn't + /// recurse into Result, so the same silent-corruption hazard applies. + #[test] + #[should_panic(expected = "LS-P-12")] + fn ls_p_12_list_of_result_string_refuses() { + let pc = empty_parsed_component(); + let list_result_string = ComponentValType::List(Box::new(ComponentValType::Result { + ok: Some(Box::new(ComponentValType::String)), + err: Some(Box::new(ComponentValType::Primitive(PrimitiveValType::U32))), + })); + let _ = pc.copy_layout(&list_result_string); + } + + /// Sanity: a list whose element contains NO pointers must still produce + /// `Bulk` (no panic). This pins down that LS-P-12's check is gated on + /// `type_contains_pointers(inner)` — pure-scalar option/result payloads + /// are unaffected by the conservative refusal. + #[test] + fn ls_p_12_pure_scalar_option_list_is_still_bulk() { + use crate::resolver::CopyLayout; + let pc = empty_parsed_component(); + // option contains no pointer; copy_layout(list>) + // must still classify as Bulk and not panic. + let list_opt_u32 = ComponentValType::List(Box::new(ComponentValType::Option(Box::new( + ComponentValType::Primitive(PrimitiveValType::U32), + )))); + match pc.copy_layout(&list_opt_u32) { + CopyLayout::Bulk { .. } => {} + CopyLayout::Elements { .. } => { + panic!("list> must still be Bulk (no inner pointers)") + } + } + } + + /// LS-P-13 — `pointer_pair_param_positions` returns *flat* indices into + /// the function's lowered param list, and the async FACT adapter's + /// param-copy step must use those positions directly. + /// + /// Before the fix, the async adapter + /// (`fact.rs::emit_param_copy_step`) ignored the resolver's positions + /// and walked `caller_type.params` looking for consecutive `(i32, i32)` + /// slots, gating each match on + /// `pointer_pair_positions.iter().any(|_| true)` — semantically + /// `!is_empty()`. Every adjacent `(i32, i32)` argument was then + /// treated as a `(ptr, len)` string/list and rewritten via + /// `cabi_realloc` + `memory.copy`, corrupting plain integer args + /// (and reading from attacker-controlled addresses when those + /// integers came from a caller). For a signature like + /// `(a: u32, s: string, b: u32, c: u32)` the buggy code produced + /// `positions = [0, 2]` instead of the correct `[1]`. This test + /// pins the resolver contract the fix relies on: positions ARE + /// flat indices into the canonical-lowered param list, and + /// adjacent flat indices in the result correspond to the actual + /// pointer pairs (not every (i32, i32) shape). + #[test] + fn ls_p_13_pointer_pair_param_positions_is_flat_indices_not_just_nonempty() { + let pc = empty_parsed_component(); + // (a: u32, s: string, b: u32, c: u32) → flat layout + // [a@0, ptr_s@1, len_s@2, b@3, c@4] + // The only pointer pair is the string at flat index 1. + let params = vec![ + ( + "a".to_string(), + ComponentValType::Primitive(PrimitiveValType::U32), + ), + ("s".to_string(), ComponentValType::String), + ( + "b".to_string(), + ComponentValType::Primitive(PrimitiveValType::U32), + ), + ( + "c".to_string(), + ComponentValType::Primitive(PrimitiveValType::U32), + ), + ]; + assert_eq!( + pc.pointer_pair_param_positions(¶ms), + vec![1], + "the string is the only pointer pair; pre-LS-P-13 the async \ + adapter would have copied `(a, ptr_s)` and `(len_s, b)` as if \ + both were string ptr-pairs, corrupting plain integer args", + ); + + // Two strings interleaved with integers: + // (a, s1, b, s2, c) → flat [a@0, ptr1@1, len1@2, b@3, ptr2@4, len2@5, c@6] + // Pointer pairs at flat indices [1, 4]. + let mixed = vec![ + ( + "a".to_string(), + ComponentValType::Primitive(PrimitiveValType::U32), + ), + ("s1".to_string(), ComponentValType::String), + ( + "b".to_string(), + ComponentValType::Primitive(PrimitiveValType::U32), + ), + ("s2".to_string(), ComponentValType::String), + ( + "c".to_string(), + ComponentValType::Primitive(PrimitiveValType::U32), + ), + ]; + assert_eq!( + pc.pointer_pair_param_positions(&mixed), + vec![1, 4], + "pointer_pair_param_positions must return flat indices for \ + EVERY pointer pair, in order — the async adapter clones this \ + Vec directly to drive its per-pair cabi_realloc + memory.copy", + ); + } + + /// LS-P-10 — a `ConditionalPointerPair` whose pointer lives inside a + /// nested option/result/variant payload must carry the full chain of + /// enclosing discriminants in `outer_guards`, so the adapter only fires + /// the cross-memory copy when *every* enclosing arm holds. + /// + /// Before the fix, `collect_conditional_pointers` / `_result_pointers` + /// emitted a `ConditionalPointerPair` for the leaf pointer guarded only + /// on the *innermost* discriminant (e.g. the option's Some byte for the + /// string inside `result, u32>`). The outer Result-Ok + /// guard was missing, so when the runtime value was `Err(some_u32)`, + /// the adapter inspected the bytes that happened to occupy the option's + /// discriminant slot — i.e. the `u32` payload — and if those bytes + /// read as `1`, fired a `memory.copy` with the adjacent slots' + /// contents as `(ptr, len)`: a cross-component arbitrary read with + /// attacker-controlled length and source address [H-2 / H-4 / H-4.2]. + /// Surfaced and clean-room-verified on PR #179. The fix threads + /// `outer_guards: &[DiscriminantGuard]` through both collectors and + /// extends `ConditionalPointerPair` with an `outer_guards` field; the + /// fact-adapter helpers (`emit_conditional_guard_chain_flat` / + /// `_byte`) emit the AND of every guard before the existing copy. + #[test] + fn ls_p_10_nested_conditional_pointer_carries_outer_guard_chain() { + use crate::resolver::DiscriminantGuard; + let pc = empty_parsed_component(); + + // result, u32>. Flat layout: + // [result_disc @ 0, option_disc @ 1, string_ptr @ 2, string_len @ 3] + // The string at flat slot 2 should fire ONLY when + // result_disc == 0 (Ok) AND option_disc == 1 (Some). + let ty = ComponentValType::Result { + ok: Some(Box::new(ComponentValType::Option(Box::new( + ComponentValType::String, + )))), + err: Some(Box::new(ComponentValType::Primitive(PrimitiveValType::U32))), + }; + let pairs = pc.conditional_pointer_pair_positions(&[("p".to_string(), ty.clone())]); + + // Find the pair guarding the string leaf (innermost = option-Some + // at slot 1). Before the fix this was the only pair; after the fix + // it must also carry the outer Result-Ok guard. + let string_pair = pairs + .iter() + .find(|p| p.discriminant_position == 1 && p.discriminant_value == 1) + .expect("expected a pair for the string under option-Some"); + assert_eq!(string_pair.ptr_position, 2); + assert_eq!( + string_pair.outer_guards, + vec![DiscriminantGuard { + position: 0, + value: 0, + byte_size: 4, + }], + "string pair inside result, _> must AND the \ + outer Result-Ok guard at slot 0; without it, an Err arm \ + whose u32 byte at slot 1 reads as 1 (Some) would trigger a \ + spurious cross-memory copy", + ); + + // Retptr / byte-offset path: the same nested type as a result with + // disc_size(2) = 1 byte. payload_offset = align_up(1, 4) = 4. + // Inner option: disc at byte 4, disc_size=1, payload at + // align_up(4+1, 4) = 8. String (ptr, len) at offsets 8 and 12. + let byte_pairs = pc.conditional_pointer_pair_result_positions(&[(None, ty)]); + let string_byte_pair = byte_pairs + .iter() + .find(|p| p.discriminant_position == 4 && p.discriminant_value == 1) + .expect("expected a byte-offset pair for the string under option-Some"); + assert_eq!(string_byte_pair.ptr_position, 8); + assert_eq!(string_byte_pair.discriminant_byte_size, 1); + assert_eq!( + string_byte_pair.outer_guards, + vec![DiscriminantGuard { + position: 0, + value: 0, + byte_size: 1, + }], + "retptr-path string pair must AND the outer Result-Ok guard \ + at byte 0; same LS-P-10 hazard, byte-offset variant", + ); + } + + /// LS-P-9 — `total_flat_params` must saturate-fold per-param `flat_count` + /// values, not use `Iterator::sum::()`. + /// + /// `flat_count(FixedSizeList(elem, len))` is + /// `flat_count(elem).saturating_mul(len)` (LS-P-4 saturation contract), + /// so a pathological nested `FixedSizeList` can produce a per-param + /// `flat_count` of `u32::MAX`. A bare `.sum::()` then panics in + /// debug builds and wraps `u32::MAX + 1` to a small value in release. + /// The wrapped sum then compares `<= MAX_FLAT_PARAMS` (16) and the + /// adapter selects the flat calling convention for a function that + /// genuinely needs params-ptr — semantic divergence in the fused + /// output. Surfaced by the mythos-auto delta-pass on PR #179. + #[test] + fn ls_p_9_total_flat_params_saturates_across_params() { + let pc = empty_parsed_component(); + // Nested FixedSizeList: inner flat_count = 1 * 2^17 = 131072, + // outer multiplies by 2^16 = 65536 → 2^33, saturates to u32::MAX. + let huge = ComponentValType::FixedSizeList( + Box::new(ComponentValType::FixedSizeList( + Box::new(ComponentValType::Primitive(PrimitiveValType::U32)), + 1 << 17, + )), + 1 << 16, + ); + let params = vec![ + ("p".to_string(), huge), + ( + "q".to_string(), + ComponentValType::Primitive(PrimitiveValType::U32), + ), + ]; + // Pre-fix: panics in debug, wraps to 0 in release (≤ 16 → flat + // convention). Post-fix: saturates near u32::MAX → params-ptr, + // which is the spec-correct lowering for the function. + let total = pc.total_flat_params(¶ms); + assert!( + total > (1u32 << 31), + "total_flat_params must saturate across params, not wrap; \ + got {total}", + ); + } + /// align_up must not panic when given a saturated u32::MAX size and /// a non-trivial alignment — the previous `(size + align - 1)` form /// would overflow. diff --git a/meld-core/src/resolver.rs b/meld-core/src/resolver.rs index 8ca44a1..d20f040 100644 --- a/meld-core/src/resolver.rs +++ b/meld-core/src/resolver.rs @@ -168,6 +168,21 @@ pub struct ReturnAreaSlot { pub is_pointer_pair: bool, } +/// A single discriminant check: "the discriminant at `position` must equal +/// `value`" (with the correct byte width for the byte-offset path). +/// +/// The adapter loads the discriminant slot (`LocalGet` for the flat path, +/// `I32Load{8U,16U,_}` for the byte-offset path picked by `byte_size`) and +/// compares it to `value`. A [`ConditionalPointerPair`] copy fires only when +/// *every* guard — its inner one plus every entry in `outer_guards` — +/// holds simultaneously (LS-P-10). +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DiscriminantGuard { + pub position: u32, + pub value: u32, + pub byte_size: u32, +} + /// A pointer pair that is conditional on a discriminant value. /// Used for option, result, variant types where /// the pointer data only exists when the discriminant matches. @@ -186,6 +201,18 @@ pub struct ConditionalPointerPair { /// (I32Load8U, I32Load16U, I32Load) for byte-offset-based paths. /// For flat (stack) paths, this is always 4 (i32). pub discriminant_byte_size: u32, + /// Discriminants of *enclosing* option/result/variant levels that must + /// also hold for the copy to fire. Empty for a single-level conditional + /// (e.g. `option`); non-empty when the pointer lives inside a + /// nested conditional payload (e.g. the string inside + /// `result, u32>` needs the outer Result-Ok guard at + /// `base` AND the inner Option-Some guard at `base+1`). Without this + /// chain, the adapter would inspect the *inner* discriminant byte even + /// in arms where the payload type is something else entirely, copying + /// memory based on unrelated bytes that happen to read as the inner + /// discriminant value — an arbitrary cross-component read with + /// attacker-controlled `(ptr, len)` (LS-P-10). + pub outer_guards: Vec, } /// A resolved resource operation for adapter generation. @@ -1342,11 +1369,36 @@ fn resolve_resource_positions( // so it does not own the representation. false } else { - component + match component .component_type_defs .get(pos.resource_type_id as usize) - .map(|def| !matches!(def, crate::parser::ComponentTypeDef::Import(_))) - .unwrap_or(true) + { + Some(def) => !matches!(def, crate::parser::ComponentTypeDef::Import(_)), + None => { + // LS-P-15: out-of-bounds resource_type_id — the previous + // `unwrap_or(true)` silently classified the resource as + // callee-defined, so the adapter generator emitted a + // [resource-rep]/[resource-new] mismatch (treating an + // import as a definition) with no warning. That + // produced wrong handle conversions on every fused + // cross-component call that passed the handle. Drop the + // resolution entry and warn instead — downstream the + // adapter will either find no work to do (if the + // handle is unused on this flat slot) or surface a + // loud missing-fixup error at adapter generation, + // never silently swap conversion sides. + log::warn!( + "resource_type_id {} out of bounds for \ + component_type_defs (len {}); dropping resource \ + resolution for {} at flat_idx {} (LS-P-15)", + pos.resource_type_id, + component.component_type_defs.len(), + field_prefix, + pos.flat_idx, + ); + continue; + } + } }; resolved.push(ResolvedResourceOp { flat_idx: pos.flat_idx, @@ -1973,9 +2025,26 @@ impl Resolver { // Build export index for this component's modules let mut module_exports: HashMap<(&str, &str), (usize, &ModuleExport)> = HashMap::new(); + // Use entry().or_insert() + explicit collision check so a duplicate + // flat-name export between two core modules in this component fails + // loudly rather than silently routing every importer to the last + // module (LS-P-11). The instance-graph resolver below uses the + // explicit instance edges and is immune to this; the flat-name path + // is reached only for components without an `InstanceSection`. for (mod_idx, module) in component.core_modules.iter().enumerate() { for export in &module.exports { let key = ("", export.name.as_str()); + if let Some((prev_idx, _)) = module_exports.get(&key) { + let prev_idx = *prev_idx; + if prev_idx != mod_idx { + return Err(Error::DuplicateModuleExport { + component_idx: comp_idx, + export_name: export.name.clone(), + first_module_idx: prev_idx, + second_module_idx: mod_idx, + }); + } + } module_exports.insert(key, (mod_idx, export)); } } @@ -2825,11 +2894,49 @@ impl Resolver { } } } - if matched_caller_enc.is_none() - && let Some((_, lo)) = + if matched_caller_enc.is_none() { + // LS-P-17: the exact-name match above only inspects + // ComponentTypeRef::Func component imports — WIT + // interface imports lower to + // ComponentTypeRef::Instance, so for typical + // wit-component / wasm-tools output the loop always + // misses and the fallback below picks the + // lowest-indexed Lower's encoding for *every* + // interface. That heuristic is correct when the + // caller is single-encoding (the common case) and + // silently wrong when the caller is mixed-encoding + // (e.g. UTF-16 for one interface, UTF-8 for another): + // string_transcoding gets miscalibrated and the + // emitted lift/lower bridge skips transcoding where + // it was needed (or vice versa). Surface mixed- + // encoding callers with a loud warning so the issue + // is no longer silent; the full structural fix + // (per-interface attribution via Instance-aliased + // func indices) is tracked as follow-up. + let first_enc = caller_lower_map + .values() + .next() + .map(|lo| lo.string_encoding); + let all_same = caller_lower_map + .values() + .all(|lo| Some(lo.string_encoding) == first_enc); + if !all_same { + log::warn!( + "LS-P-17: caller component has multiple \ + distinct string encodings across its lowered \ + imports; falling back to the lowest-indexed \ + Lower's encoding as a heuristic for interface \ + import `{}`. string_transcoding may be \ + miscalibrated if this interface's actual \ + lowered encoding differs.", + import_name, + ); + } + if let Some((_, lo)) = caller_lower_map.iter().min_by_key(|(k, _)| **k) - { - matched_caller_enc = Some(lo.string_encoding); + { + matched_caller_enc = Some(lo.string_encoding); + } } if let Some(enc) = matched_caller_enc { requirements.caller_encoding = Some(enc); @@ -3090,17 +3197,41 @@ impl Resolver { } } - if matched_caller_encoding.is_none() - && let Some((_, lower_opts)) = + if matched_caller_encoding.is_none() { + // LS-P-17 (secondary site): same ComponentTypeRef::Func + // filter / Instance-import miss / heuristic fallback as the + // primary site above. Warn on mixed-encoding callers so + // miscompiled string_transcoding becomes loud rather than + // silent; the structural per-interface attribution fix is + // tracked as follow-up. + let first_enc = caller_lower_map + .values() + .next() + .map(|lo| lo.string_encoding); + let all_same = caller_lower_map + .values() + .all(|lo| Some(lo.string_encoding) == first_enc); + if !all_same { + log::warn!( + "LS-P-17 (secondary): caller component has multiple \ + distinct string encodings across its lowered imports; \ + falling back to the lowest-indexed Lower's encoding \ + as a heuristic for interface import `{}`. \ + string_transcoding may be miscalibrated.", + import_name, + ); + } + if let Some((_, lower_opts)) = caller_lower_map.iter().min_by_key(|(k, _)| **k) - { - log::debug!( - "Using heuristic lower encoding for import '{}' \ - (name-based match not found; {} lower entries)", - import_name, - caller_lower_map.len() - ); - matched_caller_encoding = Some(lower_opts.string_encoding); + { + log::debug!( + "Using heuristic lower encoding for import '{}' \ + (name-based match not found; {} lower entries)", + import_name, + caller_lower_map.len() + ); + matched_caller_encoding = Some(lower_opts.string_encoding); + } } if let Some(enc) = matched_caller_encoding { @@ -4478,6 +4609,201 @@ mod tests { // produced exactly one entry per prefix — silently overwriting one // import. The tests below pin down the per-resource keying. + fn module_with_exports(idx: u32, exports: Vec<&str>) -> crate::parser::CoreModule { + crate::parser::CoreModule { + index: idx, + bytes: Vec::new(), + types: Vec::new(), + imports: Vec::new(), + exports: exports + .into_iter() + .map(|name| crate::parser::ModuleExport { + name: name.to_string(), + kind: crate::parser::ExportKind::Function, + index: 0, + }) + .collect(), + functions: Vec::new(), + memories: Vec::new(), + tables: Vec::new(), + globals: Vec::new(), + start: None, + data_count: None, + element_count: 0, + custom_sections: Vec::new(), + code_section_range: None, + global_section_range: None, + element_section_range: None, + data_section_range: None, + } + } + + /// LS-P-11 — `resolve_via_flat_names` populates its export index with + /// blind `HashMap::insert`. When two core modules within one component + /// export the same flat name, the second silently overwrites the first + /// (last-writer wins), routing any importer to the wrong module — a + /// semantic-preservation violation with no error or warning. + /// + /// The instance-graph resolver is immune (keys lookups through the + /// explicit instance edges), so the bug is unreachable for any + /// well-formed multi-module component produced by `wit-component` / + /// `wasm-tools` (those always emit an `InstanceSection`). It IS + /// reachable for components without instances — synthetic test + /// fixtures and a few legacy shapes. Per Mythos process this is a + /// hardening fix, not a security emergency. Surfaced by the + /// mythos-auto delta-pass on PR #179 and clean-room verified. + #[test] + fn ls_p_11_duplicate_flat_name_export_is_rejected() { + let mut comp = empty_parsed_component(); + // Two core modules in one component, both exporting `foo`. No + // InstanceSection — dispatcher routes to resolve_via_flat_names. + comp.core_modules.push(module_with_exports(0, vec!["foo"])); + comp.core_modules.push(module_with_exports(1, vec!["foo"])); + + let result = Resolver::new().resolve(&[comp]); + match result { + Err(Error::DuplicateModuleExport { + component_idx, + export_name, + first_module_idx, + second_module_idx, + }) => { + assert_eq!(component_idx, 0); + assert_eq!(export_name, "foo"); + assert_eq!(first_module_idx, 0); + assert_eq!(second_module_idx, 1); + } + other => panic!( + "expected DuplicateModuleExport for two modules exporting \ + the same flat name; got {other:?}", + ), + } + } + + /// LS-P-15 — `resolve_resource_positions` must drop (not silently + /// misclassify) a `ResourcePosition` whose `resource_type_id` is out + /// of bounds for `component.component_type_defs`. + /// + /// Before the fix, the classifier called + /// `.get(pos.resource_type_id as usize).map(...).unwrap_or(true)` — + /// out-of-bounds defaulted to `callee_defines_resource = true`, so + /// the adapter generator emitted a `[resource-rep]` call where + /// `[resource-new]` was correct (or vice versa) — a silent swap of + /// the two resource-handle conversion sides on every fused + /// cross-component call passing that handle. Surfaced by the + /// mythos-auto delta-pass on PR #179. The instance-graph normal + /// path keys `resource_type_id` through validated type indices + /// produced by the parser, so this is reachable only on parser- + /// produced stale ids, malformed input, or after alias remaps that + /// outrun the local type table — defensive hardening, not a + /// memory-safety emergency. Fix matches on `.get(...)` and emits a + /// `log::warn!` + `continue` on `None`, so the position is dropped + /// and the downstream adapter will either find no work to do for + /// the unused flat slot or surface a loud missing-fixup error, + /// never silently swap. + #[test] + fn ls_p_15_out_of_bounds_resource_type_id_is_dropped_not_misclassified() { + let mut by_type_id: std::collections::HashMap<(u32, &'static str), (String, String)> = + std::collections::HashMap::new(); + // Force the lookup to succeed at an OOB resource_type_id (999), + // so flow reaches the `component_type_defs.get(...)` check. + by_type_id.insert( + (999, "[resource-rep]"), + ("ext".to_string(), "rep".to_string()), + ); + let resource_map = ResourceImportMap { + by_type_id, + by_name: std::collections::HashMap::new(), + }; + + // empty_parsed_component has component_type_defs empty, so any + // non-zero resource_type_id is OOB. + let pc = empty_parsed_component(); + let pos = crate::parser::ResourcePosition { + flat_idx: 0, + byte_offset: 0, + is_owned: true, + resource_type_id: 999, + }; + + let resolved = resolve_resource_positions( + &resource_map, + std::slice::from_ref(&pos), + "[resource-rep]", + &pc, + /* callee_is_reexporter = */ false, + ); + + // Pre-fix: 1 entry pushed with callee_defines_resource = true, + // mis-routing the downstream [resource-rep]/[resource-new] call. + // Post-fix: warn + continue → 0 entries. + assert!( + resolved.is_empty(), + "out-of-bounds resource_type_id must be dropped (LS-P-15), \ + not silently classified as callee-defined; got {} resolved \ + ops", + resolved.len(), + ); + } + + /// LS-P-17 — the caller-encoding name-match loops in resolver.rs filter + /// on `ComponentTypeRef::Func` only, but WIT interface imports lower to + /// `ComponentTypeRef::Instance`. The loop always misses for typical + /// wit-component / wasm-tools output, and the heuristic fallback + /// (`min_by_key` over `caller_lower_map`) picks the lowest-indexed + /// Lower's encoding for **every** interface — silently miscalibrating + /// `string_transcoding` when the caller has multiple distinct string + /// encodings. + /// + /// The fix here is the conservative mitigation: when the loop misses + /// AND the caller has distinct encodings, emit a `log::warn!` before + /// the fallback so the miscompile is no longer silent. The structural + /// per-interface attribution (Instance-aliased func indices) is + /// follow-up work tracked separately. + /// + /// This test pins both occurrences of the marker (primary site around + /// 2877–2940 and the secondary fallback site around 3175–3225) and + /// verifies that the warn-on-mixed pattern (`all_same` boolean + + /// `log::warn!` with `LS-P-17` marker) is present at each. + #[test] + fn ls_p_17_mixed_caller_encoding_warns_before_heuristic_fallback() { + const SRC: &str = include_str!("resolver.rs"); + let primary_marker = "LS-P-17: the exact-name match"; + assert!( + SRC.contains(primary_marker), + "resolver.rs must retain the LS-P-17 primary-site marker \ + `{primary_marker}`; without it the warn-before-fallback \ + guard for mixed-encoding callers regressed", + ); + let secondary_marker = "LS-P-17 (secondary)"; + assert!( + SRC.contains(secondary_marker), + "resolver.rs must retain the LS-P-17 secondary-site marker \ + `{secondary_marker}` covering the fallback name-match path", + ); + + // Both sites must include the `all_same` flag (encoding-uniformity + // detection) gating the warn. + let warn_pattern_count = SRC.matches("all_same = caller_lower_map\n").count() + + SRC.matches("all_same\n").count(); + assert!( + warn_pattern_count >= 1, + "expected the LS-P-17 warn-on-mixed-encoding pattern at \ + both sites (primary 2877-, secondary 3175-); found {} \ + matches of the `all_same` flag", + warn_pattern_count, + ); + + // Loud-warn instruction must be present (rather than just a + // debug-level log) at both LS-P-17 sites. + let warn_count = SRC.matches("log::warn!").count(); + assert!( + warn_count >= 2, + "expected at least 2 log::warn! calls in resolver.rs for the \ + LS-P-17 mixed-encoding mitigation; found {warn_count}", + ); + } + fn module_with_imports(imports: Vec<(&str, &str)>) -> crate::parser::CoreModule { crate::parser::CoreModule { index: 0, diff --git a/safety/stpa/loss-scenarios.yaml b/safety/stpa/loss-scenarios.yaml index f1ae97e..fc6d8e8 100644 --- a/safety/stpa/loss-scenarios.yaml +++ b/safety/stpa/loss-scenarios.yaml @@ -161,6 +161,1010 @@ loss-scenarios: status: approved priority: high + - id: LS-P-6 + title: Area-size accumulators wrap, undoing canonical_abi_size_unpadded saturation + uca: UCA-P-3 + hazards: [H-2, H-4, H-4.1] + type: inadequate-control-algorithm + scenario: > + `meld-core/src/parser.rs::params_area_byte_size` and + `return_area_byte_size` compute the canonical-ABI memory size of a + component function's params / results by accumulating each field: + `size = align_up(size, align); size += canonical_abi_size_unpadded(ty)`. + The `+=` is a bare wrapping add. `canonical_abi_size_unpadded` + itself **saturates** to `u32::MAX` for a pathologically large + `fixed-length-list` (the LS-P-4 fix, which also hardened + `align_up` against a saturated input). But the LS-P-4 fix did not + reach these two cross-field accumulators: once a first field + saturates `size` to `u32::MAX`, the next field's `size += + canonical_abi_size_unpadded(ty)` overflows — a debug build panics + ("attempt to add with overflow"), a release build wraps `u32::MAX` + down to a small value. A function with params + `(fixed-length-list, u32)` makes `params_area_byte_size` + wrap from `u32::MAX` to ~3. The resolver stores that wrapped value + as `AdapterRequirements::params_area_byte_size`; the FACT adapter + generator passes it to `cabi_realloc`, allocating a params buffer + a few bytes long and then copying every parameter into it — + an out-of-bounds write into the callee's linear memory + [UCA-P-3 / H-2 caller-controlled offsets / H-4.1 OOB write]. + The wasm validator does not catch it; the under-sized realloc and + the copy both type-check. The sibling accumulators inside + `canonical_abi_size_unpadded` (its Record and Tuple arms) already + use `saturating_add` — these two area-size loops were simply + missed by LS-P-4. Fix replaces both `+=` sites with + `size = size.saturating_add(...)`, so a saturated field keeps the + whole area size at a near-`u32::MAX` (un-allocatable) value and + the downstream `cabi_realloc` fails safely instead of + under-allocating. Surfaced by the mythos-auto delta-pass on + PR #179. Confirmed by PoC + `meld-core::parser::tests::ls_p_6_area_byte_size_saturates_across_fields`, + which panics today on the bare `+=` and asserts a saturated + result after the fix; the nearest primitive-layer proof is + LS-P-4's `kani_fixed_size_list_size_no_overflow` Kani harness + (the symbolic surface here is a `&[(String, ComponentValType)]` + slice, which CBMC cannot enumerate directly). + causal-factors: + - >- + `canonical_abi_size_unpadded` was hardened to saturate (LS-P-4) + but its two callers' cross-field `+=` accumulators were not, + re-introducing the wrap at the next layer up + - >- + Bare `+=` used where the sibling Record/Tuple accumulators + inside the same module already use `saturating_add` — + an inconsistency that hid the gap + - >- + No regression test exercised a multi-field params/results area + whose first field saturates the size accumulator + process-model-flaw: > + The LS-P-4 fix's model was "the size primitives saturate", but the + saturation guarantee is only end-to-end if every accumulator that + consumes a saturated value also saturates. + status: approved + priority: high + + - id: LS-P-7 + title: Conditional-pointer CopyLayout computed for composite, not the leaf + uca: UCA-P-3 + hazards: [H-4, H-4.1, H-4.2] + type: inadequate-process-model + scenario: > + `meld-core/src/parser.rs::collect_conditional_pointers` (flat-param + path) and `collect_conditional_result_pointers` (retptr byte-offset + path) emit one `ConditionalPointerPair` per pointer leaf inside an + `option` / `result` / `variant` payload. They correctly + enumerated each leaf's *position* by recursing through the payload + with `collect_pointer_positions` / `collect_pointer_byte_offsets`, + but then computed the `CopyLayout` exactly once by calling + `copy_layout_for_string_or_list_at()` and + assigning that single layout to every position. `copy_layout` only + special-cases a bare `string` / `list`; any composite payload + (`record`, `tuple`, `fixed-length-list`) falls to the + `_ => CopyLayout::Bulk { byte_multiplier: 1 }` fallback. So for a + param `option>>` or a result + `result }, E>`, the `list` leaf — + whose correct layout is `Bulk { byte_multiplier: 8 }` — was tagged + `Bulk { byte_multiplier: 1 }` [UCA-P-3]. The FACT adapter then + copies `len * 1` bytes for that conditional list instead of + `len * 8`: a 7/8 silent under-copy / truncation of caller data + [H-4 / H-4.1]. Worse, when the leaf is a pointer-containing list + (e.g. `list`) whose correct layout is `CopyLayout::Elements` + with recursive inner-pointer fixup, the composite fallback collapses + it to flat `Bulk`, so the adapter performs a raw byte copy and omits + the inner pointer fixup entirely — the callee then dereferences + string/list pointers that still address the *source* memory + [H-4.2], the same misclassification class as LS-R-2 but on the + conditional (option/result/variant) payload path rather than the + list-element path. The wasm validator does not catch it; the + under-sized copy type-checks. Fix adds + `collect_pointer_positions_with_layout` / + `collect_pointer_byte_offsets_with_layout`, which carry the + `CopyLayout` of each String/List leaf alongside its position, so + every `ConditionalPointerPair` gets the layout of *its own* leaf + rather than the enclosing composite's `Bulk{1}` fallback; the now + dead `copy_layout_for_string_or_list_at` shim is removed. Surfaced + by the mythos-auto delta-pass on PR #179. Confirmed by PoC + `meld-core::parser::tests::ls_p_7_conditional_pointer_layout_is_per_leaf_not_per_composite`, + which exercises both the flat-param and retptr paths and asserts the + `list` leaf keeps an 8× multiplier; the layout primitive it + defends, `copy_layout`, is already covered by the SR-6 / LS-R-2 + `copy_layout` unit tests in `resolver.rs` (the symbolic surface here + is a nested `ComponentValType` tree, which CBMC cannot enumerate + directly). + causal-factors: + - >- + `copy_layout` was called once on the whole payload type instead + of once per pointer leaf, so its `_ => Bulk{1}` fallback masked + every composite payload + - >- + `collect_pointer_positions` returned bare positions, discarding + the leaf type needed to compute the correct per-leaf layout + - >- + No regression test exercised a conditional payload whose pointer + leaf was nested inside a record / tuple / fixed-length-list + process-model-flaw: > + The code's model was "a conditional payload is one pointer object", + but a composite payload holds several independent pointer leaves, + each needing its own CopyLayout — the same level-of-detail mistake + as LS-R-2, here on the option/result/variant path. + status: approved + priority: high + + - id: LS-P-8 + title: Record/tuple field-walk adds unpadded child size, undercounting layout + uca: UCA-P-3 + hazards: [H-4, H-4.1, H-4.2] + type: inadequate-control-algorithm + scenario: > + The Component Model canonical ABI specifies record/tuple layout as + `s = 0; for each field f: s = align_to(s, alignment(f)); s += size(f); + return align_to(s, alignment(record))`, where `size(f)` for an + aggregate field is the field's *full padded* canonical size + (`align_to(unpadded_inner, alignment(f))`). In `meld-core/src/parser.rs` + that full padded size is `canonical_abi_element_size`; + `canonical_abi_size_unpadded` is the outer size *without* the + type's own trailing align-up. Roughly 25 field-walk sites — the + `Record` and `Tuple` match arms of `canonical_abi_size_unpadded`, + `collect_pointer_byte_offsets`, + `collect_pointer_byte_offsets_with_layout` (added by LS-P-7), + `collect_conditional_result_pointers`, + `collect_return_area_type_slots`, `collect_resource_byte_positions`, + `element_inner_pointers`, `element_inner_resources`, plus the + top-level params/results walks in `params_area_byte_size`, + `return_area_byte_size`, `pointer_pair_params_byte_offsets`, + `pointer_pair_result_offsets`, `params_area_slots`, + `return_area_slots`, `resource_params_area_positions`, + `resource_result_positions`, and + `conditional_pointer_pair_result_positions` — advanced the running + offset by `canonical_abi_size_unpadded(field)` instead of + `canonical_abi_element_size(field)`. The per-field + `align_up(offset, next_field_align)` does NOT re-absorb the + preceding field's omitted trailing pad whenever the next field has + *smaller* alignment than the preceding aggregate. So a + `tuple` came out with running + `_unpadded` = 6 and `element_size` = 8 instead of the spec's + 9 / 12; a `list` following `record { u32, u8 }` was located + at offset 5 instead of the spec's 8. Wrong field offsets propagate + into `pointer_pair_*` and the FACT adapter's list-copy / inner + pointer-fixup emission, so the adapter reads pointer pairs from + misaligned offsets and copies bytes using stale offsets [UCA-P-3 / + H-4 / H-4.1 wrong byte length, H-4.2 fixup walks wrong fields]. + `params_area_byte_size` / `return_area_byte_size` under-size the + adapter buffer for the same reason: a `cabi_realloc` sized for the + under-counted total then receives every parameter, and the trailing + bytes spill past the buffer (the LS-P-6 hazard class, but reached + via the wrong field-size primitive rather than the cross-field + accumulator wrap). Fix replaces `canonical_abi_size_unpadded(field)` + with `canonical_abi_element_size(field)` at the 25 record/tuple + field-walk sites; `canonical_abi_size_unpadded` itself still + returns the outer type minus its own trailing pad (that contract is + unchanged — `canonical_abi_element_size` re-adds the pad). Surfaced + by the mythos-auto delta-pass on PR #179; the discover step + mis-located the bug as the `Option` / `Variant` / `Result` payload + contribution (which is actually spec-correct — those arms already + use `canonical_abi_element_size` for payloads). The actual location + was corrected by independent clean-room verification. Confirmed by + PoC `meld-core::parser::tests::ls_p_8_record_tuple_field_accumulation_uses_padded_field_size`, + which asserts `canonical_abi_element_size(tuple)` + = 12, `params_area_byte_size` for the same shape = 20, and a + `list` field after `record{u32,u8}` sitting at byte offset 8. + causal-factors: + - >- + `canonical_abi_size_unpadded` and `canonical_abi_element_size` + are two adjacent primitives differing only by the type's own + trailing align-up; the wrong one was called at the per-field + accumulation step and the function name made it look correct + - >- + 25 independent field-walks rather than a single shared helper + replicated the same wrong call across the file + - >- + No regression test exercised a record/tuple containing a padded + aggregate followed by a lower-aligned trailing field — the only + type shape that surfaces the bug + process-model-flaw: > + The code's model was "advance by the inner type's `_unpadded` size + because we'll align the next field anyway", but the next field's + `align_up` only re-absorbs the trailing pad when the next field's + alignment is at least as large as the preceding aggregate's own + alignment. + status: approved + priority: high + + - id: LS-P-9 + title: total_flat_params uses non-saturating Iterator::sum across params + uca: UCA-P-3 + hazards: [H-2, H-4] + type: inadequate-control-algorithm + scenario: > + `meld-core/src/parser.rs::total_flat_params` returns the total + number of flat (core wasm) values a component function's params + flatten to. The Component Model canonical ABI uses this count to + pick the calling convention: `<= MAX_FLAT_PARAMS` (16) means the + flat convention (each param a core value); `>` means the + params-ptr convention (a single i32 pointer to a params buffer in + linear memory). The function summed per-param `flat_count` values + with `Iterator::sum::()`. `flat_count` for a `FixedSizeList` + is `flat_count(elem).saturating_mul(len)` (LS-P-4 saturation + contract), so a nested `FixedSizeList` can produce a per-param + `flat_count` of `u32::MAX`. A bare `sum()` over u32 then panics + in debug builds on `u32::MAX + 1`; a release build wraps the + total to a small value. The wrapped total then compares + `<= MAX_FLAT_PARAMS` and the adapter selects the **flat** calling + convention for a function that genuinely needs **params-ptr**. + The fused output then passes the param as a sequence of flat core + values that the wasm engine cannot represent (a multi-billion-arg + function call), or — worse, when the wrapped count happens to fit + a valid flat signature — silently emits the *wrong* calling + convention, so the callee reads its parameters from the wrong + ABI slot [UCA-P-3]; the adapter's matching `params_area_*` and + `pointer_pair_params_*` paths use the params-ptr layout while the + function signature uses flat, so call-site lowering and + callee-side lifting disagree [H-2 caller-controlled offsets / + H-4 wrong adapter]. The sibling area-size accumulators inside + `params_area_byte_size` / `return_area_byte_size` already use + `saturating_add` (LS-P-6) — this calling-convention picker was + simply missed. Fix replaces `.sum()` with + `.fold(0u32, u32::saturating_add)`. Surfaced by the mythos-auto + delta-pass on PR #179. Confirmed by PoC + `meld-core::parser::tests::ls_p_9_total_flat_params_saturates_across_params`, + which builds a nested FixedSizeList param whose `flat_count` + saturates to `u32::MAX` plus a second param of `flat_count >= 1` + and asserts the total stays near `u32::MAX` rather than wrapping. + The nearest primitive-layer proof is LS-P-4's + `kani_fixed_size_list_size_no_overflow` Kani harness; the + symbolic surface here is a `&[(String, ComponentValType)]` slice + which CBMC cannot enumerate directly. + causal-factors: + - >- + `Iterator::sum::()` panics on overflow in debug and wraps + in release — neither matches the LS-P-4 saturating contract of + the values it sums + - >- + The calling-convention picker was hardened against per-type + saturation (LS-P-4) but not against cross-param accumulation — + the same gap LS-P-6 fixed for the area-size accumulators + - >- + No regression test exercised a params list whose first param's + `flat_count` already saturated to `u32::MAX` + process-model-flaw: > + LS-P-4's model was "the size/count primitives saturate", but the + saturation guarantee is only end-to-end if every accumulator that + consumes a saturated value also saturates — the same model-flaw + class as LS-P-6, here on the flat-param-count side rather than + the area-byte-size side. + status: approved + priority: high + + - id: LS-P-10 + title: Nested conditional pointer pair omits outer-discriminant guard + uca: UCA-P-3 + hazards: [H-2, H-4, H-4.2] + type: inadequate-process-model + scenario: > + For a pointer-containing type nested inside an option/result/variant + arm — for instance `result, u32>` — + `meld-core/src/parser.rs::collect_conditional_pointers` (flat-param + path) and `collect_conditional_result_pointers` (retptr byte-offset + path) emitted a `ConditionalPointerPair` guarded only on the + *innermost* discriminant. For the example, the string's pair carried + `discriminant_position = base+1, discriminant_value = 1` (the + option's Some byte), but no guard on the outer Result discriminant + at `base`. The FACT adapter (`meld-core/src/adapter/fact.rs`, four + consumer loops covering flat-param, flat-result, retptr-param, and + retptr-result paths) loaded that single discriminant and fired + `cabi_realloc` + `memory.copy` whenever it equalled the expected + value. When the runtime value was `Err(v: u32)`, the byte at + `base+1` did not hold an option discriminant — it held the `u32` + payload. If those bytes happened to read as `1`, the adapter + sampled adjacent slots (the next `(ptr, len)` pair worth of bytes + from the caller's stack / params buffer / return area) as a + string pointer pair and copied `len * 1` bytes from + caller-controlled `ptr` into a freshly allocated callee buffer, + then handed the forged pointer to the callee. That is an + **arbitrary cross-component memory read** with attacker-controlled + length and source address [UCA-P-3 / H-2 caller-controlled offsets / + H-4 wrong adapter / H-4.2 missing pointer fixup gate]. The same + defect applied symmetrically to every nesting where an outer + option/result/variant payload contains a pointer-bearing + option/result/variant (e.g. `option>`, + `variant { a(option), b(u32) }`). Surfaced by the + mythos-auto delta-pass on PR #179; independent clean-room + verification (validator traced the four `fact.rs` consumer loops + and confirmed each treats every `ConditionalPointerPair`'s guard + independently — no implicit AND with any enclosing conditional) + promoted it to approved loss scenario. Fix introduces + `DiscriminantGuard` and adds an `outer_guards: Vec` + field to `ConditionalPointerPair`; both + `collect_conditional_pointers` and + `collect_conditional_result_pointers` accept an `outer_guards` + parameter that is the chain of enclosing-arm discriminants, append + their own discriminant onto the chain when recursing into a payload, + and stamp each emitted pair with the prefix chain; the four FACT + adapter loops call two new helpers + (`emit_conditional_guard_chain_flat` / + `emit_conditional_guard_chain_byte` in `meld-core/src/adapter/fact.rs`) + that emit each guard's `(load disc, I32Const value, I32Eq)` and + `I32And` them all together before the existing `If` block fires the + copy. The innermost guard remains in the existing + `discriminant_*` fields for backward compatibility — `outer_guards` + is empty for single-level conditionals so unchanged callers behave + identically. Confirmed by PoC + `meld-core::parser::tests::ls_p_10_nested_conditional_pointer_carries_outer_guard_chain`, + which builds `result, u32>` and asserts that on both + the flat-param and retptr paths the pair guarding the string leaf + carries the outer Result-Ok guard alongside the inner Option-Some + guard. + causal-factors: + - >- + `ConditionalPointerPair` carried a single (position, value, size) + triple rather than a guard chain, so the parser had nowhere to + encode "this copy is conditional on more than one discriminant" + - >- + Both `collect_conditional_pointers` and + `collect_conditional_result_pointers` recursed into the payload + of an outer option/result/variant by calling themselves with the + payload's base, but did not thread the outer arm's discriminant + into the recursion — so inner-emitted pairs guarded only on the + inner arm + - >- + The four FACT adapter consumer loops processed each pair + independently with a single `(load + I32Eq + If)` block; there + was no separate mechanism that re-gated a nested pair on its + enclosing conditional + - >- + No regression test exercised a nested-conditional pointer shape + — every existing test had at most one level of option/result/ + variant + process-model-flaw: > + The code's model was "a `ConditionalPointerPair` is guarded by its + one discriminant", but pointer-bearing nested conditionals demand a + *conjunction* of discriminants: every enclosing arm must also hold, + because reading the inner discriminant slot in a sibling outer arm + reads unrelated payload bytes that the attacker controls. + status: approved + priority: high + + - id: LS-P-11 + title: Flat-name resolver overwrites duplicate module exports silently + uca: UCA-R-3 + hazards: [H-5, H-1] + type: inadequate-process-model + scenario: > + `meld-core/src/resolver.rs::resolve_via_flat_names` (called from + `resolve_module_imports` when `component.instances.is_empty()`) + builds an export index `HashMap<(&str, &str), (usize, &ModuleExport)>` + keyed by the flat export name. The population loop iterates + `component.core_modules` and calls `module_exports.insert(key, …)` + with no entry check, no warning, no error. When two core modules + within the same component both export the same flat name, the + second module silently overwrites the first (last-writer wins); + any subsequent import of that name resolves to the wrong module + [UCA-R-3 misrouted import], the fused module is wired wrong but + validates clean [H-5 dependency resolution produces wrong order / + H-1 fused module traps at the wrong call site]. The wasm + component validator does not catch it; the import/export shape + type-checks. The instance-graph resolver + (`resolve_via_instances`, taken whenever an `InstanceSection` + exists) keys lookups through explicit instance edges and is + immune. `wit-component` / `wasm-tools` always emit an + `InstanceSection` for multi-module components, so this bug is + latent and **practically unreachable** for well-formed + production components — it is reachable for synthetic test + fixtures and the legacy single-module fallback shape where the + flat-name path is taken. Fix replaces the blind insert with an + explicit collision check that returns a new + `Error::DuplicateModuleExport { component_idx, export_name, + first_module_idx, second_module_idx }` (mirroring the + `DuplicateModuleInstantiation` pattern at `resolver.rs:2115`), + converting silent misrouting into a loud error. Surfaced by the + mythos-auto delta-pass on PR #179 and clean-room verified; + promoted to approved loss scenario as defensive hardening rather + than an emergency. Confirmed by PoC + `meld-core::resolver::tests::ls_p_11_duplicate_flat_name_export_is_rejected`, + which constructs two `CoreModule` entries both exporting `foo` + and no `InstanceSection`, then asserts the resolver returns + `Err(DuplicateModuleExport { .. })`. + causal-factors: + - >- + `HashMap::insert` was used where the spec-correct semantics is + "reject duplicate name within one component" — the duplicate + check was simply missing on this code path + - >- + The sibling `DuplicateModuleInstantiation` error + (`resolver.rs:2115`) already established the precedent for + rejecting same-component duplicates, but the pattern was not + replicated for export names + - >- + No regression test exercised the flat-name path with two + modules emitting the same export — the path is rarely + exercised because production multi-module components have an + `InstanceSection` + process-model-flaw: > + The code's model was "module-export index is whatever the last + module wrote", but the correct invariant is "within one component + a flat export name resolves to at most one module" — same model + as `DuplicateModuleInstantiation`, applied to exports rather than + instantiations. + status: approved + priority: low + + - id: LS-P-12 + title: List of pointer-bearing option/result/variant — silent stale pointer corruption + uca: UCA-P-3 + hazards: [H-4, H-4.2] + type: inadequate-process-model + scenario: > + `meld-core/src/parser.rs::element_inner_pointers` walks the byte + layout of a list-element type and returns each inner `(byte_offset, + copy_layout)` pointer pair so the FACT adapter can do per-element + fixup (allocate in callee memory, copy the pointed-to bytes, replace + the source ptr with the callee's). The function handles `String`, + `List`, `FixedSizeList`, `Record`, `Tuple`, and `Type`-aliased + versions of those, plus a `_ => {}` catch-all (line 3203) that + silently drops `Option`, `Result`, `Variant`, `Flags`, and `Enum`. + As a consequence, `copy_layout(list>)`, + `copy_layout(list>)`, and + `copy_layout(list)` all classified as + `CopyLayout::Bulk { byte_multiplier: element_size }` — because + `element_inner_pointers(option, 0)` returns the empty + vector even though `type_contains_pointers(option)` is + true. The FACT adapter's Bulk path (`adapter/fact.rs:1564-1575` / + `2317-2331` / `2534-2549`) is a flat `memory.copy(dst, src, + len * byte_multiplier)` with **no per-element walk** — so every + copied option's `(ptr, len)` pair is written byte-for-byte into + the callee's memory, with `ptr` still referencing the *source* + component's linear memory. The callee then dereferences a wild + pointer for each `Some(...)` element — a cross-memory + dangling-reference / arbitrary read on every list use + [UCA-P-3 / H-4 wrong adapter / H-4.2 missing pointer fixup]. + `ConditionalPointerPair` (LS-P-7, LS-P-10) does not rescue this: + `collect_conditional_pointers` drops `List(_)` at its own + `_ => {}` arm, and `collect_pointer_positions_with_layout` stops + at `List(_)` and emits the pair with the (here Bulk) layout from + `copy_layout`. Reachable for any non-empty `list>` + passed across a component boundary. Surfaced by the mythos-auto + delta-pass on PR #179 and clean-room verified. + **This commit is the conservative mitigation**, not the full fix. + The proper fix needs (a) `Option`/`Result`/`Variant` arms on + `element_inner_pointers` that recurse into the payload at the + payload byte offset, AND (b) extending the + `inner_pointers` descriptor on `CopyLayout::Elements` (currently + `Vec<(u32, CopyLayout)>`) with per-element `DiscriminantGuard` + chains, AND (c) updating the FACT per-element fixup loop in + `fact.rs` to evaluate those guards before each inner copy. That + is structurally analogous to LS-P-10 but on the list-element axis + rather than the top-level conditional axis — material follow-up + work tracked separately. Until then, `copy_layout` panics with a + clearly-labelled `LS-P-12: …` message when it would otherwise + have produced the corrupting `Bulk` classification, and the + panic includes the offending element type in the diagnostic so + WIT authors and tool integrators see why the adapter can't be + generated. Confirmed by PoC + `meld-core::parser::tests::ls_p_12_list_of_option_string_refuses_rather_than_silently_corrupts` + (plus the `list>` variant); a positive sanity + test (`ls_p_12_pure_scalar_option_list_is_still_bulk`) pins the + check is gated on `type_contains_pointers(inner)` so pure-scalar + `list>` etc. remain `Bulk` as before. + causal-factors: + - >- + `element_inner_pointers` was written when only Record/Tuple + pointer-bearing list elements were considered; the + Option/Result/Variant arms were never added and the + `_ => {}` catch-all silently dropped them + - >- + `copy_layout(List(inner))` accepted the empty inner-pointer + list from a pointer-bearing inner type and produced `Bulk`, + the corrupting classification, with no consistency check + - >- + The adapter's per-element fixup machinery does not yet + support per-element discriminant guards — required for + conditional inner pointer fixup + - >- + No regression test exercised `list>` or any + nested-conditional list element + process-model-flaw: > + The shared model between `element_inner_pointers` and + `copy_layout` was "if the inner-pointer set is empty, the element + has no pointers and a flat byte copy is correct" — but emptiness + came from the missing arms, not from absence-of-pointers. The + `type_contains_pointers(inner)` predicate already encoded the + right invariant; it just wasn't checked alongside the emptiness + decision. + status: approved + priority: high + + - id: LS-P-13 + title: Async adapter param-copy treats every (i32, i32) pair as a pointer pair + uca: UCA-P-3 + hazards: [H-2, H-4, H-4.1] + type: inadequate-control-algorithm + scenario: > + `meld-core/src/adapter/fact.rs::emit_param_copy_step` (called from + `generate_async_callback_adapter` and `generate_async_stackful_adapter` + — the P3-async lift paths) determines which of the caller's flat + param slots are `(ptr, len)` pointer pairs that need a + cross-memory `cabi_realloc` + `memory.copy` fixup. Instead of + using the resolver's `site.requirements.pointer_pair_positions` + slice directly (which is already the list of *flat* indices, + computed by `parser.rs::pointer_pair_param_positions` walking the + function's params with `flat_count`), the code walked + `caller_type.params` looking for consecutive `(i32, i32)` slots + and gated each match on + `pointer_pair_positions.iter().any(|_| true)` — semantically + `!is_empty()`. Every adjacent integer-pair argument was therefore + treated as a pointer pair whenever the function had **any** + string/list param. For a signature like + `fn f(a: i32, s: string, b: i32, c: i32)` lowered to flat params + `[I32, I32, I32, I32, I32]` with the resolver's positions + `[1]`, the buggy code emitted `positions = [0, 2]`: it then ran + `cabi_realloc(0, 0, 1, ptr_s_value)` (treating the string's + *pointer* as a length, attacker-influenced byte count), copied + from caller-memory address `a` (treating the integer arg as a + source pointer), and replaced `a` with the freshly allocated + buffer — corrupting plain integer args and reading from + attacker-controlled caller-memory addresses [UCA-P-3 / H-2 + caller-controlled offsets / H-4.1 wrong byte length]. The real + string at flat index 1 was never copied. Reachable for any + mixed-signature async-lift fusion that crosses memories. Pure + single-string signatures like `fn f(s: string)` flattened to + `[I32, I32]` happen to behave correctly by accident — the only + pair the code considers is `(0, 1)`, which is the string. Same + shape with extra args is corrupting. Surfaced by the mythos-auto + delta-pass on PR #179 and clean-room verified. Fix replaces the + heuristic walk + emptiness check with + `site.requirements.pointer_pair_positions.clone()`: the resolver + already produces the correct flat indices and canonical lowering + preserves param order between caller and callee component types, + so the comment block claiming the positions are "in callee + component type order" while locals are "in caller order" was + misleading — flat indices computed from canonical lowering apply + equally to both sides. Confirmed by PoC + `meld-core::parser::tests::ls_p_13_pointer_pair_param_positions_is_flat_indices_not_just_nonempty`, + which pins the resolver contract on `(a: u32, s: string, b: u32, + c: u32)` returning `[1]` and on `(a, s1, b, s2, c)` returning + `[1, 4]` — the invariants the async adapter now consumes + directly. + causal-factors: + - >- + `.any(|_| true)` was a stand-in placeholder pattern that should + have been `.any(|&pos| pos == i as u32)` (and that, in turn, + should have been the cleaner direct use of the resolver's + positions); the placeholder was never replaced and the + predicate name made it look intentional + - >- + The comment block at the top of `emit_param_copy_step` + claimed the resolver's positions were "in callee component + type order" — implying a caller/callee mismatch the function + needed to work around — but + `pointer_pair_param_positions` returns *flat* indices + computed from the canonical-lowered param list, which + canonical lowering preserves identically between caller and + callee component-type matched signatures + - >- + No regression test exercised `emit_param_copy_step` with a + mixed-signature async export crossing memories; the + `tests/p3_async_lowering.rs` coverage focuses on trampoline + shape and event codes, not parameter copy correctness + process-model-flaw: > + The mental model was "verify there's *something* to copy before + walking the flat params," which collapsed the per-position + identity check into an emptiness check. The right invariant + ("this specific flat slot is a pointer pair") was encoded in + the resolver's positions all along — the adapter just discarded + them. + status: approved + priority: high + + - id: LS-P-14 + title: Nested-list inner copy `buf_len = len * sub_elem_size` lacks overflow guard + uca: UCA-P-3 + hazards: [H-2, H-4, H-4.1] + type: inadequate-control-algorithm + scenario: > + `meld-core/src/adapter/fact.rs::emit_patch_nested_indirections` + walks each element of a list-of-records (or list-of-tuples) at + adapter-generation time and emits, for each inner pointer + indirection inside the element, a `cabi_realloc` + `memory.copy` + sequence that copies the inner list/string from callee memory into + the caller's. The byte count for that copy is computed at runtime: + load `len` from the callee's memory at byte offset `+4` from the + element record, then multiply by `sub_elem_size` (the inner list's + element stride) via a bare `i32.mul`. `i32.mul` is modulo 2³², so + a callee-supplied `len` near `u32::MAX / sub_elem_size` wraps the + product to a small value. The subsequent bounds check + (`old_ptr + buf_len > mem_bytes`) uses `i32.add`, which **also** + wraps and is bypassed. The adapter then allocates and copies only + the wrapped byte count, while the caller-side bulk copy of the + outer element bytes keeps the original large `len` field — so the + callee receives a `(new_ptr, full_len)` pair whose buffer is far + smaller than `full_len` bytes [UCA-P-3 / H-2 caller-controlled + offsets / H-4 / H-4.1 wrong byte length]. Reads/writes into the + buffer past `wrapped_buf_len` walk into adjacent caller-allocated + memory — silent truncation in the inner list contents and OOB + access on every dereference past the truncated edge. Reachable for + any cross-memory fusion of a function whose param/result contains + a `list }>` (or tuple analogue) where + the callee can produce a length within one `sub_elem_size` of + `u32::MAX / sub_elem_size`. Surfaced by the mythos-auto delta-pass + on PR #179. Fix: stash the loaded `len` into the existing + `l_buf_len` scratch local, call + `emit_overflow_guard(body, l_buf_len, sub_elem_size as u32)` + (which traps via `unreachable` when the multiplication would wrap), + then re-fetch the local for the multiplication. `emit_overflow_guard` + with `k == 1` is a no-op so the guard is harmless when + `sub_elem_size == 1`. Confirmed by PoC + `meld-core::adapter::fact::tests::ls_p_14_nested_list_inner_copy_emits_overflow_guard`, + which emits a synthetic patch-loop for `record { items: list }` + (`sub_elem_size = 4`) and asserts the encoded function body + contains an `Unreachable` opcode — the only place that opcode + appears along this path is inside `emit_overflow_guard`. + causal-factors: + - >- + `emit_patch_nested_indirections` was written before the + `emit_overflow_guard` helper landed (the helper was added as + the LS-A-7 leg (a) fix for the outer-list copy paths), and the + retrofit never reached the inner copy + - >- + The bounds check below the multiplication uses the same + wrapping `i32.add` arithmetic, so the comment "skip patch if + (old_ptr, buf_len) doesn't fit in callee mem" overstated the + protection — a wrapped `buf_len` slips the check + - >- + No regression test exercised + `emit_patch_nested_indirections` for an element type with + non-trivial `sub_elem_size` + process-model-flaw: > + The model was "the outer copy paths guard against overflow; the + inner per-element copies do not need to," but the inner copies + consume callee-controlled `len` values just like the outer ones — + the trust boundary is the same. + status: approved + priority: high + + - id: LS-P-15 + title: Out-of-bounds resource_type_id silently misclassified as callee-defined + uca: UCA-R-3 + hazards: [H-5, H-1] + type: inadequate-process-model + scenario: > + `meld-core/src/resolver.rs::resolve_resource_positions` decides + whether the callee component owns a resource's representation + (and therefore the adapter must emit `[resource-rep]`) or merely + imports it (`[resource-new]`). The decision routes through a + lookup into `component.component_type_defs` keyed by + `pos.resource_type_id as usize`: + `.get(...).map(|def| !matches!(def, ComponentTypeDef::Import(_))).unwrap_or(true)`. + When `resource_type_id` exceeds `component_type_defs.len()` — a + stale id surviving alias propagation, a parser-produced id past + the local table, a malformed input — `.get(...)` returns `None` + and `unwrap_or(true)` silently classifies the resource as + *callee-defined* with no warning. The adapter then emits a + `[resource-rep]` call where `[resource-new]` was correct (or + vice versa) — a silent swap of the two resource-handle + conversion sides on every fused cross-component call passing that + handle [UCA-R-3 import wired to wrong source / H-5 dependency + resolution produces wrong order / H-1 fused module traps at the + wrong call site when the handle is dereferenced]. The handle + type-checks on both sides (both are i32-shaped) so the validator + does not catch it. Reachability is bounded: the normal + instance-graph path keys `resource_type_id` through validated + type indices that the parser produced from canonical entries, so + this is reachable mainly on stale ids, malformed input, or after + alias remaps that outrun the local type table — defensive + hardening rather than a memory-safety emergency. Surfaced by the + mythos-auto delta-pass on PR #179. Fix replaces the + `.unwrap_or(true)` with an explicit `match`: on `Some(def)`, + classify by type; on `None`, emit a `log::warn!` and `continue`, + so the position is *dropped* rather than silently misclassified. + The downstream adapter will either find no work to do for the + unused flat slot, or surface a loud missing-fixup error at + adapter generation — never silently swap conversion sides. + Confirmed by PoC + `meld-core::resolver::tests::ls_p_15_out_of_bounds_resource_type_id_is_dropped_not_misclassified`, + which builds a synthetic `ResourceImportMap` whose lookup + succeeds at `resource_type_id = 999`, then calls + `resolve_resource_positions` against an `empty_parsed_component` + (zero entries in `component_type_defs`) and asserts the returned + `Vec` is empty (pre-fix: one mis-classified + entry; post-fix: dropped with a warn log). + causal-factors: + - >- + `unwrap_or(true)` was a stand-in defensive default that should + have been "fall through to the import path" or "fail loud" — + `true` is the more dangerous default because it triggers the + adapter to emit a `[resource-rep]` call against a resource + whose representation the callee does not own + - >- + The lookup miss had no warning channel, so a stale/malformed + `resource_type_id` produced an adapter with quietly wrong + resource-handle conversions and no diagnostic for the + toolchain to surface + - >- + No regression test exercised `resolve_resource_positions` with + a `resource_type_id` outside the local `component_type_defs` + range + process-model-flaw: > + The model was "any failure to classify is benign," but the + adapter's `[resource-rep]` vs `[resource-new]` choice is + asymmetric — emitting the wrong call produces a handle whose + conversion side mismatches the callee's expectation, an error + that compiles cleanly and only surfaces when the handle is + dereferenced at runtime. + status: approved + priority: low + + - id: LS-P-16 + title: UTF-16→UTF-8 transcoding reads OOB on lone high surrogate at end of input + uca: UCA-P-3 + hazards: [H-2, H-4, H-4.4] + type: inadequate-control-algorithm + scenario: > + `meld-core/src/adapter/fact.rs::emit_utf16_to_utf8_transcode` + emits the per-code-unit transcoding loop the FACT adapter uses + when the caller component declares `string-encoding=utf16` and + the callee declares `utf8` (the `(caller=Utf16, callee=Utf8)` + case dispatched at fact.rs:2999–3001, gated on + `crosses_memory && needs_transcoding`). The loop's only bounds + check is `src_idx >= input_len` (lines 3460–3463) against the + *first* code unit of each iteration. When that code unit is in + `[0xD800, 0xDC00)` (high surrogate), the surrogate-pair `If` + arm unconditionally emits a second `I32Load16U` at + `mem16[ptr + (src_idx + 1) * 2]`. For an input whose **last** + code unit is a lone high surrogate, that load reads 2 bytes + past the end of the caller-supplied UTF-16 buffer — into + attacker-adjacent caller linear memory. The two bytes are + treated as the "low surrogate" without validating + `0xDC00 <= cu2 < 0xE000`, producing a `cp = 0x10000 + + ((cu - 0xD800) << 10) + (cu2_oob - 0xDC00)` whose value is + typically in the 4-byte UTF-8 range. The 4-byte encoder + then writes 4 bytes derived from the OOB read into callee + linear memory — silent cross-memory leak of attacker-adjacent + caller bytes into the callee's UTF-8 output [UCA-P-3 / + H-2 caller-controlled offsets / H-4.4 incorrect string + transcoding]. `src_idx += 2` then advances past `input_len`, + so the outer `src_idx >= input_len` check breaks the loop + cleanly the next iteration — no trap, silent corruption. + Reachable for any cross-memory UTF-16→UTF-8 string transfer + whose last UTF-16 code unit is a lone high surrogate. + Surfaced by the mythos-auto delta-pass on PR #179 and + clean-room verified. This commit is the conservative + mitigation: inject a `src_idx + 1 >= input_len` check inside + the surrogate-pair `If` arm and `unreachable`-trap on failure. + The Canonical-ABI-correct behaviour is to replace the lone + surrogate with U+FFFD (3-byte UTF-8 `EF BF BD`) and continue + — that upgrade is tracked as a separate structural follow-up. + Trap converts silent cross-memory leak into a loud + adapter-runtime trap. Confirmed by PoC + `meld-core::adapter::fact::tests::ls_p_16_utf16_lone_high_surrogate_oob_guard_emitted`, + a structural regression that pins the LS-P-16 marker comment + AND requires an `Unreachable` + `I32GeU` opcode pair to live + inside the surrogate-pair `If` arm before the second + `I32Load16U`. + causal-factors: + - >- + `emit_utf16_to_utf8_transcode`'s only bounds check guarded + the first code unit per iteration; the surrogate-pair arm + unconditionally followed up with a second load + - >- + No bounds check between the high-surrogate detection and the + low-surrogate load, despite UTF-16 trivially admitting a + lone high surrogate at the end of a string + - >- + No validation that the second code unit is actually a low + surrogate (`0xDC00 <= cu2 < 0xE000`) — without it, an OOB + read whose value happens to fall outside the low-surrogate + range still proceeds into the 4-byte encoder + - >- + No regression test for UTF-16 inputs ending on a high + surrogate; the test module's TODO list (line 5067) had this + explicitly listed as not-yet-covered + process-model-flaw: > + The model was "high-surrogate detection implies a valid + surrogate pair," but UTF-16 strings legitimately end on a lone + high surrogate (e.g., malformed input from JS, broken + truncation). The Canonical ABI mandates lossy replacement, not + OOB read. + status: approved + priority: high + + - id: LS-P-18 + title: LS-P-12 bypassed when a record mixes a covered pointer with a hidden conditional one + uca: UCA-P-3 + hazards: [H-4, H-4.2] + type: inadequate-process-model + scenario: > + The LS-P-12 mitigation in `copy_layout(List(inner))` fires only when + `element_inner_pointers(inner, 0)` is empty AND + `type_contains_pointers(inner)` is true. For a record/tuple that + mixes a *covered* pointer field (bare `string` or `list`) with a + *hidden* conditional pointer field (an `option/result/variant` whose + payload contains a pointer), `element_inner_pointers` returns a + non-empty Vec from the covered field — the LS-P-12 panic does not + fire and `copy_layout` returns + `CopyLayout::Elements { inner_pointers: [] }`. + The FACT adapter walks `inner_pointers`, fixes up the covered field, + and never touches the option-payload pointer. After the cross-memory + copy the callee receives an element whose option payload still + contains a string `(ptr, len)` pair referencing the *source* + component's memory — silent cross-memory dangling reference per + `Some(_)` element on every fused list copy [UCA-P-3 / H-4 / H-4.2]. + Reachable for any `list }>` (and the + tuple / nested / type-aliased analogues). Surfaced by the + mythos-auto delta-pass on PR #179 as a refinement attack on + LS-P-12; clean-room verified by tracing the `inner_ptrs` / + `inner_res` non-emptiness short-circuit. Fix replaces the + emptiness-based guard with a deep recursive + `has_pointer_bearing_conditional(inner)` check (new helper) that + visits Option / Result / Variant arms reachable through Records, + Tuples, FixedSizeLists, and `Type` aliases. If any pointer-bearing + conditional exists anywhere within the element layout, + `copy_layout(List(inner))` panics with the LS-P-18 marker — the + adapter refuses to generate rather than emit a `CopyLayout::Elements` + whose `inner_pointers` would omit a per-element conditional + fixup. Confirmed by PoC + `meld-core::parser::tests::ls_p_18_mixed_record_with_option_string_bypasses_p12_then_refuses` + which builds `list, maybe: option }>` + and asserts the panic. A positive sanity test, + `ls_p_18_pure_bare_pointer_record_still_works`, pins that the + strengthened guard does not over-refuse pure-bare-pointer records + (e.g. `record { name: string, value: u32 }`). + causal-factors: + - >- + The original LS-P-12 guard's emptiness criterion was a proxy for + "no pointers covered yet" — a positive presence of any covered + pointer (bare `string`/`list`) short-circuited the proxy even + when other pointer positions were silently missing + - >- + No regression test exercised a record that mixed a covered + pointer field with a hidden conditional pointer field — the + bypass had no fixture + process-model-flaw: > + The shared model was "empty `inner_pointers` is the only failure + mode," but the missing-arms behaviour of `element_inner_pointers` + produces a partial output (some pointer positions, missing + others) that the emptiness check accepts as complete. + status: approved + priority: high + + - id: LS-P-19 + title: UTF-8→UTF-16 transcoder reads OOB for truncated multi-byte UTF-8 + uca: UCA-P-3 + hazards: [H-2, H-4, H-4.4] + type: inadequate-control-algorithm + scenario: > + `meld-core/src/adapter/fact.rs::emit_utf8_to_utf16_transcode` emits + the per-byte UTF-8→UTF-16 transcoding loop the FACT adapter uses + when the caller declares `string-encoding=utf8` and the callee + declares `utf16` (the canonical mirror of LS-P-16). The outer + loop guards the lead byte with `src_idx >= input_len`. Each + multi-byte branch (2-byte: `byte < 0xE0`; 3-byte: `byte < 0xF0`; + 4-byte: `byte >= 0xF0`) then unconditionally reads its + continuation bytes via `I32Load8U` at offsets `src_idx + 1`, + `+2`, `+3`. A UTF-8 string ending on a truncated multi-byte lead + byte (e.g. `0xE2` with no following bytes; `0xF0` with only one + following byte; etc.) causes the adapter to read 1–3 bytes of + attacker-adjacent caller linear memory past the buffer end, + incorporate them into a synthesized code point via the canonical + shift/mask formula, and emit the resulting code point as UTF-16 + into the callee output — silent cross-memory leak of caller + bytes per transcoded string [UCA-P-3 / H-2 caller-controlled + offsets / H-4.4 incorrect string transcoding]. The composed + (non-fused) execution traps on invalid UTF-8 with a `len`/byte + mismatch; the fused execution silently corrupted, producing + semantic drift. Reachable for any cross-memory + UTF-8-caller/UTF-16-callee fusion whose UTF-8 string ends on a + truncated multi-byte lead. Surfaced by the mythos-auto + delta-pass on PR #179. Conservative mitigation prepends a + `src_idx + N >= input_len` `unreachable` trap to each multi-byte + branch (N = 1 for the 2-byte case, 2 for 3-byte, 3 for 4-byte). + The Canonical-ABI-correct upgrade replaces truncated sequences + with the U+FFFD replacement character (tracked alongside the + LS-P-16 upgrade). Trap converts silent leak into loud + adapter-runtime trap. Confirmed by PoC + `meld-core::adapter::fact::tests::ls_p_19_utf8_to_utf16_continuation_byte_oob_guard_emitted`, + a structural test pinning the LS-P-19 marker at all three + multi-byte branches and the cumulative Unreachable + I32GeU + opcode count across LS-P-16 and LS-P-19. + causal-factors: + - >- + The outer-loop bounds check guarded only the lead byte; the + continuation-byte reads inside each multi-byte branch had no + independent guard + - >- + The canonical UTF-8 decoder pattern assumes valid UTF-8 input + (continuation bytes always present); fused adapters consume + untrusted caller buffers where truncation is reachable + - >- + No regression test covered the truncated-lead-byte edge case + for any of the three multi-byte branches + process-model-flaw: > + The model was "if the lead byte is in-range, the implied + continuation bytes must also be in-range," but the lead byte's + classification does not constrain the buffer end — a truncated + multi-byte sequence is the obvious counterexample. + status: approved + priority: high + + - id: LS-P-17 + title: Caller-encoding match filters on Func only, miscalibrates mixed-encoding callers + uca: UCA-R-3 + hazards: [H-4, H-4.4] + type: inadequate-process-model + scenario: > + `meld-core/src/resolver.rs` has two structurally-identical + caller-encoding lookup loops (primary site around lines 2877– + 2910, fallback site around 3175–3225). Each iterates + `from_component.imports` filtering on + `wasmparser::ComponentTypeRef::Func(_)` and tries to match + `ci.name == *import_name` to find the caller's `Lower` options + for the resolved interface. WIT interface imports lower to + `ComponentTypeRef::Instance(_)` at the component-import level, + so the loop **never matches for interface imports** — the only + shape that can match is a top-level bare-`Func` component + import, which `wit-component` / `wasm-tools` does not emit + under the WIT-bindgen norm. The loops fall through to a + heuristic `caller_lower_map.iter().min_by_key(|(k, _)| **k)` + that picks the encoding of the **lowest-indexed Lower** entry, + then assigns it as `requirements.caller_encoding` for whatever + interface is currently being resolved. `string_transcoding` is + then computed as `caller_encoding != callee_encoding` against + the single heuristic-selected encoding. For single-encoding + callers (the overwhelming majority of WIT-bindgen output + today — UTF-8 throughout) the heuristic happens to pick the + only encoding present and `string_transcoding` is correctly + attributed. For mixed-encoding callers (UTF-16 for one + interface, UTF-8 for another — e.g. a JS/.NET host component + lowering UTF-16 to one import alongside a Rust UTF-8 lowering + to another, both calling a UTF-8 callee) the heuristic picks + one encoding for **every** interface, so the adapter skips + transcoding where it was needed (or forces it where it was + unnecessary). The emitted lift/lower bridge then either reads + raw UTF-16 bytes as UTF-8 or vice versa, producing scrambled + strings at the call boundary [UCA-R-3 wrong adapter wiring / + H-4 / H-4.4 incorrect string transcoding]. Reachability: + latent today since most components are single-encoding, but + legal under the Component Model and exposed whenever multi- + encoding host components arrive. Surfaced by the mythos-auto + delta-pass on PR #179 and clean-room verified. **This commit + is the conservative mitigation**: detect mixed-encoding + callers (`caller_lower_map.values()` not all identical) before + the heuristic fallback fires and emit a `log::warn!` with the + LS-P-17 marker and the interface name, so the miscompile is + no longer silent. The full structural fix — resolve the + caller's component func index from the core import via a + `caller_core_import_to_comp_func` map (or extend the loop to + walk `ComponentTypeRef::Instance` aliases) to get per-function + precision — is tracked as follow-up. Confirmed by PoC + `meld-core::resolver::tests::ls_p_17_mixed_caller_encoding_warns_before_heuristic_fallback`, + a structural test that requires the LS-P-17 marker comment to + live at **both** sites and that the warn-on-mixed pattern + (`all_same` uniformity check + `log::warn!`) is present. + causal-factors: + - >- + The component-import filter was hardcoded to + `ComponentTypeRef::Func`, but the canonical encoding of WIT + interface imports is `ComponentTypeRef::Instance` — the + filter excludes the very shape it was meant to match + - >- + The heuristic fallback (`min_by_key` over the entire + `caller_lower_map`) cannot recover per-interface attribution + from a global lowest-index choice + - >- + The fallback emitted only a `log::debug!`, so the + misattribution was invisible at default logging level + - >- + No regression test exercised + `resolve_component_imports_with_hints` with a + mixed-encoding caller (UTF-16 for one interface, UTF-8 for + another) + process-model-flaw: > + The model was "the per-interface caller encoding is a function + of the lowered-options table alone," but it is actually a + function of the (interface, func) pair — multi-encoding + callers need per-interface lookup. The global heuristic + collapses that pairing onto a single global default. + status: approved + priority: low + # ========================================================================== # Resolver scenarios # ==========================================================================