🛡️ Sentinel: [HIGH] Enforce secure file and directory permissions

.jules/sentinel.md

@@ -0,0 +1,4 @@
+## 2024-03-24 - Insecure Default File Permissions
+**Vulnerability:** The CLI application creates sensitive configuration files and directories (like wallets and snapshot data) using standard `fs::create_dir_all` and `fs::write` in Rust. These standard functions create files/directories using the system's default umask, which typically allows other users on the same Unix-like system to read the sensitive files.
+**Learning:** This could lead to a local privilege escalation or exposure of sensitive user data if the user runs the CLI on a shared machine. Relying on default system configurations for sensitive files is unsafe.
+**Prevention:** Always use `std::os::unix::fs::DirBuilderExt` and `std::os::unix::fs::OpenOptionsExt` to explicitly set file permissions (e.g., `0o700` for directories and `0o600` for files) when creating sensitive data on disk.

src/paths.rs

@@ -5,6 +5,40 @@ use std::path::{Path, PathBuf};
 use crate::lock::now_unix;
 use std::process;
 
+pub fn create_secure_dir_all<P: AsRef<Path>>(path: P) -> std::io::Result<()> {
+    let path = path.as_ref();
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::DirBuilderExt;
+        let mut builder = fs::DirBuilder::new();
+        builder.recursive(true);
+        builder.mode(0o700);
+        builder.create(path)
+    }
+    #[cfg(not(unix))]
+    {
+        fs::create_dir_all(path)
+    }
+}
+
+pub fn write_secure_file<P: AsRef<Path>>(path: P, contents: &[u8]) -> std::io::Result<()> {
+    let path = path.as_ref();
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::OpenOptionsExt;
+        use std::io::Write;
+        let mut options = fs::OpenOptions::new();
+        options.write(true).create(true).truncate(true).mode(0o600);
+        let mut file = options.open(path)?;
+        file.write_all(contents)?;
+        file.sync_all()
+    }
+    #[cfg(not(unix))]
+    {
+        fs::write(path, contents)
+    }
+}
+
 pub fn data_dir(config: &crate::config::ServiceConfig<'_>) -> std::path::PathBuf {
     if let Some(path) = config.data_dir {
         path.to_path_buf()
@@ -25,7 +59,7 @@ pub fn profile_path(config: &crate::config::ServiceConfig<'_>) -> Result<PathBuf
     let root = data_dir(config);
     let profiles = root.join("profiles");
     if !profiles.exists() {
-        fs::create_dir_all(&profiles)
+        create_secure_dir_all(&profiles)
             .map_err(|e| AppError::Config(format!("failed to create profiles dir: {e}")))?;
     }
     Ok(profiles.join(format!("{}.json", config.profile)))
@@ -38,14 +72,14 @@ pub fn profile_lock_path(config: &crate::config::ServiceConfig<'_>) -> Result<Pa
 pub fn snapshot_dir(config: &crate::config::ServiceConfig<'_>) -> Result<PathBuf, AppError> {
     let root = data_dir(config);
     let directory = root.join("snapshots").join(config.profile);
-    fs::create_dir_all(&directory)
+    create_secure_dir_all(&directory)
         .map_err(|e| AppError::Config(format!("failed to create snapshot dir: {e}")))?;
     Ok(directory)
 }
 
 pub fn write_bytes_atomic(path: &Path, bytes: &[u8], label: &str) -> Result<(), AppError> {
     if let Some(parent) = path.parent() {
-        fs::create_dir_all(parent)
+        create_secure_dir_all(parent)
             .map_err(|e| AppError::Config(format!("failed to create dir for {label}: {e}")))?;
     }
     let tmp_name = format!(
@@ -56,7 +90,7 @@ pub fn write_bytes_atomic(path: &Path, bytes: &[u8], label: &str) -> Result<(),
     );
     let tmp_path = path.with_file_name(tmp_name);
 
-    fs::write(&tmp_path, bytes)
+    write_secure_file(&tmp_path, bytes)
         .map_err(|e| AppError::Config(format!("failed to write temp {label}: {e}")))?;
     if let Err(e) = fs::rename(&tmp_path, path) {
         let _ = fs::remove_file(&tmp_path);