security: path traversal via unvalidated context/context_type/context_id in filesystem storage

casper-stevens commented

2026-05-07 08:28:30 +00:00

Member

Summary

Three new file-based storage paths join user-controlled route parameters directly into filesystem paths without any validation:

// save_chat_message
self.db_root.join("chat").join(&msg.context_type).join(&msg.context_sid)

// save_upload
self.db_root.join("uploads").join(context_type).join(context_id)

// link_dir
self.db_root.join(context).join("person_company_links")

If context, context_type, or context_id contain .. or /, an attacker can read or write files outside db_root. The filename sanitizer in save_upload only applies to the filename, not the directory components.

Fix

Validate each path component matches [a-zA-Z0-9_-]+ before joining, or canonicalize and assert the resolved path starts with db_root.

Affected code

crates/hero_biz_ui/src/services/mod.rs — save_chat_message, save_upload, link_dir

Found in

PR #21 review

## Summary Three new file-based storage paths join user-controlled route parameters directly into filesystem paths without any validation: ```rust // save_chat_message self.db_root.join("chat").join(&msg.context_type).join(&msg.context_sid) // save_upload self.db_root.join("uploads").join(context_type).join(context_id) // link_dir self.db_root.join(context).join("person_company_links") ``` If `context`, `context_type`, or `context_id` contain `..` or `/`, an attacker can read or write files outside `db_root`. The filename sanitizer in `save_upload` only applies to the filename, not the directory components. ## Fix Validate each path component matches `[a-zA-Z0-9_-]+` before joining, or canonicalize and assert the resolved path starts with `db_root`. ## Affected code - `crates/hero_biz_ui/src/services/mod.rs` — `save_chat_message`, `save_upload`, `link_dir` ## Found in PR #21 review

casper-stevens referenced this issue

2026-05-07 08:28:52 +00:00

security: path traversal via link_sid in delete_person_company_link #42

casper-stevens commented

2026-05-07 08:56:47 +00:00

Author

Member

Implementation Spec — Issue #41

Objective

Eliminate path traversal vulnerabilities in three Store methods that join user-controlled strings directly into filesystem paths without validation. An attacker who can send arbitrary values for context, context_type, or context_id (all sourced from URL route parameters) can escape db_root and read or write arbitrary files on the server.

Affected Code

crates/hero_biz_ui/src/services/mod.rs — save_chat_message, save_upload, link_dir

Method	Vulnerable join
`save_chat_message`	`.join(&msg.context_type).join(&msg.context_sid)`
`save_upload`	`.join(context_type).join(context_id)`
`link_dir`	`.join(context)`
`delete_person_company_link`	`link_dir(context)` + `.join(link_sid)` (also unvalidated)

Fix Approach

Add a module-scope helper:

fn validate_path_component(s: &str) -> Result<()> {
    if s.is_empty() || !s.chars().all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-') {
        return Err(HeroBizError::Validation(format!(
            "Invalid path component {:?}: must match [a-zA-Z0-9_-]+",
            s
        )));
    }
    Ok(())
}

Call it before every PathBuf::join that uses user-supplied input.

Files to Modify

crates/hero_biz_ui/src/services/mod.rs — all changes

Implementation Steps

Step 1: Add `validate_path_component` helper

Add the function at module scope in services/mod.rs, before impl Store.

Step 2: Update `save_chat_message`

Before the let dir = ... line, add:

validate_path_component(&msg.context_type)?;
validate_path_component(&msg.context_sid)?;

Step 3: Update `save_upload`

Before the let dir = ... line, add:

validate_path_component(context_type)?;
validate_path_component(context_id)?;

Step 4: Update `link_dir` signature

Change return type from PathBuf to Result<PathBuf> and add validation:

fn link_dir(&self, context: &str) -> Result<PathBuf> {
    validate_path_component(context)?;
    Ok(self.db_root.join(context).join("person_company_links"))
}

Step 5–6: Update `link_dir` callers

