From 9668fe88257ee45785cc99debe00b3161ee8d5bb Mon Sep 17 00:00:00 2001 From: Sasha Mitchell Date: Wed, 19 Aug 2026 00:22:26 +0700 Subject: [PATCH] fix(wallet): reject keystore names that escape the keystore directory cast wallet import/new/remove/decrypt/change-password joined ACCOUNT_NAME onto the keystore directory, so ../name wrote or touched files outside it. The directory is already a separate flag. Reject names that are not a single path segment. --- .../reject-path-keystore-account-names.md | 5 ++++ crates/cast/src/cmd/wallet/mod.rs | 20 +++++++++++++ crates/cast/tests/cli/main.rs | 30 +++++++++++++++++++ 3 files changed, 55 insertions(+) create mode 100644 .changelog/reject-path-keystore-account-names.md diff --git a/.changelog/reject-path-keystore-account-names.md b/.changelog/reject-path-keystore-account-names.md new file mode 100644 index 0000000000000..773fcbf9aa798 --- /dev/null +++ b/.changelog/reject-path-keystore-account-names.md @@ -0,0 +1,5 @@ +--- +cast: patch +--- + +Rejected keystore account names that escape the keystore directory. diff --git a/crates/cast/src/cmd/wallet/mod.rs b/crates/cast/src/cmd/wallet/mod.rs index c8b367582064f..937886726402f 100644 --- a/crates/cast/src/cmd/wallet/mod.rs +++ b/crates/cast/src/cmd/wallet/mod.rs @@ -1003,6 +1003,7 @@ flag to set your key via: args.run()?; } Self::Remove { name, dir, unsafe_password } => { + ensure_account_name_available(&name)?; let dir = if let Some(path) = dir { Path::new(&path).to_path_buf() } else { @@ -1089,6 +1090,7 @@ flag to set your key via: } } Self::DecryptKeystore { account_name, keystore_dir, unsafe_password } => { + ensure_account_name_available(&account_name)?; // Set up keystore directory let dir = if let Some(path) = keystore_dir { Path::new(&path).to_path_buf() @@ -1131,6 +1133,7 @@ flag to set your key via: unsafe_password, unsafe_new_password, } => { + ensure_account_name_available(&account_name)?; // Set up keystore directory let dir = if let Some(path) = keystore_dir { Path::new(&path).to_path_buf() @@ -1321,6 +1324,10 @@ fn ensure_touch_id_available(touch_id: bool) -> Result<()> { const TOUCH_ID_SIDECAR_SUFFIX: &str = ".touchid"; fn ensure_account_name_available(name: &str) -> Result<()> { + let file_name = Path::new(name).file_name().and_then(|s| s.to_str()); + if name.is_empty() || name.contains('\\') || file_name != Some(name) { + eyre::bail!("account name must be a single path segment"); + } if name.ends_with(TOUCH_ID_SIDECAR_SUFFIX) { eyre::bail!("account names ending in `{TOUCH_ID_SIDECAR_SUFFIX}` are reserved"); } @@ -2325,4 +2332,17 @@ mod tests { "expected error when both --nonce and --self-broadcast are provided" ); } + + #[test] + fn rejects_path_keystore_account_name() { + assert!(ensure_account_name_available("dev").is_ok()); + assert!(ensure_account_name_available("testAccount").is_ok()); + assert!(ensure_account_name_available("../pwned").is_err()); + assert!(ensure_account_name_available("nested/alias").is_err()); + assert!(ensure_account_name_available("foo/../bar").is_err()); + assert!(ensure_account_name_available("..").is_err()); + assert!(ensure_account_name_available(".").is_err()); + assert!(ensure_account_name_available("").is_err()); + assert!(ensure_account_name_available("foo\\bar").is_err()); + } } diff --git a/crates/cast/tests/cli/main.rs b/crates/cast/tests/cli/main.rs index 9095f0dfbca33..aa7f7cddaa110 100644 --- a/crates/cast/tests/cli/main.rs +++ b/crates/cast/tests/cli/main.rs @@ -1254,6 +1254,36 @@ Error: account names ending in `.touchid` are reserved "#]]); }); +// `cast wallet import` treats ACCOUNT_NAME as a file name under the keystore dir. +// A path segment would write the encrypted keystore outside that directory. +casttest!(wallet_import_rejects_path_account_name, |prj, cmd| { + let keystore_dir = prj.root().join("keystore"); + fs::create_dir_all(&keystore_dir).unwrap(); + let escaped = prj.root().join("pwned_foundry_alias"); + + cmd.set_current_dir(prj.root()); + cmd.args([ + "wallet", + "import", + "../pwned_foundry_alias", + "--private-key", + "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80", + "--keystore-dir", + keystore_dir.to_str().unwrap(), + "--unsafe-password", + "test", + ]) + .assert_failure() + .stdout_eq(str![""]) + .stderr_eq(str![[r#" +Error: account name must be a single path segment + +"#]]); + + assert!(!escaped.exists()); + assert!(!keystore_dir.join("../pwned_foundry_alias").exists()); +}); + // tests that `cast wallet sign message` outputs the expected signature casttest!(wallet_sign_message_utf8_data, |_prj, cmd| { let pk = "0x0000000000000000000000000000000000000000000000000000000000000001";