fix(security): path-traversal guard on 5 sensing-server endpoints (closes #615)#616
Merged
Conversation
…oses #615) Reported by @bannned-bit. Five endpoints in v2/crates/wifi-densepose-sensing-server embedded user-controlled identifiers in format!() paths with no sanitization: recording.rs POST /api/v1/recording/start (session_name) recording.rs GET /api/v1/recording/download/:id (id) recording.rs DELETE /api/v1/recording/delete/:id (id) model_manager.rs POST /api/v1/models/load (model_id) training_api.rs load_recording_frames (dataset_ids[]) Each unauthenticated caller could: - READ arbitrary files via ../../etc/passwd, ../../.env, etc. - WRITE attacker-controlled JSONL via recording/start - LOAD attacker-controlled .rvf model files - DELETE arbitrary files the server process can touch New `path_safety` module exports `safe_id(&str) -> Result<&str, PathSafetyError>` that enforces the rejection envelope BEFORE any user input reaches a format!() that builds a path: - Allowed character set: [A-Za-z0-9._-] - Reject leading '.' (rules out '.', '..', '.env', hidden files) - Reject empty strings - Reject anything > 64 bytes - Reject all whitespace, path separators, null bytes, non-ASCII Applied at all 5 sites. Errors return 400 Bad Request (download) / status:"error" JSON (others) — not panics. 9 unit tests in path_safety::tests cover: - accepts simple alphanumeric / hyphen / underscore / dot - rejects empty, leading dot, path separators ('/', '\'), null byte, whitespace, shell specials, non-ASCII (including fullwidth slash U+FF0F), too-long, boundary at MAX_ID_LEN test result: ok. 9 passed; 0 failed cargo build -p wifi-densepose-sensing-server --no-default-features: 33s Fix-marker RuView#615 in scripts/fix-markers.json prevents removing the guard at any of the 5 call sites. CHANGELOG entry under [Unreleased] / Security documents the patched endpoints and the rejection envelope. Severity: critical per reporter — five remotely-reachable paths to read, write, or delete arbitrary files. Hot per-request paths, not edge cases. Co-Authored-By: claude-flow <ruv@ruv.net>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #615 — credit @bannned-bit for the precise exploit catalogue and the fix template.
Five vulnerable endpoints, all patched
recording.rsPOST /api/v1/recording/startsession_name../../-prefixed namerecording.rsGET /api/v1/recording/download/:idid../../.env,/etc/passwd)recording.rsDELETE /api/v1/recording/delete/:ididmodel_manager.rsPOST /api/v1/models/loadmodel_id.rvffile from arbitrary pathtraining_api.rsload_recording_frames(internal, exposed via/api/v1/train/start)dataset_ids[]All five built filesystem paths via
format!("...{user_input}...")with no validation. Thereplace(' ', "_")inrecording.rsonly normalised whitespace — it did nothing against..,/, null bytes, etc.Fix shape
New
wifi_densepose_sensing_server::path_safetymodule exports:Rejection envelope:
[A-Za-z0-9._-].(rules out.,..,.env, hidden files)MAX_ID_LEN)/,\), null bytes, non-ASCII (including the fullwidth slashU+FF0Fwhich can normalise to/on some filesystems)Applied at all 5 sites before any user input reaches a
format!()that builds a path. Errors return400 Bad Request(download) orstatus: "error"JSON (others) — not panics. Stay consistent with the existing error response shapes.Test coverage
9 unit tests in
path_safety::tests:cargo build -p wifi-densepose-sensing-server --no-default-features— 33s, no errors (one pre-existing unused-import warning unchanged).Regression guard
Fix-marker
RuView#615inscripts/fix-markers.jsonrequires bothpath_safety::safe_id(called sites) andpub fn safe_id(the helper itself). Removing the guard at any of the 5 call sites or deleting the module would fail CI.Why this is severity-critical
All 5 endpoints are unauthenticated when
RUVIEW_API_TOKENis unset (the LAN-mode default from #443). Combined with the recent #580 (host-header allowlist for DNS rebinding) — both classes of attack land same-origin from any page the user visits during a sensing session. This PR closes the filesystem-side hole; #580 already closed the cross-origin reach.Audit completeness
grep -rn 'format!("[^"]*\\{[^}]*_id[^}]*\\}' v2/crates/wifi-densepose-sensing-server/src/— the 5 sites in the PR are the only matches that touch user-controlled identifiers AND build a filesystem path. Otherformat!calls in that crate either build URLs (host_validation), log messages, or use server-internal identifiers.🤖 Generated with claude-flow