Skip to content

Commit d7deb69

Browse files
committed
fix: return error when parsing xprv into DescriptorPublicKey
Refactor FromStr for DescriptorPublicKey so that it can parse a xprv/tprv but then error with a new variant `DescriptorKeyParseError::UnexpectedXPrivateKey` so that the user knows to use `Descriptor<DescriptorPublicKey>::parse_descriptor()` instead. Added test that reflects what was reported in the issues: bitcoinfuzz/bitcoinfuzz#70 #785
1 parent 64eed44 commit d7deb69

2 files changed

Lines changed: 80 additions & 43 deletions

File tree

src/descriptor/key.rs

Lines changed: 60 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,8 @@ pub enum DescriptorKeyParseError {
534534
XonlyPublicKey(bitcoin::secp256k1::Error),
535535
/// XKey parsing error
536536
XKeyParseError(XKeyParseError),
537+
/// Unexpected XPrivateKey when parsing [`Descriptor<DescriptorPublicKey>`][crate::Descriptor].
538+
UnexpectedXPrivateKey,
537539
}
538540

539541
impl fmt::Display for DescriptorKeyParseError {
@@ -553,6 +555,9 @@ impl fmt::Display for DescriptorKeyParseError {
553555
Self::WifPrivateKey(err) => err.fmt(f),
554556
Self::XonlyPublicKey(err) => err.fmt(f),
555557
Self::XKeyParseError(err) => err.fmt(f),
558+
Self::UnexpectedXPrivateKey => f.write_str(
559+
"unexpected private extended key. Use `Descriptor::parse_descriptor` instead to preserve private key data in the returned KeyMap",
560+
),
556561
}
557562
}
558563
}
@@ -570,7 +575,7 @@ impl error::Error for DescriptorKeyParseError {
570575
Self::WifPrivateKey(err) => Some(err),
571576
Self::XonlyPublicKey(err) => Some(err),
572577
Self::XKeyParseError(err) => Some(err),
573-
Self::MalformedKeyData(_) => None,
578+
Self::MalformedKeyData(_) | Self::UnexpectedXPrivateKey => None,
574579
}
575580
}
576581
}
@@ -733,50 +738,54 @@ impl FromStr for DescriptorPublicKey {
733738

734739
let (key_part, origin) = parse_key_origin(s)?;
735740

736-
if key_part.contains("pub") {
737-
let (xpub, derivation_paths, wildcard) = parse_xkey_deriv(key_part)?;
738-
if derivation_paths.len() > 1 {
739-
Ok(Self::MultiXPub(DescriptorMultiXKey {
740-
origin,
741-
xkey: xpub,
742-
derivation_paths: DerivPaths::new(derivation_paths).expect("Not empty"),
743-
wildcard,
744-
}))
745-
} else {
746-
Ok(Self::XPub(DescriptorXKey {
747-
origin,
748-
xkey: xpub,
749-
derivation_path: derivation_paths.into_iter().next().unwrap_or_default(),
750-
wildcard,
751-
}))
752-
}
753-
} else {
754-
let key = match key_part.len() {
755-
64 => {
756-
let x_only_key = XOnlyPublicKey::from_str(key_part)
757-
.map_err(DescriptorKeyParseError::XonlyPublicKey)?;
758-
SinglePubKey::XOnly(x_only_key)
741+
match key_part.get(..4) {
742+
Some("xprv" | "tprv") => Err(DescriptorKeyParseError::UnexpectedXPrivateKey),
743+
Some("xpub" | "tpub") => {
744+
let (xpub, derivation_paths, wildcard) = parse_xkey_deriv(key_part)?;
745+
if derivation_paths.len() > 1 {
746+
Ok(Self::MultiXPub(DescriptorMultiXKey {
747+
origin,
748+
xkey: xpub,
749+
derivation_paths: DerivPaths::new(derivation_paths).expect("Not empty"),
750+
wildcard,
751+
}))
752+
} else {
753+
Ok(Self::XPub(DescriptorXKey {
754+
origin,
755+
xkey: xpub,
756+
derivation_path: derivation_paths.into_iter().next().unwrap_or_default(),
757+
wildcard,
758+
}))
759759
}
760-
66 | 130 => {
761-
if !(&key_part[0..2] == "02"
762-
|| &key_part[0..2] == "03"
763-
|| &key_part[0..2] == "04")
764-
{
760+
}
761+
_ => {
762+
let key = match key_part.len() {
763+
64 => {
764+
let x_only_key = XOnlyPublicKey::from_str(key_part)
765+
.map_err(DescriptorKeyParseError::XonlyPublicKey)?;
766+
SinglePubKey::XOnly(x_only_key)
767+
}
768+
66 | 130 => {
769+
if !(&key_part[0..2] == "02"
770+
|| &key_part[0..2] == "03"
771+
|| &key_part[0..2] == "04")
772+
{
773+
return Err(DescriptorKeyParseError::MalformedKeyData(
774+
MalformedKeyDataKind::InvalidFullPublicKeyPrefix,
775+
));
776+
}
777+
let key = bitcoin::PublicKey::from_str(key_part)
778+
.map_err(DescriptorKeyParseError::FullPublicKey)?;
779+
SinglePubKey::FullKey(key)
780+
}
781+
_ => {
765782
return Err(DescriptorKeyParseError::MalformedKeyData(
766-
MalformedKeyDataKind::InvalidFullPublicKeyPrefix,
767-
));
783+
MalformedKeyDataKind::InvalidPublicKeyLength,
784+
))
768785
}
769-
let key = bitcoin::PublicKey::from_str(key_part)
770-
.map_err(DescriptorKeyParseError::FullPublicKey)?;
771-
SinglePubKey::FullKey(key)
772-
}
773-
_ => {
774-
return Err(DescriptorKeyParseError::MalformedKeyData(
775-
MalformedKeyDataKind::InvalidPublicKeyLength,
776-
))
777-
}
778-
};
779-
Ok(Self::Single(SinglePub { key, origin }))
786+
};
787+
Ok(Self::Single(SinglePub { key, origin }))
788+
}
780789
}
781790
}
782791
}
@@ -1514,7 +1523,8 @@ mod test {
15141523
use serde_test::{assert_tokens, Token};
15151524

15161525
use super::{
1517-
DescriptorMultiXKey, DescriptorPublicKey, DescriptorSecretKey, MiniscriptKey, Wildcard,
1526+
DescriptorKeyParseError, DescriptorMultiXKey, DescriptorPublicKey, DescriptorSecretKey,
1527+
MiniscriptKey, Wildcard,
15181528
};
15191529
use crate::descriptor::key::NonDefiniteKeyError;
15201530
use crate::prelude::*;
@@ -1570,6 +1580,13 @@ mod test {
15701580
DescriptorPublicKey::from_str(desc).unwrap_err().to_string(),
15711581
"only full public keys with prefixes '02', '03' or '04' are allowed"
15721582
);
1583+
1584+
// unexpected xprv key when attempting to parse a DescriptorPublicKey
1585+
let desc = "tprv8ZgxMBicQKsPcwcD4gSnMti126ZiETsuX7qwrtMypr6FBwAP65puFn4v6c3jrN9VwtMRMph6nyT63NrfUL4C3nBzPcduzVSuHD7zbX2JKVc";
1586+
assert_eq!(
1587+
desc.parse::<DescriptorPublicKey>().unwrap_err().to_string(),
1588+
DescriptorKeyParseError::UnexpectedXPrivateKey.to_string()
1589+
);
15731590
}
15741591

