Skip to content

Commit a8c6edc

Browse files
committed
Validate hd-path on cached Ledger lookups.
1 parent 52ad3c0 commit a8c6edc

2 files changed

Lines changed: 52 additions & 16 deletions

File tree

cmd/soroban-cli/src/commands/message/verify.rs

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -312,20 +312,6 @@ mod tests {
312312
));
313313
}
314314

315-
#[test]
316-
fn test_verify_rejects_ledger_as_public_key() {
317-
let cmd = super::Cmd {
318-
message: Some("Hello, World!".to_string()),
319-
base64: false,
320-
signature: "fO5dbYhXUhBMhe6kId/cuVq/AfEnHRHEvsP8vXh03M1uLpi5e46yO2Q8rEBzu3feXQewcQE5GArp88u6ePK6BA==".to_string(),
321-
public_key: "ledger".to_string(),
322-
hd_path: None,
323-
locator: setup_locator(),
324-
};
325-
let err = cmd.run(&global_args()).unwrap_err();
326-
assert!(matches!(err, Error::Locator(_)));
327-
}
328-
329315
#[test]
330316
fn test_verify_rejects_secure_store_as_public_key() {
331317
let cmd = super::Cmd {

cmd/soroban-cli/src/config/secret.rs

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ pub enum Error {
3232
SecureStoreDoesNotRevealSecretKey,
3333
#[error(transparent)]
3434
Ledger(#[from] signer::ledger::Error),
35+
#[error(
36+
"--hd-path {requested} does not match the path stored on this Ledger identity ({cached})"
37+
)]
38+
LedgerHdPathMismatch { cached: usize, requested: usize },
39+
#[error("--hd-path {0} is out of range for a Ledger account index")]
40+
HdPathOutOfRange(usize),
3541
}
3642

3743
#[derive(Debug, clap::Args, Clone)]
@@ -180,7 +186,18 @@ impl Secret {
180186
}
181187
Ok(secure_store::get_public_key(entry_name, effective)?)
182188
}
183-
Secret::Ledger { public_key, .. } => Ok(PublicKey::from_string(public_key)?),
189+
Secret::Ledger {
190+
public_key,
191+
hd_path: cached_hd_path,
192+
..
193+
} => {
194+
let cached = cached_hd_path.unwrap_or_default();
195+
let requested = index.unwrap_or(cached);
196+
if cached != requested {
197+
return Err(Error::LedgerHdPathMismatch { cached, requested });
198+
}
199+
Ok(PublicKey::from_string(public_key)?)
200+
}
184201
_ => {
185202
let key = self.key_pair(index)?;
186203
Ok(stellar_strkey::ed25519::PublicKey::from_payload(
@@ -202,7 +219,9 @@ impl Secret {
202219
..
203220
} => {
204221
let effective = hd_path.or(*cached_hd_path).unwrap_or_default();
205-
let hd_path: u32 = effective.try_into().expect("usize bigger than u32");
222+
let hd_path: u32 = effective
223+
.try_into()
224+
.map_err(|_| Error::HdPathOutOfRange(effective))?;
206225
SignerKind::Ledger(ledger::new(hd_path).await?)
207226
}
208227
Secret::SecureStore {
@@ -506,6 +525,37 @@ mod tests {
506525
assert_eq!(pk.to_string(), TEST_PUBLIC_KEY);
507526
}
508527

528+
#[test]
529+
fn test_ledger_public_key_rejects_mismatched_hd_path() {
530+
// Caller asks for a different account index than the one cached on
531+
// disk; returning the cached key would leak the wrong address.
532+
let secret = Secret::Ledger {
533+
hardware: HardwareKind::Ledger,
534+
public_key: TEST_PUBLIC_KEY.to_string(),
535+
hd_path: Some(5),
536+
};
537+
assert!(matches!(
538+
secret.public_key(Some(7)).unwrap_err(),
539+
Error::LedgerHdPathMismatch {
540+
cached: 5,
541+
requested: 7
542+
},
543+
));
544+
}
545+
546+
#[test]
547+
fn test_ledger_public_key_treats_none_and_zero_as_equivalent() {
548+
let secret = Secret::Ledger {
549+
hardware: HardwareKind::Ledger,
550+
public_key: TEST_PUBLIC_KEY.to_string(),
551+
hd_path: None,
552+
};
553+
assert_eq!(
554+
secret.public_key(Some(0)).unwrap().to_string(),
555+
TEST_PUBLIC_KEY
556+
);
557+
}
558+
509559
#[test]
510560
fn test_ledger_private_key_is_rejected() {
511561
let secret = Secret::Ledger {

0 commit comments

Comments
 (0)