feat: [US-038] - Fix path traversal vulnerability in browser context_id

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Nathan Flurry 2026-03-17 16:47:39 -07:00
parent ca05ec9c20
commit f55cb03164
2 changed files with 71 additions and 6 deletions

View file

@ -17,6 +17,38 @@ struct ContextMeta {
const META_FILE: &str = "context.json";
/// Validate that a context_id is a valid hex string (generated by us) and that
/// the resulting path does not escape the base directory. Returns the
/// validated, canonical context directory path on success.
pub fn validate_context_id(base: &Path, context_id: &str) -> Result<PathBuf, BrowserProblem> {
// Only hex characters are valid rejects ../, %2F, etc.
if !context_id.chars().all(|c| c.is_ascii_hexdigit()) || context_id.is_empty() {
return Err(BrowserProblem::not_found(format!(
"browser context '{context_id}' not found"
)));
}
let context_dir = base.join(context_id);
// Defence-in-depth: even after the regex check, canonicalize and verify
// the resolved path is still under the base directory.
if context_dir.exists() {
let canonical = context_dir.canonicalize().map_err(|e| {
BrowserProblem::internal_error(format!("failed to canonicalize context path: {e}"))
})?;
let canonical_base = base.canonicalize().map_err(|e| {
BrowserProblem::internal_error(format!("failed to canonicalize base path: {e}"))
})?;
if !canonical.starts_with(&canonical_base) {
return Err(BrowserProblem::not_found(format!(
"browser context '{context_id}' not found"
)));
}
}
Ok(context_dir)
}
/// Return the base directory that holds all browser context directories.
pub fn contexts_base_dir(state_dir: &Path) -> PathBuf {
state_dir.join("browser-contexts")
@ -107,7 +139,7 @@ pub fn create_context(
/// Delete a browser context directory.
pub fn delete_context(state_dir: &Path, context_id: &str) -> Result<(), BrowserProblem> {
let base = contexts_base_dir(state_dir);
let context_dir = base.join(context_id);
let context_dir = validate_context_id(&base, context_id)?;
if !context_dir.exists() {
return Err(BrowserProblem::not_found(format!(
@ -221,4 +253,40 @@ mod tests {
let result = delete_context(tmp.path(), "nonexistent");
assert!(result.is_err());
}
#[test]
fn test_delete_context_path_traversal_dotdot() {
let tmp = TempDir::new().unwrap();
let result = delete_context(tmp.path(), "../../../tmp");
assert!(result.is_err());
}
#[test]
fn test_delete_context_path_traversal_encoded() {
let tmp = TempDir::new().unwrap();
let result = delete_context(tmp.path(), "..%2F..%2Ftmp");
assert!(result.is_err());
}
#[test]
fn test_delete_context_valid_hex_not_found() {
let tmp = TempDir::new().unwrap();
let result = delete_context(tmp.path(), "abcdef0123456789");
assert!(result.is_err());
}
#[test]
fn test_validate_context_id_rejects_empty() {
let tmp = TempDir::new().unwrap();
let base = contexts_base_dir(tmp.path());
assert!(validate_context_id(&base, "").is_err());
}
#[test]
fn test_validate_context_id_rejects_slashes() {
let tmp = TempDir::new().unwrap();
let base = contexts_base_dir(tmp.path());
assert!(validate_context_id(&base, "abc/def").is_err());
assert!(validate_context_id(&base, "abc\\def").is_err());
}
}

View file

@ -899,11 +899,8 @@ impl BrowserRuntime {
// Set user-data-dir for persistent contexts
if let Some(ref context_id) = state.context_id {
let context_dir = self
.config
.state_dir
.join("browser-contexts")
.join(context_id);
let base = crate::browser_context::contexts_base_dir(&self.config.state_dir);
let context_dir = crate::browser_context::validate_context_id(&base, context_id)?;
args.push(format!("--user-data-dir={}", context_dir.display()));
}