load_all_person_company_links: let dir = self.link_dir(context)?;
save_person_company_link: let dir = self.link_dir(context)?;

Step 7: Update `delete_person_company_link`

validate_path_component(link_sid)?;
let path = self.link_dir(context)?.join(format!("{}.json", link_sid));

Step 8: Add unit tests

Add #[cfg(test)] module at the bottom of services/mod.rs covering: valid components, .., /, empty string, null byte, and percent-encoded sequences.

Acceptance Criteria

cargo build -p hero_biz_ui passes with no errors
cargo test -p hero_biz_ui passes with all new tests green
Path components containing .., /, spaces, or non-[a-zA-Z0-9_-] characters are rejected before any filesystem access
Normal operation with valid identifiers is unaffected
No public function signatures change

Notes

link_sid in delete_person_company_link is also user-supplied and must be validated (Step 7 covers it — not in original issue scope but same vulnerability class)
The allowlist approach blocks percent-encoded traversal too (% is rejected)
HTTP status for validation errors will remain 500 for now; can be improved separately

## Implementation Spec — Issue #41 ### Objective Eliminate path traversal vulnerabilities in three `Store` methods that join user-controlled strings directly into filesystem paths without validation. An attacker who can send arbitrary values for `context`, `context_type`, or `context_id` (all sourced from URL route parameters) can escape `db_root` and read or write arbitrary files on the server. ### Affected Code `crates/hero_biz_ui/src/services/mod.rs` — `save_chat_message`, `save_upload`, `link_dir` | Method | Vulnerable join | |---|---| | `save_chat_message` | `.join(&msg.context_type).join(&msg.context_sid)` | | `save_upload` | `.join(context_type).join(context_id)` | | `link_dir` | `.join(context)` | | `delete_person_company_link` | `link_dir(context)` + `.join(link_sid)` (also unvalidated) | ### Fix Approach Add a module-scope helper: ```rust fn validate_path_component(s: &str) -> Result<()> { if s.is_empty() || !s.chars().all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-') { return Err(HeroBizError::Validation(format!( "Invalid path component {:?}: must match [a-zA-Z0-9_-]+", s ))); } Ok(()) } ``` Call it before every `PathBuf::join` that uses user-supplied input. ### Files to Modify - `crates/hero_biz_ui/src/services/mod.rs` — all changes ### Implementation Steps #### Step 1: Add `validate_path_component` helper Add the function at module scope in `services/mod.rs`, before `impl Store`. #### Step 2: Update `save_chat_message` Before the `let dir = ...` line, add: ```rust validate_path_component(&msg.context_type)?; validate_path_component(&msg.context_sid)?; ``` #### Step 3: Update `save_upload` Before the `let dir = ...` line, add: ```rust validate_path_component(context_type)?; validate_path_component(context_id)?; ``` #### Step 4: Update `link_dir` signature Change return type from `PathBuf` to `Result<PathBuf>` and add validation: ```rust fn link_dir(&self, context: &str) -> Result<PathBuf> { validate_path_component(context)?; Ok(self.db_root.join(context).join("person_company_links")) } ``` #### Step 5–6: Update `link_dir` callers - `load_all_person_company_links`: `let dir = self.link_dir(context)?;` - `save_person_company_link`: `let dir = self.link_dir(context)?;` #### Step 7: Update `delete_person_company_link` ```rust validate_path_component(link_sid)?; let path = self.link_dir(context)?.join(format!("{}.json", link_sid)); ``` #### Step 8: Add unit tests Add `#[cfg(test)]` module at the bottom of `services/mod.rs` covering: valid components, `..`, `/`, empty string, null byte, and percent-encoded sequences. ### Acceptance Criteria - [ ] `cargo build -p hero_biz_ui` passes with no errors - [ ] `cargo test -p hero_biz_ui` passes with all new tests green - [ ] Path components containing `..`, `/`, spaces, or non-`[a-zA-Z0-9_-]` characters are rejected before any filesystem access - [ ] Normal operation with valid identifiers is unaffected - [ ] No public function signatures change ### Notes - `link_sid` in `delete_person_company_link` is also user-supplied and must be validated (Step 7 covers it — not in original issue scope but same vulnerability class) - The allowlist approach blocks percent-encoded traversal too (`%` is rejected) - HTTP status for validation errors will remain 500 for now; can be improved separately

