Skip to content

Commit de97e4f

Browse files
adacapo21claude
andcommitted
fix(ows-lib): refuse Cardano operations on wallets lacking an ed25519_bip32 key
Loading a wallet created before Cardano support silently substituted an all-zero 64-byte key for the missing ed25519_bip32 field. Any Cardano address derived from it would be identical across all legacy wallets and publicly predictable, so funds sent there could be stolen by anyone. The missing key is now kept empty: account derivation skips Cardano for such wallets, and explicit Cardano operations fail with an error telling the user to re-import the wallet. Adds a regression test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 87b9446 commit de97e4f

1 file changed

Lines changed: 56 additions & 10 deletions

File tree

ows/crates/ows-lib/src/ops.rs

Lines changed: 56 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,25 @@ impl Drop for KeyPair {
7777

7878
impl KeyPair {
7979
/// Get the key for a given curve.
80-
fn key_for_curve(&self, curve: ows_signer::Curve) -> &[u8] {
80+
///
81+
/// Errors for `Ed25519Bip32` when the wallet predates Cardano support and
82+
/// has no such key — a fabricated fallback key would be predictable and
83+
/// any funds sent to its addresses stealable.
84+
fn key_for_curve(&self, curve: ows_signer::Curve) -> Result<&[u8], OwsLibError> {
8185
match curve {
82-
ows_signer::Curve::Secp256k1 => &self.secp256k1,
83-
ows_signer::Curve::Ed25519 => &self.ed25519,
84-
ows_signer::Curve::Ed25519Bip32 => &self.ed25519_bip32,
86+
ows_signer::Curve::Secp256k1 => Ok(&self.secp256k1),
87+
ows_signer::Curve::Ed25519 => Ok(&self.ed25519),
88+
ows_signer::Curve::Ed25519Bip32 => {
89+
if self.ed25519_bip32.is_empty() {
90+
Err(OwsLibError::InvalidInput(
91+
"this wallet was created before Cardano support and has no \
92+
Ed25519-BIP32 key; re-import the wallet to enable Cardano"
93+
.into(),
94+
))
95+
} else {
96+
Ok(&self.ed25519_bip32)
97+
}
98+
}
8599
}
86100
}
87101

@@ -109,12 +123,14 @@ impl KeyPair {
109123
let ed = obj["ed25519"]
110124
.as_str()
111125
.ok_or_else(|| OwsLibError::InvalidInput("missing ed25519 key".into()))?;
112-
// Optional — absent in wallets created before Cardano support.
126+
// Optional — absent in wallets created before Cardano support. Kept
127+
// empty in that case; Cardano operations on such wallets fail with an
128+
// actionable error instead of using a predictable placeholder key.
113129
let ed25519_bip32 = if let Some(hex_str) = obj["ed25519_bip32"].as_str() {
114130
hex::decode(hex_str)
115131
.map_err(|e| OwsLibError::InvalidInput(format!("invalid ed25519_bip32 hex: {e}")))?
116132
} else {
117-
vec![0u8; 64]
133+
Vec::new()
118134
};
119135
Ok(KeyPair {
120136
secp256k1: hex::decode(secp)
@@ -131,7 +147,11 @@ fn derive_all_accounts_from_keys(keys: &KeyPair) -> Result<Vec<WalletAccount>, O
131147
let mut accounts = Vec::with_capacity(ALL_CHAIN_TYPES.len());
132148
for ct in &ALL_CHAIN_TYPES {
133149
let signer = signer_for_chain(*ct);
134-
let key = keys.key_for_curve(signer.curve());
150+
// Legacy wallets have no Ed25519-BIP32 key; skip those chains rather
151+
// than derive addresses from a placeholder.
152+
let Ok(key) = keys.key_for_curve(signer.curve()) else {
153+
continue;
154+
};
135155
let address = signer.derive_address(key)?;
136156
let chain = default_chain_for_type(*ct);
137157
accounts.push(WalletAccount {
@@ -168,7 +188,7 @@ pub(crate) fn secret_to_signing_key(
168188
// JSON key pair — extract the right key for this chain's curve
169189
let keys = KeyPair::from_json_bytes(secret.expose())?;
170190
let signer = signer_for_chain(chain_type);
171-
Ok(SecretBytes::from_slice(keys.key_for_curve(signer.curve())))
191+
Ok(SecretBytes::from_slice(keys.key_for_curve(signer.curve())?))
172192
}
173193
}
174194
}
@@ -1197,6 +1217,30 @@ mod tests {
11971217
use super::*;
11981218
use ows_core::OwsError;
11991219

1220+
#[test]
1221+
fn legacy_keypair_without_bip32_key_refuses_cardano() {
1222+
// Wallet JSON from before Cardano support: no ed25519_bip32 field.
1223+
let legacy = serde_json::json!({
1224+
"secp256k1": hex::encode([7u8; 32]),
1225+
"ed25519": hex::encode([9u8; 32]),
1226+
})
1227+
.to_string();
1228+
let keys = KeyPair::from_json_bytes(legacy.as_bytes()).unwrap();
1229+
1230+
// Non-Cardano curves still work.
1231+
assert!(keys.key_for_curve(ows_signer::Curve::Secp256k1).is_ok());
1232+
assert!(keys.key_for_curve(ows_signer::Curve::Ed25519).is_ok());
1233+
1234+
// The missing key must be an error, never a predictable placeholder.
1235+
assert!(keys.key_for_curve(ows_signer::Curve::Ed25519Bip32).is_err());
1236+
1237+
// Account derivation skips Cardano instead of failing or deriving
1238+
// an address from a known key.
1239+
let accounts = derive_all_accounts_from_keys(&keys).unwrap();
1240+
assert!(accounts.iter().all(|a| !a.chain_id.starts_with("cardano:")));
1241+
assert!(accounts.iter().any(|a| a.chain_id.starts_with("eip155:")));
1242+
}
1243+
12001244
// ---- helpers ----
12011245

12021246
/// Build a private-key wallet directly in the vault, bypassing
@@ -1209,14 +1253,16 @@ mod tests {
12091253
) -> WalletInfo {
12101254
let key_bytes = hex::decode(privkey_hex).unwrap();
12111255

1212-
// Generate a random ed25519 key for the other curve
1256+
// Generate random keys for the other curves
12131257
let mut ed_key = vec![0u8; 32];
12141258
getrandom::getrandom(&mut ed_key).unwrap();
1259+
let mut bip32_key = vec![0u8; 64];
1260+
getrandom::getrandom(&mut bip32_key).unwrap();
12151261

12161262
let keys = KeyPair {
12171263
secp256k1: key_bytes,
12181264
ed25519: ed_key,
1219-
ed25519_bip32: vec![0u8; 64],
1265+
ed25519_bip32: bip32_key,
12201266
};
12211267
let accounts = derive_all_accounts_from_keys(&keys).unwrap();
12221268
let payload = keys.to_json_bytes();

0 commit comments

Comments
 (0)