fix(automation security): template-bomb DoS (100MB/11s render → fuel-bounded, HIGH) + delay panic-on-config (MEDIUM) (#1083)

* fix(homecore-automation): bound template render to stop unbounded-expansion DoS (HC-SEC-01)

A `template:` condition / value_template comes straight from user
automation config and was rendered with MiniJinja's default (no
instruction budget, no output cap). A single condition such as
`{% for i in range(5000) %}{% for j in range(5000) %}xxxx{% endfor %}{% endfor %}`
rendered a 100 MB string over ~11 s on one render call (proven
empirically) — a CPU/memory denial of service, the bfld-class
"unbounded expansion".

Fix:
- Enable MiniJinja's `fuel` feature and set a per-render instruction
  budget (`set_fuel(Some(1_000_000))`). A nested loop burns one unit
  per iteration, so the budget caps total work regardless of nesting;
  the attack now fails fast (~90 ms) with "engine ran out of fuel".
- Reject template sources over 64 KiB before compilation (defense in
  depth so a pathological literal can neither compile nor emit verbatim).

Legitimate HA templates (a few dozen instructions) are unaffected.

Tests (fail on old — unbounded render / no rejection):
- nested_loop_template_is_bounded_not_unbounded_dos
- single_huge_repeat_template_is_bounded
- oversized_template_source_is_rejected
- legitimate_template_still_renders_within_fuel (no regression)

Co-Authored-By: claude-flow <ruv@ruv.net>

* fix(homecore-automation): stop crafted delay/timeout from panicking the run task (HC-SEC-02)

