From 20d700ee4f96f00b831a2baa3afeb892f68bf20e Mon Sep 17 00:00:00 2001 From: Rabindra Dhakal Date: Thu, 16 Jul 2026 23:03:10 +0545 Subject: [PATCH] fix(security): validate pkg_name and pkg_id as path components --- crates/soar-core/src/package/url.rs | 45 +++++++++++++++++++++++ crates/soar-db/src/repository/metadata.rs | 13 +++++++ crates/soar-operations/src/install.rs | 10 +++++ 3 files changed, 68 insertions(+) diff --git a/crates/soar-core/src/package/url.rs b/crates/soar-core/src/package/url.rs index 78bbf837..3aa6db96 100644 --- a/crates/soar-core/src/package/url.rs +++ b/crates/soar-core/src/package/url.rs @@ -3,6 +3,7 @@ use std::sync::OnceLock; use regex::Regex; +use soar_utils::path::is_safe_component; use crate::{database::models::Package, error::SoarError, SoarResult}; @@ -84,6 +85,27 @@ impl UrlPackage { } } + /// Rejects a name or id that is not usable as a single path component. + /// + /// Both are joined into the install dir and interpolated into resource + /// paths, so a caller-supplied override containing `/` or `..` would escape + /// it. The derived defaults are always dot-separated and unaffected. + fn validate_names(pkg_name: &str, pkg_id: &str) -> SoarResult<()> { + if !is_safe_component(pkg_name) { + return Err(SoarError::Custom(format!( + "Invalid package name '{}': must be a single path component", + pkg_name + ))); + } + if !is_safe_component(pkg_id) { + return Err(SoarError::Custom(format!( + "Invalid package id '{}': must be a single path component", + pkg_id + ))); + } + Ok(()) + } + /// Parse a GHCR reference and extract package metadata. pub fn from_ghcr( reference: &str, @@ -129,6 +151,8 @@ impl UrlPackage { let pkg_type = pkg_type_override.map(|s| s.to_lowercase()); + Self::validate_names(&pkg_name, &pkg_id)?; + Ok(Self { url: reference.to_string(), pkg_name, @@ -207,6 +231,8 @@ impl UrlPackage { } }); + Self::validate_names(&pkg_name, &pkg_id)?; + Ok(Self { url: url.to_string(), pkg_name, @@ -564,6 +590,25 @@ mod tests { assert!(pkg.is_ghcr); } + #[test] + fn test_ghcr_rejects_traversal_overrides() { + let ghcr = "ghcr.io/org/repo:v1.0"; + + assert!(UrlPackage::from_ghcr(ghcr, Some("../../evil"), None, None, None).is_err()); + assert!(UrlPackage::from_ghcr(ghcr, None, None, None, Some("../../evil")).is_err()); + assert!(UrlPackage::from_ghcr(ghcr, None, None, None, Some("/abs/evil")).is_err()); + assert!(UrlPackage::from_ghcr(ghcr, Some(".."), None, None, None).is_err()); + } + + #[test] + fn test_url_rejects_traversal_overrides() { + let url = "https://github.com/user/repo/releases/download/v1.0/app"; + + assert!(UrlPackage::from_url(url, Some("../../evil"), None, None, None).is_err()); + assert!(UrlPackage::from_url(url, None, None, None, Some("../../evil")).is_err()); + assert!(UrlPackage::from_url(url, None, None, None, Some("a/b")).is_err()); + } + #[test] fn test_ghcr_to_package() { let ghcr = "ghcr.io/pkgforge/soar:v0.8.1"; diff --git a/crates/soar-db/src/repository/metadata.rs b/crates/soar-db/src/repository/metadata.rs index 436346cd..cc1854a8 100644 --- a/crates/soar-db/src/repository/metadata.rs +++ b/crates/soar-db/src/repository/metadata.rs @@ -6,6 +6,7 @@ use diesel::{dsl::sql, prelude::*, sql_types::Text}; use regex::Regex; use serde_json::json; use soar_registry::RemotePackage; +use soar_utils::path::is_safe_component; use tracing::{debug, trace, warn}; /// Regex for extracting name and contact from maintainer string format "Name (contact)". @@ -524,6 +525,18 @@ impl MetadataRepository { version = package.version, "inserting remote package" ); + + // pkg_name and pkg_id are joined into the install dir and interpolated + // into resource paths, so a name with separators or '..' would escape it. + if !is_safe_component(&package.pkg_name) || !is_safe_component(&package.pkg_id) { + warn!( + pkg_id = package.pkg_id, + pkg_name = package.pkg_name, + "skipping package with unsafe path component in pkg_name/pkg_id" + ); + return Ok(()); + } + let provides = package.provides.as_ref().map(|vec| { vec.iter() .map(|p| PackageProvide::from_string(p)) diff --git a/crates/soar-operations/src/install.rs b/crates/soar-operations/src/install.rs index f8ff7d84..dd342042 100644 --- a/crates/soar-operations/src/install.rs +++ b/crates/soar-operations/src/install.rs @@ -35,6 +35,7 @@ use soar_package::integrate_package; use soar_utils::{ hash::{calculate_checksum, hash_string}, lock::FileLock, + path::is_safe_component, pattern::apply_sig_variants, }; use tokio::sync::Semaphore; @@ -830,6 +831,15 @@ async fn install_single_package( hash_string(&input)[..12].to_string() }); + // pkg_name/pkg_id are joined into install_dir and interpolated into resource + // paths downstream, so they must not be able to escape the packages dir. + if !is_safe_component(&pkg.pkg_name) || !is_safe_component(&pkg.pkg_id) { + return Err(SoarError::Custom(format!( + "Refusing to install {}#{}: package name or id is not a valid path component", + pkg.pkg_name, pkg.pkg_id + ))); + } + let install_dir = config .get_packages_path(target.profile.clone())? .join(format!("{}-{}-{}", pkg.pkg_name, pkg.pkg_id, dir_suffix));