fix(signer): match BIP32 origin via matches, not string prefix

src/signer.rs

@@ -1,6 +1,4 @@
 use alloc::collections::BTreeMap;
-use alloc::string::ToString;
-use alloc::vec::Vec;
 
 use bitcoin::{
     psbt::{GetKey, GetKeyError, KeyRequest},
@@ -34,28 +32,30 @@ impl GetKey for Signer {
                 }
                 (_, desc_sk) => {
                     for desc_sk in desc_sk.clone().into_single_keys() {
-                        if let KeyRequest::Bip32((fingerprint, derivation)) = &key_request {
-                            if let DescriptorSecretKey::XPrv(k) = desc_sk {
-                                // We have the xprv for the request
-                                if let Ok(Some(prv)) =
-                                    GetKey::get_key(&k.xkey, key_request.clone(), secp)
-                                {
-                                    return Ok(Some(prv));
-                                }
-                                // The key origin is a strict prefix of the request derivation
-                                if let Some((fp, path)) = &k.origin {
-                                    if fingerprint == fp
-                                        && derivation.to_string().starts_with(&path.to_string())
-                                    {
-                                        let to_derive = derivation
-                                            .into_iter()
-                                            .skip(path.len())
-                                            .cloned()
-                                            .collect::<Vec<_>>();
-                                        let derived = k.xkey.derive_priv(secp, &to_derive)?;
-                                        return Ok(Some(derived.to_priv()));
-                                    }
-                                }
+                        if let (DescriptorSecretKey::XPrv(k), KeyRequest::Bip32(key_source)) =
+                            (desc_sk, &key_request)
+                        {
+                            // We may hold the xprv for the request directly.
+                            if let Ok(Some(prv)) =
+                                GetKey::get_key(&k.xkey, key_request.clone(), secp)
+                            {
+                                return Ok(Some(prv));
+                            }
+                            // Otherwise let miniscript's `matches` confirm this key
+                            // represents the request (handling the key origin and
+                            // wildcard); a raw prefix check would also derive paths the
+                            // descriptor never declares, e.g. a sibling of the wildcard.
+                            if k.matches(key_source, secp).is_some() {
+                                // `xkey` is anchored at the origin, so strip the origin
+                                // prefix from the master-relative request path; with no
+                                // origin the whole path is relative to `xkey`.
+                                let (_, derivation) = key_source;
+                                let to_derive = match &k.origin {
+                                    Some((_, origin)) => &derivation[origin.len()..],
+                                    None => derivation.as_ref(),
+                                };
+                                let derived = k.xkey.derive_priv(secp, &to_derive)?;
+                                return Ok(Some(derived.to_priv()));
                             }
                         }
                     }
@@ -70,6 +70,7 @@ impl GetKey for Signer {
 #[cfg(test)]
 mod test {
     use crate::bitcoin::bip32::ChildNumber;
+    use alloc::string::ToString;
     use core::str::FromStr;
     use std::string::String;
 
@@ -147,11 +148,6 @@ mod test {
                 desc: format!("tr([{fp}/{path}]{derived}/0/*)"),
                 derivation: format!("{path}/0/7"),
             },
-            TestCase {
-                name: "key origin matches request derivation",
-                desc: format!("tr([{fp}/{path}]{derived}/0/*)"),
-                derivation: path.to_string(),
-            },
         ];
 
         for test in cases {
@@ -172,6 +168,33 @@ mod test {
         Ok(())
     }
 
+    // `matches` only signs for keys the descriptor actually represents. Requests
+    // that share the origin as a prefix but fall outside the declared derivation
+    // are rejected: the account-level key itself (request == origin) and a sibling
+    // of the wildcard branch (`/9/*` when the descriptor declares `/0/*`).
+    #[test]
+    fn get_key_bip32_rejects_paths_outside_descriptor() -> anyhow::Result<()> {
+        let secp = Secp256k1::new();
+        let xprv: Xpriv = "tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L".parse()?;
+        let fp = xprv.fingerprint(&secp);
+        let path: DerivationPath = "86h/1h/0h".parse()?;
+        let derived = xprv.derive_priv(&secp, &path)?;
+        let desc = format!("tr([{fp}/{path}]{derived}/0/*)");
+
+        for derivation in [path.to_string(), format!("{path}/9/7")] {
+            let deriv: DerivationPath = derivation.parse()?;
+            let req = KeyRequest::Bip32((fp, deriv));
+            let (_, keymap) = Descriptor::parse_descriptor(&secp, &desc)?;
+            let res = Signer(keymap).get_key(req, &secp);
+            assert!(
+                matches!(res, Ok(None)),
+                "expected None for {derivation}: {res:?}"
+            );
+        }
+
+        Ok(())
+    }
+
     #[test]
     fn get_key_xpriv_with_key_origin() -> anyhow::Result<()> {
         let secp = Secp256k1::new();
@@ -202,4 +225,26 @@ mod test {
 
         Ok(())
     }
+
+    // The origin "84h/1h/0h/1" is a string prefix of "84h/1h/0h/10" even though
+    // m/84'/1'/0'/1 is NOT a derivation-path prefix of m/84'/1'/0'/10. The signer
+    // must reject this request instead of leaking the key at the origin path.
+    #[test]
+    fn get_key_bip32_string_prefix_not_path_prefix() -> anyhow::Result<()> {
+        let secp = Secp256k1::new();
+        let xprv: Xpriv = "tprv8ZgxMBicQKsPdy6LMhUtFHAgpocR8GC6QmwMSFpZs7h6Eziw3SpThFfczTDh5rW2krkqffa11UpX3XkeTTB2FvzZKWXqPY54Y6Rq4AQ5R8L".parse()?;
+        let fp = xprv.fingerprint(&secp);
+
+        let origin_path: DerivationPath = "84h/1h/0h/1".parse()?;
+        let derived = xprv.derive_priv(&secp, &origin_path)?;
+        let desc = format!("wpkh([{fp}/{origin_path}]{derived}/*)");
+        let (_, keymap) = Descriptor::parse_descriptor(&secp, &desc)?;
+
+        let request_path: DerivationPath = "84h/1h/0h/10".parse()?;
+        let req = KeyRequest::Bip32((fp, request_path));
+        let res = Signer(keymap).get_key(req, &secp);
+
+        assert!(matches!(res, Ok(None)), "expected None, got: {res:?}");
+        Ok(())
+    }
 }