Skip to content

Commit e35b2f9

Browse files
committed
Merge fix/security-hardening: cache path traversal, firewall shell injection fixes
- Cache: sanitize name/version/hash to prevent path traversal; atomic write-then-rename - Firewall: eliminate shell injection in container mode by passing cp args directly - Process firewall: use UUID temp filenames and explicit error handling - Remove unused DEEP_SKIP_DIRS constant and dead component_type field
2 parents 2e0eba1 + 0a28853 commit e35b2f9

4 files changed

Lines changed: 62 additions & 43 deletions

File tree

src/cache/mod.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,15 @@ impl CspCache {
3131
}
3232

3333
fn cache_path(&self, name: &str, version: &str, content_hash: &str) -> PathBuf {
34-
self.dir
35-
.join(format!("{name}@{version}-{content_hash}.json"))
34+
// Sanitize components to prevent path traversal. Replace any character
35+
// that could escape the cache directory (path separators, `..`) with `_`.
36+
let safe = |s: &str| s.replace(['/', '\\'], "_").replace("..", "_");
37+
self.dir.join(format!(
38+
"{}-{}-{}.json",
39+
safe(name),
40+
safe(version),
41+
safe(content_hash)
42+
))
3643
}
3744

3845
pub fn get(&self, name: &str, version: &str, content_hash: &str) -> Option<CspSpec> {
@@ -51,7 +58,11 @@ impl CspCache {
5158
std::fs::create_dir_all(&self.dir)?;
5259
let path = self.cache_path(name, version, content_hash);
5360
let json = serde_json::to_string_pretty(csp)?;
54-
std::fs::write(path, json)?;
61+
// Write atomically: write to a temp file then rename into place so
62+
// concurrent readers never see a partial write.
63+
let tmp_path = path.with_extension("tmp");
64+
std::fs::write(&tmp_path, json)?;
65+
std::fs::rename(&tmp_path, &path)?;
5566
Ok(())
5667
}
5768
}

src/firewall/mod.rs