casper-stevens commented

2026-05-07 09:15:21 +00:00

Author

Member

Test Results

Build: pass

Tests: 7 passed, 0 failed

Test suite: hero_biz_ui unit tests

All 7 unit tests passed:

parser::tests::test_name_fix
services::tests::dot_dot_is_rejected
services::tests::empty_string_is_rejected
services::tests::path_traversal_sequences_rejected
services::tests::null_byte_is_rejected
services::tests::slash_is_rejected
services::tests::valid_components_pass

Doc-tests and binary tests: 0 tests (none defined).

Branch: development_casper

## Test Results **Build:** pass **Tests:** 7 passed, 0 failed **Test suite:** `hero_biz_ui` unit tests All 7 unit tests passed: - `parser::tests::test_name_fix` - `services::tests::dot_dot_is_rejected` - `services::tests::empty_string_is_rejected` - `services::tests::path_traversal_sequences_rejected` - `services::tests::null_byte_is_rejected` - `services::tests::slash_is_rejected` - `services::tests::valid_components_pass` Doc-tests and binary tests: 0 tests (none defined). Branch: `development_casper`

casper-stevens commented

2026-05-07 09:17:26 +00:00

Author

Member

Implementation Complete

Changes Made

All changes are in crates/hero_biz_ui/src/services/mod.rs:

Added module-scope validate_path_component(s: &str) -> Result<()> helper that rejects any string not matching [a-zA-Z0-9_-]+, using the existing HeroBizError::Validation error variant.
save_chat_message: validates context_type and context_sid before joining into path.
save_upload: validates context_type and context_id before joining into path.
link_dir: signature changed from -> PathBuf to -> Result<PathBuf>; validates context before joining.
load_all_person_company_links, save_person_company_link: updated to propagate link_dir's new Result return with ?.
delete_person_company_link: validates link_sid (also user-supplied, same vulnerability class) before joining into path; updated link_dir call.
Added 6 unit tests covering valid inputs, .., /, empty string, null byte, and percent-encoded/spaced traversal sequences.

Test Results

Build: pass
Tests: 7 passed, 0 failed

## Implementation Complete ### Changes Made All changes are in `crates/hero_biz_ui/src/services/mod.rs`: - Added module-scope `validate_path_component(s: &str) -> Result<()>` helper that rejects any string not matching `[a-zA-Z0-9_-]+`, using the existing `HeroBizError::Validation` error variant. - `save_chat_message`: validates `context_type` and `context_sid` before joining into path. - `save_upload`: validates `context_type` and `context_id` before joining into path. - `link_dir`: signature changed from `-> PathBuf` to `-> Result<PathBuf>`; validates `context` before joining. - `load_all_person_company_links`, `save_person_company_link`: updated to propagate `link_dir`'s new `Result` return with `?`. - `delete_person_company_link`: validates `link_sid` (also user-supplied, same vulnerability class) before joining into path; updated `link_dir` call. - Added 6 unit tests covering valid inputs, `..`, `/`, empty string, null byte, and percent-encoded/spaced traversal sequences. ### Test Results Build: pass Tests: 7 passed, 0 failed

casper-stevens referenced this issue from a commit

2026-05-07 09:24:44 +00:00

fix(security): validate path components to prevent directory traversal

casper-stevens commented

2026-05-07 09:34:10 +00:00

Author

Member

Fixed and committed in 0a24041. All validation checks in place.

Fixed and committed in 0a24041. All validation checks in place.

casper-stevens closed this issue

2026-05-07 09:34:31 +00:00

casper-stevens referenced this issue