fix(peer): apply catalog portability rules in validate_relative_path
Security audit finding EXP2-SEC-03. `path_validation.rs` guarded against traversal, UNC prefixes, drive letters and symlink escapes, but unlike the catalog validators in lanspread-db it did not reject Windows device names (CON, NUL, COM1..9, LPT1..9), components with a trailing dot or space, reserved characters (`<>:"|?*`), or control characters. On Windows, opening `NUL.txt` talks to a device and `file.txt.` is silently rewritten to `file.txt`, so such names must never reach the filesystem. The catalog component validator is now exported from lanspread-db as `validate_portable_component` and applied to every normal component in `validate_relative_path`. The only current caller is Stream Install's staging-path resolution, whose inputs are already canonical catalog paths, so this changes nothing for valid archives; it removes a divergence between two validators that are supposed to agree. Test plan: `just test` (new cases cover device names in any position, trailing dot/space, a reserved character and a control character, and confirm `console.txt` and `com10.txt` stay valid). Claude-Session: https://claude.ai/code/session_017C3Nbgwpdm3YNwZhhFLHwg
This commit is contained in:
@@ -41,7 +41,7 @@ pub use model::{
|
|||||||
MAX_CATALOG_PATH_BYTES,
|
MAX_CATALOG_PATH_BYTES,
|
||||||
MAX_CATALOG_TOTAL_BYTES,
|
MAX_CATALOG_TOTAL_BYTES,
|
||||||
};
|
};
|
||||||
pub use path::CanonicalCatalogPath;
|
pub use path::{CanonicalCatalogPath, validate_portable_component};
|
||||||
pub use store::{
|
pub use store::{
|
||||||
CATALOG_PUBLICATION_MARKER_NAME,
|
CATALOG_PUBLICATION_MARKER_NAME,
|
||||||
CatalogManifestStore,
|
CatalogManifestStore,
|
||||||
|
|||||||
@@ -164,6 +164,17 @@ fn validate_path(path: &str) -> eyre::Result<()> {
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Validates one path component under the portable catalog rules: no
|
||||||
|
/// `.`/`..`, at most 255 bytes, no trailing dot or space, no control or
|
||||||
|
/// Windows-reserved characters, and no Windows device name stem.
|
||||||
|
///
|
||||||
|
/// # Errors
|
||||||
|
///
|
||||||
|
/// Returns the first violated rule.
|
||||||
|
pub fn validate_portable_component(component: &str) -> eyre::Result<()> {
|
||||||
|
validate_component(component)
|
||||||
|
}
|
||||||
|
|
||||||
fn validate_component(component: &str) -> eyre::Result<()> {
|
fn validate_component(component: &str) -> eyre::Result<()> {
|
||||||
if component.is_empty() || matches!(component, "." | "..") {
|
if component.is_empty() || matches!(component, "." | "..") {
|
||||||
eyre::bail!("catalog path contains a non-canonical component: {component:?}");
|
eyre::bail!("catalog path contains a non-canonical component: {component:?}");
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
use std::path::{Component, Path, PathBuf};
|
use std::path::{Component, Path, PathBuf};
|
||||||
|
|
||||||
use eyre::WrapErr;
|
use eyre::WrapErr;
|
||||||
|
use lanspread_db::content_manifest::validate_portable_component;
|
||||||
|
|
||||||
fn canonicalize_base_dir(base_dir: &Path) -> eyre::Result<PathBuf> {
|
fn canonicalize_base_dir(base_dir: &Path) -> eyre::Result<PathBuf> {
|
||||||
if !base_dir.is_absolute() {
|
if !base_dir.is_absolute() {
|
||||||
@@ -59,6 +60,16 @@ pub fn validate_relative_path(base_dir: &Path, relative_path: &str) -> eyre::Res
|
|||||||
}
|
}
|
||||||
Component::CurDir => {}
|
Component::CurDir => {}
|
||||||
Component::Normal(part) => {
|
Component::Normal(part) => {
|
||||||
|
// Apply the same portability rules as the catalog manifests
|
||||||
|
// (Windows device names, trailing dots/spaces, reserved and
|
||||||
|
// control characters) so a name that the Win32 API would
|
||||||
|
// silently rewrite or route to a device never reaches the
|
||||||
|
// filesystem.
|
||||||
|
let component = part.to_str().ok_or_else(|| {
|
||||||
|
eyre::eyre!("Path component is not valid UTF-8: {relative_path}")
|
||||||
|
})?;
|
||||||
|
validate_portable_component(component)
|
||||||
|
.wrap_err_with(|| format!("Path component is not portable: {relative_path}"))?;
|
||||||
resolved.push(part);
|
resolved.push(part);
|
||||||
|
|
||||||
if let Ok(metadata) = std::fs::symlink_metadata(&resolved)
|
if let Ok(metadata) = std::fs::symlink_metadata(&resolved)
|
||||||
@@ -132,6 +143,31 @@ mod tests {
|
|||||||
assert!(validate_game_file_path(base, "../../etc/passwd").is_err());
|
assert!(validate_game_file_path(base, "../../etc/passwd").is_err());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_windows_device_names_and_trailing_dots_rejected() {
|
||||||
|
let temp_dir = TempDir::new("lanspread-path-validation");
|
||||||
|
let base = temp_dir.path();
|
||||||
|
|
||||||
|
for path in [
|
||||||
|
"CON",
|
||||||
|
"nul.txt",
|
||||||
|
"sub/COM1",
|
||||||
|
"LPT1.log",
|
||||||
|
"trailing.",
|
||||||
|
"trailing ",
|
||||||
|
"sub./file",
|
||||||
|
"bad:name",
|
||||||
|
"ctrl\u{7}char",
|
||||||
|
] {
|
||||||
|
assert!(
|
||||||
|
validate_game_file_path(base, path).is_err(),
|
||||||
|
"accepted {path:?}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
assert!(validate_game_file_path(base, "console.txt").is_ok());
|
||||||
|
assert!(validate_game_file_path(base, "com10.txt").is_ok());
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_double_dot_in_filename_allowed() {
|
fn test_double_dot_in_filename_allowed() {
|
||||||
let temp_dir = TempDir::new("lanspread-path-validation");
|
let temp_dir = TempDir::new("lanspread-path-validation");
|
||||||
|
|||||||
Reference in New Issue
Block a user