`Action::Delay { seconds }` and `Action::WaitForTrigger { timeout_seconds }`
fed the user-supplied float straight into `Duration::from_secs_f64`, which
PANICS on negative, NaN, infinite, or overflowing inputs. All of those are
reachable from a crafted (or simply typo'd) automation YAML —
`delay: {seconds: -1}`, `.nan`, `.inf`, `1e308` — so one hostile config
aborts the spawned automation task with a panic
("cannot convert float seconds to Duration: value is negative", proven
empirically).

Fix: a `safe_duration_from_secs` guard that saturates instead of panicking,
matching Home Assistant's lenient "non-positive delay = no delay":
- NaN / ±inf / negative -> Duration::ZERO
- absurdly large (would overflow) -> clamped to ~100 years (MAX_DELAY_SECS)

Tests (fail on old — panic = failure):
- delay_negative_seconds_does_not_panic
- delay_nan_seconds_does_not_panic
- delay_infinite_seconds_does_not_panic
- wait_for_trigger_negative_timeout_does_not_panic
- safe_duration_saturates_hostile_values (incl. overflow clamp)

Co-Authored-By: claude-flow <ruv@ruv.net>

* docs(homecore-automation): record HC-SEC-01/02 security review (CHANGELOG + ADR-129 §8a)

Document the two DoS findings (template unbounded-expansion HC-SEC-01,
delay panic-on-config HC-SEC-02) and the dimensions probed clean
(condition fail-closed, bounded run-modes, sandboxed read-only templates).

Co-Authored-By: claude-flow <ruv@ruv.net>
This commit is contained in:
rUv
2026-06-14 20:22:07 -04:00
committed by GitHub
parent e1f4897269
commit 5bc3b634b7
5 changed files with 218 additions and 4 deletions
+1
View File
@@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- **ADR-260: RuField MFS — the open specification for camera-free multimodal field sensing.** A common event / tensor / calibration / privacy / provenance model that sits *above* WiFi CSI/CIR/BFLD, UWB, BLE Channel Sounding, mmWave radar, ultrasound, subsonic, infrared, and future quantum sensors (each modality emits a normalized `FieldEvent``FieldTensor``FusionGraph``PrivacyClass``ProvenanceReceipt`). Published as a **standalone repo** [`ruvnet/rufield`](https://github.com/ruvnet/rufield) and vendored here as the `vendor/rufield` submodule (the `vendor/rvcsi` pattern — not a `v2/` workspace member). The v0.1 reference stack is a self-contained 6-crate Rust workspace (`rufield-core`, `-provenance` [sha256 + ed25519], `-privacy` [P0P5 guard], `-adapters` [deterministic `SyntheticSim` across wifi_csi/mmwave_radar/infrared_thermal], `-fusion` [graph + TOML weighted-Bayes rules → 7 room-state inferences], `-bench` [deterministic runner + the §31 acceptance test]). **60 tests / 0 failed, clippy-clean.** §27 acceptance criteria 18 and 10 PASS; the live dashboard (9) is deferred. **All benchmark metrics are SYNTHETIC** (scored against the simulator's own ground truth — presence/breathing/bed_exit/room_transition F1 = 1.000, nocturnal_scratch 0.923 reported honestly, p95 latency ~0.01 ms, provenance coverage 100%, 0 privacy violations) — they prove the pipeline recovers known truth, **not** field accuracy; real hardware adapters (ESP32 CSI, mmWave, thermal IR) are a documented roadmap item, none validated in v0.1. The Python deterministic proof is unchanged (rufield is off the signal-processing proof path).
### Security
- **`homecore-automation` security review — two real DoS findings fixed (template unbounded-expansion + delay panic-on-config), each pinned by a fails-on-old test; condition-bypass / fail-closed / action-authz dimensions confirmed clean (ADR-129 §8a).** Beyond-SOTA review of the HA-compat automation engine (the execution/eval surface: triggers → conditions → actions, with user-config Jinja2 templates), un-covered by the ADR-154159 sweep. **HC-SEC-01 (template DoS, HIGH):** a `template:` condition / `value_template` is user config and was rendered with MiniJinja's defaults — **no instruction budget, no output cap**. A single nested-loop condition rendered a **100 MB string in ~11 s on one render call** (measured) — the bfld-class unbounded expansion (MiniJinja's per-call `range()` 10k cap does **not** stop nesting). **Fixed** by enabling MiniJinja's `fuel` feature + `set_fuel(Some(1_000_000))` (the attack now fails fast ~90 ms with "engine ran out of fuel") and a 64 KiB source-length cap; legitimate templates unaffected. **HC-SEC-02 (panic-on-config DoS, MEDIUM):** `Action::Delay`/`WaitForTrigger` fed the user float straight into `Duration::from_secs_f64`, which **panics** on negative/NaN/inf/overflow — all reachable from a crafted or typo'd YAML (`delay: {seconds: -1}`, `.nan`, `.inf`, `1e308`), aborting the spawned run task (measured panic). **Fixed** by a `safe_duration_from_secs` guard that saturates (NaN/±inf/negative → `0`, matching HA's lenient "non-positive delay = no delay"; huge → clamped to ~100 yr). **Dimensions probed clean (evidence in ADR-129 §8a):** condition eval is **fail-closed** (template-render error → `false`; un-parseable `choose` branch condition → branch skipped, never silently passing); run-modes are **bounded** (Single/Restart/Queued/`max:N` — a self-triggering automation does not livelock, ADR-162 tests); templates are **read-only sandboxed** (no service-call/state-set global exposed to template scope, so a template cannot escalate to an action); no `unwrap`/`expect`/index panic reachable from a crafted config in the eval/exec path beyond the fixed `from_secs_f64`. Fails-on-old verified by reverting each fix in isolation (delay tests panic; template nested-loop test runs unbounded >60 s; oversized-source test fails). `cargo test -p homecore-automation --no-default-features`: **40 → 54 passed, 0 failed** (+14: 4 template-DoS, 1 no-regression render, 5 delay/wait + safe-duration unit). Workspace green; Python deterministic proof unchanged (homecore-automation is off the signal proof path).
- **`cog-ha-matter` witness/manifest crypto review — engine-class signed-digest collision confirmed ABSENT (length-prefixing already correct); domain-separation tag ADDED + `verify_strict` HARDENED; key-handling & verify-before-trust confirmed clean (ADR-116 §2.2).** Beyond-SOTA crypto+security review of the Cognitum/HA-Matter bridge's SHA-256 + Ed25519 witness chain — the exact signing chain ADR-262 P2 proposes to reuse — un-covered by the ADR-154159 sweep. **Top-priority check: the sibling `wifi-densepose-engine` bug class (unframed boundary-to-boundary concatenation of operator-influenceable strings into a signed/hashed digest).** Result reported honestly: **that bug class is ABSENT here**`witness::canonical_bytes` already length-prefixes the two variable-length operator-influenceable fields (`kind_len:u32-be ‖ kind`, `payload_len:u32-be ‖ payload`) over fixed-width `prev_hash[32] ‖ seq:u64-be ‖ ts:u64-be`, an injective encoding (proven pre-existing by `canonical_bytes_length_prefixing_prevents_ambiguity`), and `witness_signing::sign_event`/`verify_signature` sign/verify the **identical** bytes the hash chain commits to (no separate unframed concatenation). The manifest `binary_signature` (Ed25519 over the fixed 64-hex-char `binary_sha256`) is signed **at build time by the Makefile**, not in-crate, and over a single fixed-length value — no in-crate manifest-signing concatenation surface. **Two real hardening gaps fixed, the first pinned by fails-on-old tests:**
- **CHM-WIT-01 (missing domain-separation tag, LOW) — ADDED.** The engine review's prescribed fix is "domain-tag **+** length-prefix"; the length-prefix half was present, the **domain tag was absent**. The witness SHA-256 preimage / Ed25519 message carried no tag distinguishing it from any other signing context that shares key infrastructure — notably the manifest `binary_signature`, the very chain ADR-262 P2 reuses. **Fix:** prepend a versioned, NUL-terminated `WITNESS_DOMAIN_TAG = b"cog-ha-matter/witness-event/v1\x00"` to `canonical_bytes` (the doc-comment already anticipated a leading version migration). Cross-protocol separation now holds: a witness signature can never be replayed as a message for another Ed25519 context. **Witness-bytes change by design** (prior on-disk witness hashes/signatures invalidated, like the engine fix) — verified safe: **no in-repo crate consumes cog-ha-matter's witness bytes/signatures programmatically** (all references are doc-comment mentions; the crate is self-contained, no `use cog_ha_matter::` anywhere). Pinned by `canonical_bytes_is_domain_separated`, `canonical_bytes_starts_with_domain_tag_then_prev_hash`, `witness_preimage_cannot_collide_with_a_bare_manifest_digest` (witness.rs) and `signature_commits_to_domain_tag_not_bare_fields` (witness_signing.rs — a signature over the **un-tagged** field concatenation must NOT verify); the domain-separation guard **FAILED on the reverted un-tagged encoding** ("canonical message is not domain-separated").
- **CHM-WIT-02 (permissive Ed25519 verification, LOW) — HARDENED to `verify_strict`.** For a tamper-evident **audit** chain the signature is the attestation, so `verify_signature` now uses `VerifyingKey::verify_strict` (rejects non-canonical encodings + small-order public keys per RFC 8032) instead of the permissive `Verifier::verify` — giving auditors the "one canonical signature per event" property they rely on when comparing/deduplicating signed records. Not a forgery fix (the public key is caller-pinned, never parsed from the event), reported at true LOW severity. Guarded by `verify_uses_strict_path_and_pins_caller_key`.