Skip to content

Commit

Permalink
feat: Only use -save-to-file to protected folder
Browse files Browse the repository at this point in the history
  • Loading branch information
Raphexion committed Nov 6, 2024
1 parent d9fb995 commit ef05e86
Showing 1 changed file with 65 additions and 8 deletions.
73 changes: 65 additions & 8 deletions cli/src/commands/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ use rand::SeedableRng;
use rand_chacha::ChaChaRng;
use rayon::prelude::*;
use std::{
io::{Read, Write},
fs::File,
io::{Read, Write},
os::unix::fs::PermissionsExt,
path::PathBuf,
};
Expand Down Expand Up @@ -256,8 +256,9 @@ impl Account {
PrivateKey::try_from(seed).map_err(|_| anyhow!("Failed to convert the seed into a valid private key"))?;
// Construct the account.
let account = snarkos_account::Account::<N>::try_from(private_key)?;
// Save to file instead of printing it back to the user
// Save to file in addition to printing it back to the user
if let Some(path) = save_to_file {
check_parent_permissions(&path)?;
let mut file = File::create_new(path)?;
file.write_all(account.private_key().to_string().as_bytes())?;
file.set_permissions(PermissionsExt::from_mode(0o400))?;
Expand Down Expand Up @@ -348,14 +349,33 @@ fn wait_for_keypress() {
std::io::stdin().read_exact(&mut single_key).unwrap();
}

fn check_parent_permissions(path_str: &String) -> Result<()> {
#[cfg(target_family = "unix")]
{
use anyhow::ensure;
use std::os::unix::fs::PermissionsExt;
use std::path::Path;

let path = Path::new(path_str);
if let Some(parent) = path.parent() {
let permissions = parent.metadata()?.permissions().mode();
ensure!(permissions & 0o777 == 0o700, "The folder {:?} must be readable only by the owner (0700)", parent);
} else {
bail!("Parent does not exist for path={}", path_str);
}
}
Ok(())
}

#[cfg(test)]
mod tests {
use crate::commands::Account;
use tempfile::NamedTempFile;
use tempfile::TempDir;
use std::fs;
use std::fs::Permissions;
use std::io::Write;
use std::os::unix::fs::PermissionsExt;
use tempfile::NamedTempFile;
use tempfile::TempDir;

use colored::Colorize;

Expand Down Expand Up @@ -422,6 +442,9 @@ mod tests {
#[test]
fn test_new_save_to_file() {
let dir = TempDir::new().expect("Failed to create temp folder");
let dir_path = dir.path();
fs::set_permissions(dir_path, Permissions::from_mode(0o700)).expect("Failed to set permissions");

let file = dir.path().join("my-private-key-file").to_string_lossy().to_string();

let seed = Some(1231275789u64.to_string());
Expand All @@ -434,7 +457,7 @@ mod tests {
let expected = "APrivateKey1zkp2n22c19hNdGF8wuEoQcuiyuWbquY6up4CtG5DYKqPX2X";
assert!(actual.contains(expected));

let content = fs::read_to_string(file.clone()).expect("Failed to read private-key-file");
let content = fs::read_to_string(&file).expect("Failed to read private-key-file");
assert_eq!(expected, content);

// check the permissions - to read-only for the owner
Expand All @@ -443,6 +466,37 @@ mod tests {
assert_eq!(permissions.mode() & 0o777, 0o400, "File permissions are not 0o400");
}

#[test]
fn test_new_prevent_save_to_file_in_non_protected_folder() {
let dir = TempDir::new().expect("Failed to create temp folder");
let dir_path = dir.path();
fs::set_permissions(dir_path, Permissions::from_mode(0o444)).expect("Failed to set permissions");

let file = dir.path().join("my-private-key-file").to_string_lossy().to_string();

let seed = None;
let vanity = None;
let discreet = false;
let save_to_file = Some(file);
let account = Account::New { network: 0, seed, vanity, discreet, save_to_file };
let res = account.parse();
assert!(res.is_err());
}

#[test]
fn test_new_prevent_save_to_file_in_non_existing_folder() {
let dir = TempDir::new().expect("Failed to create temp folder");
let file = dir.path().join("missing").join("my-private-key-file").to_string_lossy().to_string();

let seed = None;
let vanity = None;
let discreet = false;
let save_to_file = Some(file);
let account = Account::New { network: 0, seed, vanity, discreet, save_to_file };
let res = account.parse();
assert!(res.is_err());
}

#[test]
fn test_new_prevent_overwrite_existing_file() {
let mut file = NamedTempFile::new().expect("Failed to create temp file");
Expand Down Expand Up @@ -509,15 +563,18 @@ mod tests {
let message = "Hello, world!".to_string();

let dir = TempDir::new().expect("Failed to create temp folder");
let path = dir.path().join("my-private-key-file").to_string_lossy().to_string();
let dir_path = dir.path();
fs::set_permissions(dir_path, Permissions::from_mode(0o700)).expect("Failed to set permissions");

let file = dir.path().join("my-private-key-file").to_string_lossy().to_string();

let seed = None;
let vanity = None;
let discreet = false;
let account = Account::New { network: 0, seed, vanity, discreet, save_to_file: Some(path.clone()) };
let account = Account::New { network: 0, seed, vanity, discreet, save_to_file: Some(file.clone()) };
assert!(account.parse().is_ok());

let account = Account::Sign { network: 0, private_key: None, private_key_file: Some(path), message, raw: true };
let account = Account::Sign { network: 0, private_key: None, private_key_file: Some(file), message, raw: true };
assert!(account.parse().is_ok());
}

Expand Down

0 comments on commit ef05e86

Please sign in to comment.