Lines changed: 40 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -65,25 +65,43 @@ fn cross_firewall_context(csp: CspSpec) -> (CspSpec, AuditEvent) {
6565
async fn cross_firewall_process(csp: CspSpec) -> (CspSpec, AuditEvent) {
6666
let temp_dir = std::env::temp_dir().join("phalus-firewall");
6767
let _ = tokio::fs::create_dir_all(&temp_dir).await;
68-
let safe_name = csp
69-
.package_name
70-
.replace(['/', '\\'], "_")
71-
.replace("..", "_");
72-
let safe_version = csp
73-
.package_version
74-
.replace(['/', '\\'], "_")
75-
.replace("..", "_");
76-
let temp_path = temp_dir.join(format!("csp-{}-{}.json", safe_name, safe_version));
68+
let run_id = uuid::Uuid::new_v4();
69+
let temp_path = temp_dir.join(format!("csp-{}.json", run_id));
7770

7871
// Serialize to disk
79-
let serialized = serde_json::to_string_pretty(&csp).unwrap_or_default();
80-
let _ = tokio::fs::write(&temp_path, &serialized).await;
72+
let serialized = match serde_json::to_string_pretty(&csp) {
73+
Ok(s) => s,
74+
Err(e) => {
75+
tracing::error!("process firewall: failed to serialize CSP: {}", e);
76+
let _ = tokio::fs::remove_file(&temp_path).await;
77+
return cross_firewall_context(csp);
78+
}
79+
};
80+
if let Err(e) = tokio::fs::write(&temp_path, &serialized).await {
81+
tracing::error!("process firewall: failed to write temp file: {}", e);
82+
return cross_firewall_context(csp);
83+
}
8184

8285
// Read back from disk (proving serialization boundary)
83-
let read_back = tokio::fs::read_to_string(&temp_path)
84-
.await
85-
.unwrap_or(serialized);
86-
let deserialized: CspSpec = serde_json::from_str(&read_back).unwrap_or(csp);
86+
let read_back = match tokio::fs::read_to_string(&temp_path).await {
87+
Ok(s) => s,
88+
Err(e) => {
89+
tracing::error!("process firewall: failed to read temp file: {}", e);
90+
let _ = tokio::fs::remove_file(&temp_path).await;
91+
return cross_firewall_context(csp);
92+
}
93+
};
94+
let deserialized: CspSpec = match serde_json::from_str(&read_back) {
95+
Ok(c) => c,
96+
Err(e) => {
97+
tracing::error!(
98+
"process firewall: failed to deserialize CSP from disk: {}",
99+
e
100+
);
101+
let _ = tokio::fs::remove_file(&temp_path).await;
102+
return cross_firewall_context(csp);
103+
}
104+
};
87105

88106
// Clean up temp file
89107
let _ = tokio::fs::remove_file(&temp_path).await;
@@ -202,10 +220,10 @@ async fn cross_firewall_container(csp: CspSpec, cfg: &ContainerConfig) -> (CspSp
202220
let input_mount = format!("{}:/input:ro", input_abs.display());
203221
let output_mount = format!("{}:/output:rw", output_abs.display());
204222
// Copy exactly the CSP file across the Docker boundary.
205-
let container_cmd = format!(
206-
"cp /input/{csp_filename} /output/{csp_filename}",
207-
csp_filename = csp_filename
208-
);
223+
// Pass cp arguments directly (no shell) to prevent shell injection via
224+
// package name or version containing metacharacters like $(), backticks, etc.
225+
let input_file = format!("/input/{}", csp_filename);
226+
let output_file = format!("/output/{}", csp_filename);
209227

210228
tracing::info!(
211229
"container firewall: starting container for {}@{} (image={}, network={}, memory={}, cpus={})",
@@ -236,9 +254,9 @@ async fn cross_firewall_container(csp: CspSpec, cfg: &ContainerConfig) -> (CspSp
236254
"--stop-timeout",
237255
&cfg.timeout_secs.to_string(),
238256
&cfg.image,
239-
"sh",
240-
"-c",
241-
&container_cmd,
257+
"cp",
258+
&input_file,
259+
&output_file,
242260
])
243261
.stdout(std::process::Stdio::null())
244262
.stderr(std::process::Stdio::piped())

src/sbom/mod.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
use crate::license;
21
/// SBOM ingestion: parse CycloneDX BOM JSON and SPDX JSON into `ScannedPackage` lists.
2+
use crate::license;
33
use crate::{Ecosystem, ScannedPackage};
44
use serde::Deserialize;
55
use thiserror::Error;
@@ -25,9 +25,6 @@ struct CycloneDxBom {
2525
#[derive(Debug, Deserialize)]
2626
#[serde(rename_all = "camelCase")]
2727
struct CycloneDxComponent {
28-
#[serde(rename = "type")]
29-
#[allow(dead_code)]
30-
component_type: Option<String>,
3128
name: String,
3229
version: Option<String>,
3330
purl: Option<String>,

src/scan/mod.rs

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -137,16 +137,13 @@ fn collect_files(
137137
}
138138

139139
if path.is_dir() {
140-
// Recurse, but only one level for common vendor/dependency dirs
141-
if !DEEP_SKIP_DIRS.contains(&file_name) {
142-
collect_files(
143-
&path,
144-
manifest_files,
145-
sbom_files,
146-
package_refs,
147-
sbom_packages,
148-
)?;
149-
}
140+
collect_files(
141+
&path,
142+
manifest_files,
143+
sbom_files,
144+
package_refs,
145+
sbom_packages,
146+
)?;
150147
continue;
151148
}
152149

@@ -219,10 +216,6 @@ const SKIP_DIRS: &[&str] = &[
219216
"env",
220217
];
221218

222-
/// Directories we recurse into but don't look for manifests at deeper levels
223-
/// (prevents picking up nested test-fixture package.json etc.).
224-
const DEEP_SKIP_DIRS: &[&str] = &[];
225-
226219
// ---------------------------------------------------------------------------
227220
// Registry license resolution
228221
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)