diff --git a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightclient.rs b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightclient.rs index 2171807..defad0f 100644 --- a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightclient.rs +++ b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightclient.rs @@ -1120,6 +1120,13 @@ impl LightClient { /// Import a new private key pub fn do_import_tk(&self, sk: String, birthday: u64) -> Result { + // Checked before "locked": unlocking would not help (s1-D1). No "Error: " prefix here; the console + // import/timport commands add it. + if self.wallet.read().unwrap().is_encrypted() { + let e = "Can't import a private key while the wallet is encrypted - remove encryption, import, then encrypt again".to_string(); + error!("{}", e); + return Err(e); + } if !self.wallet.read().unwrap().is_unlocked_for_spending() { error!("Wallet is locked"); return Err("Wallet is locked".to_string()); @@ -1232,6 +1239,13 @@ pub fn start_mempool_monitor(lc: Arc) -> Result<(), String> { /// Import a new private key pub fn do_import_sk(&self, sk: String, birthday: u64) -> Result { + // Checked before "locked": unlocking would not help (s1-D1). No "Error: " prefix here; the console + // import/timport commands add it. + if self.wallet.read().unwrap().is_encrypted() { + let e = "Can't import a private key while the wallet is encrypted - remove encryption, import, then encrypt again".to_string(); + error!("{}", e); + return Err(e); + } if !self.wallet.read().unwrap().is_unlocked_for_spending() { error!("Wallet is locked"); return Err("Wallet is locked".to_string()); @@ -2010,4 +2024,68 @@ pub fn test_bad_import() { w.join().unwrap(); } + // C251 (s2): a WIF that cannot be decoded returns Err and leaves the wallet lock usable. Before, import_taddr + // panicked under the wallet write guard (bs58 unwrap / hex slice), which poisoned the wallet until restart. + #[test] + pub fn test_bad_import_does_not_poison() { + use std::panic::{catch_unwind, AssertUnwindSafe}; + let tmp = TempDir::new("lctest").unwrap(); + let lc = LightClient::new_from_phrase(TEST_SEED.to_string(), &main_config(&tmp), 0, 0, false).unwrap(); + let before = lc.do_address(); + + for bad in &["zs1notawif", "secret-extended-key-main1qqqqqqqqqq", "", "1"] { + let r = catch_unwind(AssertUnwindSafe(|| lc.do_import_tk(bad.to_string(), 0))); + let e = match r { + Ok(Err(e)) => e, + Ok(Ok(a)) => panic!("{:?} imported as {}", bad, a), + Err(_) => panic!("do_import_tk({:?}) panicked", bad), + }; + assert!(e.contains("Couldn't decode the transparent private key"), "{:?}: {}", bad, e); + assert!(bad.len() < 2 || !e.contains(bad), "the error echoes the key: {}", e); + // separate statements: a read guard alive in the same expression would deadlock the write() + assert!(lc.wallet.read().is_ok(), "the wallet lock is poisoned"); + assert!(lc.wallet.write().is_ok(), "the wallet lock is poisoned"); + } + assert_eq!(lc.do_address(), before); + } + + // C252 (s1 test 8): a private-key import into an encrypted wallet is refused with the "encrypted" reason + // whether the wallet is locked or unlocked (never "Wallet is locked", which would send the user to unlock and + // retry), and it leaves the wallet able to lock, save and show its real seed. Viewing keys still import. + #[test] + pub fn test_encrypted_import_is_clean() { + let tmp = TempDir::new("lctest").unwrap(); + let lc = LightClient::new_from_phrase(TEST_SEED.to_string(), &main_config(&tmp), 0, 0, false).unwrap(); + let tmp2 = TempDir::new("lctest").unwrap(); + let other = LightClient::new(&main_config(&tmp2), 0).unwrap(); + let (wif, sk, vk) = { + let w = other.wallet.read().unwrap(); + let z = w.get_z_private_keys()[0].clone(); + (w.get_t_secret_keys()[0].1.clone(), z.1, z.2) + }; + + // encrypt() also locks the wallet + lc.wallet.write().unwrap().encrypt("password".to_string()).unwrap(); + for &unlocked in &[false, true] { + if unlocked { + lc.wallet.write().unwrap().unlock("password".to_string()).unwrap(); + } + for r in vec![lc.do_import_sk(sk.clone(), 0), lc.do_import_tk(wif.clone(), 0), lc.do_import_key(sk.clone(), 0)] { + let e = r.err().expect("a private-key import must fail while the wallet is encrypted"); + assert!(e.starts_with("Can't import a private key while the wallet is encrypted"), "{}", e); + assert!(!e.contains("locked") && !e.contains("decode"), "{}", e); + assert!(!e.contains(&wif) && !e.contains(&sk), "the error echoes the key"); + } + assert_eq!(lc.wallet.read().unwrap().is_unlocked_for_spending(), unlocked); + } + + // A viewing key still imports while encrypted (its save locks the wallet again). + assert!(lc.do_import_vk(vk, 0).is_ok()); + assert!(!lc.wallet.read().unwrap().is_unlocked_for_spending()); + + assert!(lc.do_save().is_ok()); + lc.wallet.write().unwrap().unlock("password".to_string()).unwrap(); + assert_eq!(lc.do_seed_phrase().unwrap()["seed"], TEST_SEED.to_string()); + } + } diff --git a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs index 62388f0..276e9fd 100644 --- a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs +++ b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs @@ -675,19 +675,17 @@ impl LightWallet { return "Error: Can't add key while wallet is locked".to_string(); } - //// Decode Wif to base58 to hex - let sk_to_bs58 = bs58::decode(sk).into_vec().unwrap(); + // An imported key would be stored with no enc_key; lock() could then never succeed (see lock()). + if self.encrypted { + return "Error: Can't import a private key while the wallet is encrypted - remove encryption, import, then encrypt again".to_string(); + } - let bs58_to_hex = hex::encode(sk_to_bs58); - - //// Manipulate string, to exclude last 4 bytes (checksum bytes), first 2 bytes (secretkey prefix) and the compressed flag (works only for compressed Wifs!) - - let slice_sk = &bs58_to_hex[2..66]; - - //// Get the SecretKey from slice - let secret_key = SecretKey::from_slice(&hex::decode(slice_sk).unwrap()); - - let sk_raw = secret_key.unwrap(); + //// Decode the WIF: Base58Check (checksum verified), version byte, 32-byte key, 0x01 compressed flag. + //// Never panics, and never echoes the key in an error. + let sk_raw = match LightWallet::decode_taddr_wif(&sk, self.config.base58_secretkey_prefix()[0]) { + Ok(k) => k, + Err(e) => return format!("Error: {}", e), + }; //// Make sure the key doesn't already exist if self.tkeys.read().unwrap().iter().find(|&wk| wk.tkey.is_some() && wk.tkey.as_ref().unwrap() == &sk_raw.clone()).is_some() { @@ -714,6 +712,11 @@ impl LightWallet { return "Error: Can't add key while wallet is locked".to_string(); } + // An imported key would be stored with no enc_key; lock() could then never succeed (see lock()). + if self.encrypted { + return "Error: Can't import a private key while the wallet is encrypted - remove encryption, import, then encrypt again".to_string(); + } + // First, try to interpret the key let extsk = match decode_extended_spending_key(self.config.hrp_sapling_private_key(), &sk) { Ok(Some(k)) => k, @@ -934,6 +937,27 @@ impl LightWallet { hash160.result().to_base58check(prefix, &[]) } + /// WIF = version(1) || key(32) || 0x01 (compressed). `bs58 ... with_check(None)` verifies the double-SHA256 + /// checksum and strips it (bs58 0.2.5 decode.rs:284-310), keeping the version byte. Do NOT pass + /// `with_check(Some(v))`: it would pin one version and reject the 0x80 (K/L) WIFs the help text advertises. + /// Accepts the config prefix (0xBC on main) and 0x80. The decoded bytes are wiped on every path (s2-D5), + /// and no error contains the key. + pub(crate) fn decode_taddr_wif(wif: &str, network_prefix: u8) -> Result { + let wif = wif.trim(); + // Decode into a buffer we own (bs58's into_vec allocates the same size), so it is wiped even when the + // checksum fails; into_vec would drop its buffer, which then holds the decoded bytes, unwiped. + let mut buf = vec![0u8; wif.len()]; + let r = match bs58::decode(wif).with_check(None).into(&mut buf[..]) { + Err(_) => Err("Couldn't decode the transparent private key (not valid Base58Check)".to_string()), + Ok(33) => Err("Uncompressed transparent private keys are not supported; import the compressed form".to_string()), + Ok(n) if n != 34 || buf[33] != 0x01 => Err("Couldn't decode the transparent private key (unrecognized format)".to_string()), + Ok(_) if buf[0] != network_prefix && buf[0] != 0x80 => Err("The transparent private key is for a different network".to_string()), + Ok(_) => SecretKey::from_slice(&buf[1..33]).map_err(|_| "Invalid transparent private key".to_string()), + }; + sodiumoxide::utils::memzero(&mut buf[..]); // already a dependency; no new crate + r + } + pub fn address_from_sk(&self, sk: &secp256k1::SecretKey) -> String { LightWallet::address_from_prefix_sk(&self.config.base58_pubkey_address(), sk) } diff --git a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/tests.rs b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/tests.rs index 7c71622..fedabdb 100644 --- a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/tests.rs +++ b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/tests.rs @@ -3292,3 +3292,213 @@ fn test_g4_transaction_too_large_refused() { let txs = wallet.txs.read().unwrap(); assert!(txs.values().flat_map(|t| t.notes.iter()).all(|nd| nd.unconfirmed_spent.is_none())); } + +// ---- R3: transparent key import (C251) and private-key import while encrypted (C252) ---- +// A WIF is Base58Check(version || key(32) || 0x01); the version is the config prefix (0xBC on main) or 0x80. +// Bad input returns "Error: ..." (never a panic under the wallet lock), and no error contains the key. + +fn r3_wif(version: u8, payload: &[u8], suffix: &[u8]) -> String { + use super::ToBase58Check; + payload.to_base58check(&[version], suffix) +} + +fn r3_key_one() -> SecretKey { + let mut k = [0u8; 32]; + k[31] = 1; + SecretKey::from_slice(&k).unwrap() +} + +// Swap the first pair of adjacent, different characters at or after `from` (a one-character typo class). +fn r3_transpose(s: &str, from: usize) -> String { + let mut c: Vec = s.chars().collect(); + let i = (from..c.len() - 1).find(|&i| c[i] != c[i + 1]).unwrap(); + c.swap(i, i + 1); + c.into_iter().collect() +} + +// Replace the character at `at` with a different base58 character. +fn r3_flip(s: &str, at: usize) -> String { + let mut c: Vec = s.chars().collect(); + c[at] = if c[at] == 'z' { 'y' } else { 'z' }; + c.into_iter().collect() +} + +fn r3_import_rejected(wallet: &mut LightWallet, input: &str) -> String { + let n = wallet.tkeys.read().unwrap().len(); + let r = wallet.import_taddr(input.to_string(), 0); + assert!(r.starts_with("Error"), "{:?} was imported as {}", input, r); + assert!(input.trim().len() < 8 || !r.contains(input.trim()), "the error echoes the key: {}", r); + assert!(wallet.tkeys.read().is_ok(), "tkeys lock poisoned"); + assert_eq!(wallet.tkeys.read().unwrap().len(), n); + r +} + +#[test] +fn test_import_taddr_bad_input_never_panics() { + // s2 C251 test 1 (+ the 0xEF case). Before: panics at the bs58 unwrap, the [2..66] slice or the key unwrap. + let config = get_test_config(); + let mut wallet = LightWallet::new(None, &config, 0, 0).unwrap(); + let other = LightWallet::new(None, &config, 0, 0).unwrap(); + let good = other.get_t_secret_keys()[0].1.clone(); + let (zaddr, zsk, zvk) = other.get_z_private_keys()[0].clone(); + let key = [7u8; 32]; + + let mut bad_checksum = good.clone(); + let last = bad_checksum.pop().unwrap(); + bad_checksum.push(if last == '2' { '3' } else { '2' }); + + let inputs = vec![ + "".to_string(), "x".to_string(), "0OIl".to_string(), " ".to_string(), + zaddr, zsk, zvk, + "zivks1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq".to_string(), + bs58::encode(&[1u8, 2, 3, 4, 5]).into_string(), // plain base58, 5 bytes + bad_checksum.clone(), + r3_wif(0xBC, &[9u8; 9], &[]), // checksum-valid, 10-byte payload + r3_wif(0xBC, &[0u8; 32], &[0x01]), // zero scalar + r3_wif(0xBC, &key, &[0x02]), // 34 bytes, wrong compressed flag + r3_wif(0x80, &key, &[]), // uncompressed + r3_wif(0x05, &key, &[0x01]), // foreign version byte + r3_wif(0xEF, &key, &[0x01]), // testnet WIF on main + "5HpHagT65TZzG1PH3CSu63k8DbpvD8s5ip4nEB3kEsreAnchuDf".to_string(), // key 1, uncompressed (0x80) + ]; + for input in &inputs { + r3_import_rejected(&mut wallet, input); + } + + assert_eq!(r3_import_rejected(&mut wallet, &bad_checksum), "Error: Couldn't decode the transparent private key (not valid Base58Check)"); + assert_eq!(r3_import_rejected(&mut wallet, "0OIl"), "Error: Couldn't decode the transparent private key (not valid Base58Check)"); + assert_eq!(r3_import_rejected(&mut wallet, &r3_wif(0xBC, &[9u8; 9], &[])), + "Error: Couldn't decode the transparent private key (unrecognized format)"); + assert_eq!(r3_import_rejected(&mut wallet, &r3_wif(0xBC, &key, &[0x02])), + "Error: Couldn't decode the transparent private key (unrecognized format)"); + assert_eq!(r3_import_rejected(&mut wallet, &r3_wif(0xBC, &[0u8; 32], &[0x01])), "Error: Invalid transparent private key"); + for unc in &[r3_wif(0x80, &key, &[]), "5HpHagT65TZzG1PH3CSu63k8DbpvD8s5ip4nEB3kEsreAnchuDf".to_string()] { + assert_eq!(r3_import_rejected(&mut wallet, unc), + "Error: Uncompressed transparent private keys are not supported; import the compressed form"); + } + for foreign in &[r3_wif(0xEF, &key, &[0x01]), r3_wif(0x05, &key, &[0x01])] { + assert_eq!(r3_import_rejected(&mut wallet, foreign), "Error: The transparent private key is for a different network"); + } + + // The wallet still works: the good key imports afterwards. + assert_eq!(wallet.import_taddr(good, 0), other.get_t_secret_keys()[0].0); +} + +#[test] +fn test_import_taddr_accepts_own_export_and_btc_wif() { + // s2 C251 test 2 (passes before and after; guards compatibility): the wallet's own export (0xBC) and a + // standard compressed 0x80 WIF both import, under the compressed-key address. + let config = get_test_config(); + let other = LightWallet::new(None, &config, 0, 0).unwrap(); + let (taddr, wif) = other.get_t_secret_keys()[0].clone(); + assert_eq!(bs58::decode(&wif).with_check(None).into_vec().unwrap()[0], 0xBC); + + let mut wallet = LightWallet::new(None, &config, 0, 0).unwrap(); + let n = wallet.tkeys.read().unwrap().len(); + assert_eq!(wallet.import_taddr(wif.clone(), 0), taddr); + assert!(wallet.get_t_secret_keys().iter().any(|(a, k)| a == &taddr && k == &wif), "export round trip"); + + // 0x80 WIF of another key, built here, and the published key-1 vector (Bitcoin 1BgGZ9tc...; on DragonX + // the same hash160 with prefix 0x3C is RKxTdfmt...). + let k42 = SecretKey::from_slice(&[0x42u8; 32]).unwrap(); + let wif80 = r3_wif(0x80, &k42[..], &[0x01]); + assert!(wif80.starts_with('K') || wif80.starts_with('L'), "{}", wif80); + assert_eq!(wallet.import_taddr(wif80, 0), wallet.address_from_sk(&k42)); + assert_eq!(wallet.import_taddr("KwDiBf89QgGbjEhKnhXJuH7LrciVrZi3qYjgd9M7rFU73sVHnoWn".to_string(), 0), + "RKxTdfmtxtfLDKZBgx6SvNkBtNu9jRYnLh"); + assert_eq!(wallet.address_from_sk(&r3_key_one()), "RKxTdfmtxtfLDKZBgx6SvNkBtNu9jRYnLh"); + assert_eq!(wallet.tkeys.read().unwrap().len(), n + 3); + + // A key that is already there is refused. + assert_eq!(wallet.import_taddr(wif, 0), "Error: Key already exists"); + assert_eq!(wallet.tkeys.read().unwrap().len(), n + 3); +} + +#[test] +fn test_decode_taddr_wif_checksum_typo_is_rejected() { + // s2 C251 test 3. Before: no checksum check, so a transposed WIF silently imported a different key. + let config = get_test_config(); + let other = LightWallet::new(None, &config, 0, 0).unwrap(); + let (taddr, wif) = other.get_t_secret_keys()[0].clone(); + let mut wallet = LightWallet::new(None, &config, 0, 0).unwrap(); + for from in &[1usize, 20, 40] { + let typo = r3_transpose(&wif, *from); + assert_ne!(typo, wif); + assert_eq!(r3_import_rejected(&mut wallet, &typo), + "Error: Couldn't decode the transparent private key (not valid Base58Check)"); + assert!(LightWallet::decode_taddr_wif(&typo, 0xBC).is_err()); + } + assert!(LightWallet::decode_taddr_wif(&wif, 0xBC).is_ok()); + // The untouched WIF imports, also as pasted with surrounding whitespace (it is trimmed). + assert_eq!(wallet.import_taddr(format!(" {}\n", wif), 0), taddr); +} + +#[test] +fn test_import_taddr_garbage_no_panic() { + // s1 test 3 (+ D3: one flipped character). Before: panic, and the wallet lock poisoned behind it. + let config = get_test_config(); + let mut wallet = LightWallet::new(None, &config, 0, 0).unwrap(); + let other = LightWallet::new(None, &config, 0, 0).unwrap(); + let wif = other.get_t_secret_keys()[0].1.clone(); + for g in &["secret-extended-key-main1qqqqqqqqqqqqqqqqqqqqqqqq", "", "1", "0OIl", + "3mJr7AoUXx2Wqd"] { // valid base58, far shorter than 33 bytes + r3_import_rejected(&mut wallet, g); + } + for at in &[5usize, 25, 45] { + let typo = r3_flip(&wif, *at); + assert_ne!(typo, wif); + assert_eq!(r3_import_rejected(&mut wallet, &typo), + "Error: Couldn't decode the transparent private key (not valid Base58Check)"); + } +} + +#[test] +fn test_import_refused_when_encrypted() { + // s1 test 1. Before: both imports succeeded with no enc_key, then lock() failed after zeroing the seed. + let config = get_test_config(); + let mut wallet = LightWallet::new(None, &config, 0, 0).unwrap(); + let other = LightWallet::new(None, &config, 0, 0).unwrap(); + let wif = other.get_t_secret_keys()[0].1.clone(); + let sk = other.get_z_private_keys()[0].1.clone(); + let phrase = wallet.get_seed_phrase(); + let counts = |w: &LightWallet| (w.tkeys.read().unwrap().len(), w.zkeys.read().unwrap().len()); + let before = counts(&wallet); + + wallet.encrypt("pw".to_string()).unwrap(); + wallet.unlock("pw".to_string()).unwrap(); + for r in vec![wallet.add_imported_sk(sk.clone(), 0), wallet.import_taddr(wif.clone(), 0)] { + assert_eq!(r, "Error: Can't import a private key while the wallet is encrypted - remove encryption, import, then encrypt again"); + // the C++ reason mapper keys on these words; this reason must not be mistaken for them + assert!(!r.contains("locked") && !r.contains("decode") && !r.contains("already exists"), "{}", r); + } + assert_eq!(counts(&wallet), before); + + wallet.lock().unwrap(); + wallet.unlock("pw".to_string()).unwrap(); + assert_eq!(wallet.get_seed_phrase(), phrase); + assert!(wallet.tkeys.read().unwrap().iter().all(|tk| tk.tkey.is_some())); + assert!(wallet.zkeys.read().unwrap().iter().all(|zk| zk.extsk.is_some())); + + // Once encryption is removed, the same keys import. + wallet.remove_encryption("pw".to_string()).unwrap(); + assert_eq!(wallet.import_taddr(wif, 0), other.get_t_secret_keys()[0].0); + assert!(!wallet.add_imported_sk(sk, 0).starts_with("Error")); +} + +#[test] +fn test_import_vk_ok_when_encrypted_unlocked() { + // s1 test 4: a regression guard (passes before and after). A viewing key needs no enc_key, so it still + // imports into an encrypted, unlocked wallet, and the wallet still locks and unlocks. + let config = get_test_config(); + let mut wallet = LightWallet::new(None, &config, 0, 0).unwrap(); + let other = LightWallet::new(None, &config, 0, 0).unwrap(); + let (zaddr, _, vk) = other.get_z_private_keys()[0].clone(); + let phrase = wallet.get_seed_phrase(); + + wallet.encrypt("pw".to_string()).unwrap(); + wallet.unlock("pw".to_string()).unwrap(); + assert_eq!(wallet.add_imported_vk(vk, 0), zaddr); + wallet.lock().unwrap(); + wallet.unlock("pw".to_string()).unwrap(); + assert_eq!(wallet.get_seed_phrase(), phrase); +}