security(desktop): IPC serial-command-injection + over-broad shell capability + ADR-178 (#1100)
* fix(security): desktop IPC serial-command-injection + over-broad shell capability (ADR-178) Beyond-SOTA security review of wifi-densepose-desktop (Tauri v2). Two real findings, each MEASURED on Windows (crate builds + tests under --no-default-features): WDP-DESK-01 (MODERATE) — serial command injection via configure_esp32_wifi. The #[tauri::command] handler concatenated webview-supplied ssid/password into newline-terminated serial commands with no validation; a \r\n let a compromised webview inject an arbitrary follow-up firmware command (reboot/erase). Added validate_wifi_credentials() enforcing WPA2 length bounds and rejecting all control characters, called fail-closed before any serial write. Pinned by 3 new tests (rejects \r\n / \n / NUL injection, rejects out-of-range, accepts valid boundaries). WDP-DESK-02 (MODERATE) — removed unused shell:allow-execute / shell:allow-open from capabilities/default.json. The Rust backend spawns processes via std::process::Command (bypassing the allowlist) and the UI only uses dialog.open; the shell perms were unused privilege granting the webview arbitrary host command execution on compromise. Regenerated capabilities.json confirms only core:default + dialog perms remain. lib tests 18 -> 21 (+3 pins), integration 21 -> 21, 0 failed. Python deterministic proof unchanged (f8e76f21...46f7a; desktop off the signal path). Co-Authored-By: claude-flow <ruv@ruv.net> * docs(adr): ADR-178 — desktop IPC injection fix + capability least-privilege Records the 2 MEASURED MODERATE fixes in feddcde9d: WDP-DESK-01 (webview ssid/password \r\n-injected arbitrary firmware serial commands → validated fail-closed) and WDP-DESK-02 (unused shell:allow-execute/open capability granted to the webview → removed). 30-command IPC surface + capability scope audited; 6 dimensions clean-with-evidence. desktop 18→21. Co-Authored-By: claude-flow <ruv@ruv.net>
This commit is contained in:
parent
20ad75f30c
commit
10c813fde3
File diff suppressed because one or more lines are too long
|
|
@ -0,0 +1,87 @@
|
|||
# ADR-178: `wifi-densepose-desktop` IPC Injection Fix + Capability Least-Privilege
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| **Status** | Accepted — 2 real MODERATE bugs fixed + pinned (MEASURED on Windows) |
|
||||
| **Date** | 2026-06-15 |
|
||||
| **Deciders** | ruv |
|
||||
| **Codename** | **DESK-LOCKDOWN** |
|
||||
| **Reviews** | `wifi-densepose-desktop` (Tauri v2 desktop app) |
|
||||
| **Milestone** | #9 (ungated-crate security sweep) — crate 3 of 4 |
|
||||
|
||||
## Context
|
||||
|
||||
`wifi-densepose-desktop` is the Tauri v2 desktop app (ESP32 discovery, firmware
|
||||
flashing, OTA, provisioning, server control). The real attack surface is the
|
||||
**Tauri IPC boundary** — `#[tauri::command]` handlers that take arguments from the
|
||||
webview/JS — and the **capability/allowlist scope**. The crate **builds and tests
|
||||
on Windows** (Tauri 2.10.3, webview2 path, no GTK), so both findings are MEASURED,
|
||||
not source-analysis-only.
|
||||
|
||||
## Decision
|
||||
|
||||
Fix the two real findings; attest the rest of the surface clean with evidence.
|
||||
|
||||
### Findings fixed (both MEASURED)
|
||||
|
||||
| # | Severity | Location | Issue | Fix |
|
||||
|---|----------|----------|-------|-----|
|
||||
| WDP-DESK-01 | MODERATE | `src/commands/discovery.rs:438` (`configure_esp32_wifi`) | Webview-supplied `ssid`/`password` are concatenated into newline-terminated serial commands (`wifi_config {} {}\r\n`, `set ssid {}\r\n`) with **no validation** → a `\r\n` in either field **injects an arbitrary follow-up firmware command** (`reboot`, `erase_nvs`) across the IPC trust boundary. | `validate_wifi_credentials()` — WPA2 length bounds (SSID 1–32, password 8–63) **+ reject all control chars** (`char::is_control()`), called fail-closed before any serial write. |
|
||||
| WDP-DESK-02 | MODERATE | `capabilities/default.json:7-8` | `shell:allow-execute` + `shell:allow-open` granted to the webview but **unused** (Rust spawns via `std::process::Command`; the UI uses only `dialog.open`). A webview compromise (a UI-dependency XSS) → arbitrary **unscoped host command execution**. | Removed both `shell:` permissions (kept `core:default` + the two in-use `dialog:` perms); regenerated `gen/schemas/capabilities.json` now asserts `["core:default","dialog:allow-open","dialog:allow-save"]`. |
|
||||
|
||||
Both are MODERATE (not HIGH): each requires a webview compromise or a malicious
|
||||
local caller to weaponize. The unifying lesson is **least privilege at the IPC
|
||||
boundary** — validate every webview-supplied argument that reaches a serial/FS/
|
||||
process sink, and grant only the capabilities actually exercised.
|
||||
|
||||
### Tauri-command + capability audit (every handler)
|
||||
|
||||
All 30+ command handlers were mapped. Only `configure_esp32_wifi` lacked input
|
||||
validation on a string that reached a command sink (WDP-DESK-01). Every
|
||||
subprocess uses `Command::new(prog).args([...])` (argv vector — no shell-string
|
||||
interpolation), so `port`/`source`/`chip`/`baud` cannot inject a second command
|
||||
even unvalidated. `tauri.conf.json` ships **no** `fs`/`http` plugin and **no**
|
||||
`"all":true`/`"$HOME/**"` scope; after WDP-DESK-02 the allowlist is minimal.
|
||||
|
||||
### Dimensions confirmed clean (with evidence)
|
||||
|
||||
1. **Directory traversal / arbitrary file** — path args (`firmware_path`/`wasm_path`)
|
||||
are blobs the local user selects via the native `dialog.open` picker; settings
|
||||
I/O is a fixed filename under `app_data_dir`. No attacker-named path sink.
|
||||
2. **Shell-string injection** — every subprocess is an argv vector; grep found no
|
||||
shell-string interpolation anywhere.
|
||||
3. **SSRF-to-secret** — `node_ip`-built URLs target the local ESP32 mesh and return
|
||||
only device status JSON; no credential returned to the webview.
|
||||
4. **Panic-on-input** — handlers use `.map_err(|e| e.to_string())?`; the one
|
||||
`expect` is guarded by an `is_none()` early-return; provision/discovery
|
||||
deserializers bounds-check every slice index (NVS size capped ≤ 4096).
|
||||
5. **Hardcoded secrets** — `ota_psk` is a per-call `Option<String>`, never embedded;
|
||||
grep for embedded keys/tokens over `src/` is empty.
|
||||
6. **Shell plugin genuinely unused** — `tauri_plugin_shell` is `init()`-ed but its
|
||||
`Command`/`open` API is never invoked from Rust or the TS UI (which imports only
|
||||
`@tauri-apps/plugin-dialog`) — confirming WDP-DESK-02 is safe to remove.
|
||||
|
||||
## Validation
|
||||
|
||||
- `cargo check -p wifi-densepose-desktop --no-default-features` → `Finished` (Windows, MEASURED).
|
||||
- `cargo test -p wifi-densepose-desktop --no-default-features` → lib **18 → 21** (+3 validator pins:
|
||||
`test_validate_wifi_credentials_rejects_injection` / `_rejects_out_of_range` / `_accepts_valid`),
|
||||
integration 21/21, **0 failed**.
|
||||
- Capability narrowing MEASURED: regenerated `capabilities.json` permission set verified.
|
||||
- `python archive/v1/data/proof/verify.py` → **VERDICT: PASS**, hash `f8e76f21…46f7a`
|
||||
unchanged (desktop off the signal proof path).
|
||||
|
||||
## Consequences
|
||||
|
||||
### Positive
|
||||
- An IPC serial-command-injection path and an over-broad shell capability are
|
||||
closed in the desktop app, each pinned / verified, with the rest of the
|
||||
30-command IPC surface attested clean.
|
||||
|
||||
### Negative / Neutral
|
||||
- None. The removed shell capability was unused; the validator rejects only
|
||||
malformed/hostile credentials.
|
||||
|
||||
## Links
|
||||
- ADR-176 / ADR-177 — sibling Milestone-#9 reviews (ruview-swarm, nvsim)
|
||||
- ADR-172 — core/cli review
|
||||
|
|
@ -4,8 +4,6 @@
|
|||
"windows": ["main"],
|
||||
"permissions": [
|
||||
"core:default",
|
||||
"shell:allow-execute",
|
||||
"shell:allow-open",
|
||||
"dialog:allow-open",
|
||||
"dialog:allow-save"
|
||||
]
|
||||
|
|
|
|||
|
|
@ -1 +1 @@
|
|||
{"default":{"identifier":"default","description":"RuView default capability set","local":true,"windows":["main"],"permissions":["core:default","shell:allow-execute","shell:allow-open","dialog:allow-open","dialog:allow-save"]}}
|
||||
{"default":{"identifier":"default","description":"RuView default capability set","local":true,"windows":["main"],"permissions":["core:default","dialog:allow-open","dialog:allow-save"]}}
|
||||
|
|
@ -430,6 +430,35 @@ fn is_esp32_compatible(vid: u16, pid: u16) -> bool {
|
|||
false
|
||||
}
|
||||
|
||||
/// Validate WiFi credentials before they are interpolated into a
|
||||
/// newline-delimited serial command protocol.
|
||||
///
|
||||
/// The ESP32 firmware accepts line-oriented commands such as
|
||||
/// `wifi_config <ssid> <password>\r\n`. Because the SSID and password
|
||||
/// arrive from the webview (untrusted) and are concatenated directly into
|
||||
/// those command strings, a control character (`\r`, `\n`, or NUL) embedded
|
||||
/// in either field would let a malicious caller terminate the current line
|
||||
/// early and inject an arbitrary follow-up command (e.g. `reboot`, `erase`).
|
||||
///
|
||||
/// Enforce the IEEE 802.11 / WPA2 bounds and reject any control characters:
|
||||
/// - SSID: 1-32 bytes, no control characters
|
||||
/// - Password: 8-63 bytes (WPA2 PSK ASCII range), no control characters
|
||||
fn validate_wifi_credentials(ssid: &str, password: &str) -> Result<(), String> {
|
||||
if ssid.is_empty() || ssid.len() > 32 {
|
||||
return Err("SSID must be 1-32 characters".into());
|
||||
}
|
||||
if password.len() < 8 || password.len() > 63 {
|
||||
return Err("WiFi password must be 8-63 characters".into());
|
||||
}
|
||||
if ssid.chars().any(|c| c.is_control()) {
|
||||
return Err("SSID must not contain control characters".into());
|
||||
}
|
||||
if password.chars().any(|c| c.is_control()) {
|
||||
return Err("WiFi password must not contain control characters".into());
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Configure WiFi credentials on an ESP32 via serial port.
|
||||
///
|
||||
/// Sends WiFi credentials to the ESP32 using a simple serial protocol.
|
||||
|
|
@ -443,6 +472,10 @@ pub async fn configure_esp32_wifi(
|
|||
use std::io::{Read, Write};
|
||||
use std::time::Duration;
|
||||
|
||||
// Reject control characters / out-of-range lengths before the credentials
|
||||
// are spliced into the line-oriented serial command protocol below.
|
||||
validate_wifi_credentials(&ssid, &password)?;
|
||||
|
||||
tracing::info!("Configuring WiFi on port: {}", port);
|
||||
|
||||
// Open serial port
|
||||
|
|
@ -549,6 +582,37 @@ mod tests {
|
|||
assert_eq!(node.tdm_total, Some(4));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_wifi_credentials_accepts_valid() {
|
||||
assert!(validate_wifi_credentials("MyNetwork", "password123").is_ok());
|
||||
// Boundary: 32-char SSID, 63-char password are allowed.
|
||||
assert!(validate_wifi_credentials(&"A".repeat(32), &"B".repeat(63)).is_ok());
|
||||
// Boundary: 8-char password (WPA2 minimum) is allowed.
|
||||
assert!(validate_wifi_credentials("net", "12345678").is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_wifi_credentials_rejects_injection() {
|
||||
// Newline/CR in SSID would terminate the serial command line early and
|
||||
// let the caller inject a follow-up firmware command. Must be rejected.
|
||||
assert!(validate_wifi_credentials("net\r\nreboot", "password123").is_err());
|
||||
assert!(validate_wifi_credentials("net\ninjected", "password123").is_err());
|
||||
// Same vector via the password field.
|
||||
assert!(validate_wifi_credentials("net", "pass\r\nerase_nvs").is_err());
|
||||
// Embedded NUL.
|
||||
assert!(validate_wifi_credentials("net", "pass\0word1").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_wifi_credentials_rejects_out_of_range() {
|
||||
// Empty / over-length SSID.
|
||||
assert!(validate_wifi_credentials("", "password123").is_err());
|
||||
assert!(validate_wifi_credentials(&"A".repeat(33), "password123").is_err());
|
||||
// Too-short / too-long password (WPA2 PSK bounds).
|
||||
assert!(validate_wifi_credentials("net", "short").is_err());
|
||||
assert!(validate_wifi_credentials("net", &"B".repeat(64)).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_esp32_compatible() {
|
||||
// CP2102
|
||||
|
|
|
|||
Loading…
Reference in New Issue