Skip to content

Commit d4d834a

Browse files
committed
fix(cortex-update): address security vulnerabilities
- Remove blanket warning suppression in lib.rs - Add SAFETY comment for unsafe MoveFileExW call in install.rs - Add path traversal protection in extract_tar_gz and extract_zip - Use cryptographically secure random bytes (getrandom) for temp dir - Add HTTPS URL validation with localhost exception for dev - Log warning when config parsing fails
1 parent e6658f8 commit d4d834a

6 files changed

Lines changed: 95 additions & 17 deletions

File tree

src/cortex-update/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ flate2 = "1.0"
3939
zip = "2.2"
4040
tar = "0.4"
4141

42+
# Secure random
43+
getrandom = "0.2"
44+
4245
# Self-replacement
4346
self-replace = "1.5"
4447

src/cortex-update/src/api.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,18 @@ impl CortexSoftwareClient {
9595

9696
/// Create a new client with a custom URL.
9797
pub fn with_url(base_url: String) -> Self {
98+
// Validate URL uses HTTPS for security (allow http for localhost development only)
99+
let url_lower = base_url.to_lowercase();
100+
if !url_lower.starts_with("https://")
101+
&& !url_lower.starts_with("http://localhost")
102+
&& !url_lower.starts_with("http://127.0.0.1")
103+
{
104+
tracing::warn!(
105+
"Custom update URL does not use HTTPS. This could allow man-in-the-middle attacks: {}",
106+
base_url
107+
);
108+
}
109+
98110
let client = create_client_builder()
99111
.build()
100112
.unwrap_or_else(|_| Client::new());

src/cortex-update/src/config.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,15 @@ impl UpdateConfig {
108108

109109
if let Some(path) = config_path {
110110
if let Ok(content) = std::fs::read_to_string(&path) {
111-
if let Ok(config) = serde_json::from_str(&content) {
112-
return config;
111+
match serde_json::from_str(&content) {
112+
Ok(config) => return config,
113+
Err(e) => {
114+
tracing::warn!(
115+
"Failed to parse update config at {}: {}. Using defaults.",
116+
path.display(),
117+
e
118+
);
119+
}
113120
}
114121
}
115122
}

src/cortex-update/src/download.rs

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -63,16 +63,13 @@ impl Downloader {
6363
/// Uses a randomly-named subdirectory to prevent symlink attacks and
6464
/// predictable file name exploits.
6565
pub fn new(client: CortexSoftwareClient) -> UpdateResult<Self> {
66-
// Use a random suffix to prevent predictable temp directory names
67-
// This mitigates symlink attacks where an attacker pre-creates
68-
// files with expected names
69-
let random_suffix: u64 = std::time::SystemTime::now()
70-
.duration_since(std::time::UNIX_EPOCH)
71-
.map(|d| d.as_nanos() as u64)
72-
.unwrap_or(0)
73-
^ std::process::id() as u64;
74-
75-
let temp_dir = std::env::temp_dir().join(format!("cortex-update-{:x}", random_suffix));
66+
// Use cryptographically secure random bytes to prevent predictable temp directory names
67+
// This mitigates symlink attacks where an attacker pre-creates files with expected names
68+
let mut random_bytes = [0u8; 8];
69+
getrandom::getrandom(&mut random_bytes).map_err(|_| UpdateError::TempDirFailed)?;
70+
let random_suffix = u64::from_ne_bytes(random_bytes);
71+
72+
let temp_dir = std::env::temp_dir().join(format!("cortex-update-{:016x}", random_suffix));
7673

7774
// Create with restrictive permissions on Unix
7875
#[cfg(unix)]

src/cortex-update/src/install.rs

Lines changed: 63 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,12 +120,39 @@ impl Installer {
120120
}
121121

122122
/// Extract a tar.gz archive.
123-
async fn extract_tar_gz(&self, archive: &Path, dest: &Path) -> UpdateResult<PathBuf> {
123+
async fn extract_tar_gz(&self, archive_path: &Path, dest: &Path) -> UpdateResult<PathBuf> {
124124
use flate2::read::GzDecoder;
125125
use std::fs::File;
126126
use tar::Archive;
127127

128-
let file = File::open(archive)?;
128+
let file = File::open(archive_path)?;
129+
let gz = GzDecoder::new(file);
130+
let mut archive = Archive::new(gz);
131+
132+
// Validate paths before extraction to prevent path traversal attacks
133+
for entry in archive.entries().map_err(|e| UpdateError::ExtractionFailed {
134+
message: e.to_string(),
135+
})? {
136+
let entry = entry.map_err(|e| UpdateError::ExtractionFailed {
137+
message: e.to_string(),
138+
})?;
139+
let path = entry.path().map_err(|e| UpdateError::ExtractionFailed {
140+
message: e.to_string(),
141+
})?;
142+
143+
// Check for path traversal
144+
if path
145+
.components()
146+
.any(|c| matches!(c, std::path::Component::ParentDir))
147+
{
148+
return Err(UpdateError::ExtractionFailed {
149+
message: format!("Path traversal detected in archive: {}", path.display()),
150+
});
151+
}
152+
}
153+
154+
// Re-open and extract after validation
155+
let file = File::open(archive_path)?;
129156
let gz = GzDecoder::new(file);
130157
let mut archive = Archive::new(gz);
131158

@@ -139,8 +166,35 @@ impl Installer {
139166
}
140167

141168
/// Extract a zip archive.
142-
async fn extract_zip(&self, archive: &Path, dest: &Path) -> UpdateResult<PathBuf> {
143-
let file = std::fs::File::open(archive)?;
169+
async fn extract_zip(&self, archive_path: &Path, dest: &Path) -> UpdateResult<PathBuf> {
170+
let file = std::fs::File::open(archive_path)?;
171+
let mut archive =
172+
zip::ZipArchive::new(file).map_err(|e| UpdateError::ExtractionFailed {
173+
message: e.to_string(),
174+
})?;
175+
176+
// Validate all paths before extraction to prevent path traversal attacks
177+
for i in 0..archive.len() {
178+
let file = archive
179+
.by_index(i)
180+
.map_err(|e| UpdateError::ExtractionFailed {
181+
message: e.to_string(),
182+
})?;
183+
let path = std::path::Path::new(file.name());
184+
185+
// Check for path traversal
186+
if path
187+
.components()
188+
.any(|c| matches!(c, std::path::Component::ParentDir))
189+
{
190+
return Err(UpdateError::ExtractionFailed {
191+
message: format!("Path traversal detected in archive: {}", file.name()),
192+
});
193+
}
194+
}
195+
196+
// Re-open and extract after validation
197+
let file = std::fs::File::open(archive_path)?;
144198
let mut archive =
145199
zip::ZipArchive::new(file).map_err(|e| UpdateError::ExtractionFailed {
146200
message: e.to_string(),
@@ -244,6 +298,11 @@ impl Installer {
244298
const MOVEFILE_REPLACE_EXISTING: u32 = 0x1;
245299
const MOVEFILE_DELAY_UNTIL_REBOOT: u32 = 0x4;
246300

301+
// SAFETY: MoveFileExW is a Windows API function that schedules a file move/rename
302+
// operation to occur at the next system restart. We pass valid null-terminated
303+
// wide strings for source and destination paths. The MOVEFILE_DELAY_UNTIL_REBOOT
304+
// flag ensures this is a deferred operation. The return value of 0 indicates failure,
305+
// in which case we retrieve the error via last_os_error().
247306
let result = unsafe {
248307
windows_sys::Win32::Storage::FileSystem::MoveFileExW(
249308
old_path.as_ptr(),

src/cortex-update/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
#![allow(warnings, clippy::all)]
21
//! Cortex Update - Auto-update system for Cortex CLI
32
//!
43
//! Provides automatic update checking and installation via:
@@ -36,6 +35,7 @@ mod version;
3635

3736
pub use api::{CortexSoftwareClient, ReleaseAsset, ReleaseInfo};
3837
pub use config::{ReleaseChannel, UpdateConfig, UpdateMode};
38+
pub use download::DownloadProgress;
3939
pub use error::{UpdateError, UpdateResult};
4040
pub use install::DownloadedUpdate;
4141
pub use manager::{UpdateInfo, UpdateManager, UpdateOutcome};

0 commit comments

Comments
 (0)