Skip to content

Commit 0544bf1

Browse files
committed
fix: Add input validation and improve error handling
- disk.rs: Replace unwrap() calls with proper error handling in diskutil parallel scanning, limit concurrent threads to 8 to prevent resource exhaustion on systems with many partitions - shell.rs: Add image name validation against whitelist (alpine, freebsd-15.0) - apk.rs: Add package name validation to prevent command injection (alphanumeric, dots, underscores, hyphens, plus signs, @ for repos)
1 parent ae6bb42 commit 0544bf1

3 files changed

Lines changed: 82 additions & 17 deletions

File tree

src-tauri/src/commands/apk.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,30 @@
11
use crate::cli::execute_command;
22

3+
/// Validate package name to prevent command injection
4+
/// Package names must contain only alphanumeric characters, dots, underscores, hyphens,
5+
/// and optionally a version specifier like @edge
6+
fn validate_package_name(name: &str) -> Result<(), String> {
7+
if name.is_empty() {
8+
return Err("Package name cannot be empty".to_string());
9+
}
10+
if name.len() > 128 {
11+
return Err("Package name too long".to_string());
12+
}
13+
// Allow: alphanumeric, dots, underscores, hyphens, plus signs (for g++ etc)
14+
// Also allow @ for repository tags like package@edge
15+
let valid = name.chars().all(|c| {
16+
c.is_ascii_alphanumeric() || c == '.' || c == '_' || c == '-' || c == '+' || c == '@'
17+
});
18+
if !valid {
19+
return Err(format!("Package name '{}' contains invalid characters", name));
20+
}
21+
// Must not start with a dash (could be interpreted as an option)
22+
if name.starts_with('-') {
23+
return Err("Package name cannot start with '-'".to_string());
24+
}
25+
Ok(())
26+
}
27+
328
#[tauri::command]
429
pub fn list_packages() -> Result<Vec<String>, String> {
530
let output = execute_command(&["apk", "info"], false, None)?;
@@ -19,6 +44,11 @@ pub async fn add_packages(packages: Vec<String>) -> Result<(), String> {
1944
return Err("No packages specified".to_string());
2045
}
2146

47+
// Validate all package names before executing
48+
for pkg in &packages {
49+
validate_package_name(pkg)?;
50+
}
51+
2252
tokio::task::spawn_blocking(move || {
2353
let mut args = vec!["apk", "add"];
2454
let pkg_refs: Vec<&str> = packages.iter().map(|s| s.as_str()).collect();
@@ -36,6 +66,11 @@ pub async fn remove_packages(packages: Vec<String>) -> Result<(), String> {
3666
return Err("No packages specified".to_string());
3767
}
3868

69+
// Validate all package names before executing
70+
for pkg in &packages {
71+
validate_package_name(pkg)?;
72+
}
73+
3974
tokio::task::spawn_blocking(move || {
4075
let mut args = vec!["apk", "del"];
4176
let pkg_refs: Vec<&str> = packages.iter().map(|s| s.as_str()).collect();

src-tauri/src/commands/disk.rs

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -239,26 +239,38 @@ fn update_filesystem_support(result: &mut DiskListResult) {
239239
}
240240
}
241241

242-
// Run diskutil info calls in parallel
242+
// Run diskutil info calls in parallel with limited concurrency
243+
// Limit to 8 concurrent threads to avoid resource exhaustion on systems with many partitions
244+
const MAX_DISKUTIL_THREADS: usize = 8;
243245
let diskutil_results: Arc<Mutex<HashMap<String, (String, bool, Option<String>)>>> =
244246
Arc::new(Mutex::new(HashMap::new()));
245247

246-
std::thread::scope(|s| {
247-
for device_id in &needs_diskutil {
248-
let results = Arc::clone(&diskutil_results);
249-
let device = device_id.clone();
250-
s.spawn(move || {
251-
if let Some(info) = get_diskutil_fs_info(&device) {
252-
results.lock().unwrap().insert(device, info);
253-
}
254-
});
255-
}
256-
});
248+
// Process in batches to limit concurrency
249+
for chunk in needs_diskutil.chunks(MAX_DISKUTIL_THREADS) {
250+
std::thread::scope(|s| {
251+
for device_id in chunk {
252+
let results = Arc::clone(&diskutil_results);
253+
let device = device_id.clone();
254+
s.spawn(move || {
255+
if let Some(info) = get_diskutil_fs_info(&device) {
256+
// Handle mutex lock failure gracefully
257+
if let Ok(mut guard) = results.lock() {
258+
guard.insert(device, info);
259+
}
260+
}
261+
});
262+
}
263+
});
264+
}
257265

258-
let diskutil_map = Arc::try_unwrap(diskutil_results)
259-
.unwrap()
260-
.into_inner()
261-
.unwrap();
266+
// Extract results from Arc<Mutex<...>> safely
267+
let diskutil_map = match Arc::try_unwrap(diskutil_results) {
268+
Ok(mutex) => mutex.into_inner().unwrap_or_default(),
269+
Err(arc) => {
270+
// If other references exist (shouldn't happen), clone the inner data
271+
arc.lock().map(|g| g.clone()).unwrap_or_default()
272+
}
273+
};
262274

263275
// Apply results to partitions
264276
for disk in &mut result.disks {

src-tauri/src/commands/shell.rs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,23 @@ use std::sync::{Arc, Mutex};
44
use tauri::{AppHandle, Emitter};
55
use crate::cli::get_path;
66

7+
/// Valid image names for shell command
8+
/// This whitelist prevents potential command injection via malformed image names
9+
const VALID_IMAGES: &[&str] = &["alpine", "freebsd-15.0"];
10+
11+
/// Validate image name against whitelist
12+
fn validate_image_name(image: &str) -> Result<(), String> {
13+
if VALID_IMAGES.contains(&image) {
14+
Ok(())
15+
} else {
16+
Err(format!(
17+
"Invalid image '{}'. Valid images: {}",
18+
image,
19+
VALID_IMAGES.join(", ")
20+
))
21+
}
22+
}
23+
724
pub struct PtyState {
825
writer: Option<Box<dyn Write + Send>>,
926
master: Option<Box<dyn portable_pty::MasterPty + Send>>,
@@ -41,8 +58,9 @@ pub async fn start_shell(
4158
let mut cmd = CommandBuilder::new(cli_path);
4259
cmd.arg("shell");
4360

44-
// Add image option if specified
61+
// Add image option if specified (validated against whitelist)
4562
if let Some(ref img) = image {
63+
validate_image_name(img)?;
4664
cmd.arg("-i");
4765
cmd.arg(img);
4866
}

0 commit comments

Comments
 (0)