15751592
#[test]

src/descriptor/mod.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2128,6 +2128,26 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))";
21282128
assert_eq!(descriptor_str, descriptor.to_string_with_secret(&keymap));
21292129
}
21302130

2131+
// https://github.com/bitcoinfuzz/bitcoinfuzz/issues/70
2132+
// https://github.com/rust-bitcoin/rust-miniscript/issues/785
2133+
#[test]
2134+
fn parse_descriptor_preserves_xprv_in_keymap() {
2135+
let secp = &secp256k1::Secp256k1::signing_only();
2136+
2137+
for desc in [
2138+
"pk(xprv9s21ZrQH143K31xYSDQpPDxsXRTUcvj2iNHm5NUtrGiGG5e2DtALGdso3pGz6ssrdK4PFmM8NSpSBHNqPqm55Qn3LqFtT2emdEXVYsCzC2U/*)#dl4n4vmp",
2139+
// xprv containing "pub"
2140+
"pk(xprv9s21ZrQH143K3MByafknWupz9D5c3Wz3MrAZpC5KFxUuwwbefg6BVSWFwyUbEcqxGTpubCGtQyC3m3vm8rZkmN1TNoc7n6VdZBt5NeXdxwV/*)#nyw8pgdu",
2141+
] {
2142+
assert_eq!(desc.parse::<Descriptor<DescriptorPublicKey>>().unwrap_err().to_string(), DescriptorKeyParseError::UnexpectedXPrivateKey.to_string());
2143+
2144+
let (descriptor, key_map) =
2145+
Descriptor::<DescriptorPublicKey>::parse_descriptor(secp, desc).unwrap();
2146+
assert_eq!(key_map.len(), 1);
2147+
assert_eq!(descriptor.to_string_with_secret(&key_map), desc);
2148+
}
2149+
}
2150+
21312151
#[test]
21322152
fn checksum_for_nested_sh() {
21332153
let descriptor_str = "sh(wpkh(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL))";

0 commit comments

Comments
 (0)