Skip to content

Commit 1d22086

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 a19d92c commit 1d22086

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
}
@@ -735,50 +740,54 @@ impl FromStr for DescriptorPublicKey {
735740

736741
let (key_part, origin) = parse_key_origin(s)?;
737742

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

15201529
use super::{
1521-
DescriptorMultiXKey, DescriptorPublicKey, DescriptorSecretKey, MiniscriptKey, Wildcard,
1530+
DescriptorKeyParseError, DescriptorMultiXKey, DescriptorPublicKey, DescriptorSecretKey,
1531+
MiniscriptKey, Wildcard,
15221532
};
15231533
use crate::descriptor::key::NonDefiniteKeyError;
15241534
use crate::prelude::*;
@@ -1574,6 +1584,13 @@ mod test {
15741584
DescriptorPublicKey::from_str(desc).unwrap_err().to_string(),
15751585
"only full public keys with prefixes '02', '03' or '04' are allowed"
15761586
);
1587+
1588+
// unexpected xprv key when attempting to parse a DescriptorPublicKey
1589+
let desc = "tprv8ZgxMBicQKsPcwcD4gSnMti126ZiETsuX7qwrtMypr6FBwAP65puFn4v6c3jrN9VwtMRMph6nyT63NrfUL4C3nBzPcduzVSuHD7zbX2JKVc";
1590+
assert_eq!(
1591+
desc.parse::<DescriptorPublicKey>().unwrap_err().to_string(),
1592+
DescriptorKeyParseError::UnexpectedXPrivateKey.to_string()
1593+
);
15771594
}
15781595

15791596
#[test]

src/descriptor/mod.rs

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

2155+
// https://github.com/bitcoinfuzz/bitcoinfuzz/issues/70
2156+
// https://github.com/rust-bitcoin/rust-miniscript/issues/785
2157+
#[test]
2158+
fn parse_descriptor_preserves_xprv_in_keymap() {
2159+
let secp = &secp256k1::Secp256k1::signing_only();
2160+
2161+
for desc in [
2162+
"pk(xprv9s21ZrQH143K31xYSDQpPDxsXRTUcvj2iNHm5NUtrGiGG5e2DtALGdso3pGz6ssrdK4PFmM8NSpSBHNqPqm55Qn3LqFtT2emdEXVYsCzC2U/*)#dl4n4vmp",
2163+
// xprv containing "pub"
2164+
"pk(xprv9s21ZrQH143K3MByafknWupz9D5c3Wz3MrAZpC5KFxUuwwbefg6BVSWFwyUbEcqxGTpubCGtQyC3m3vm8rZkmN1TNoc7n6VdZBt5NeXdxwV/*)#nyw8pgdu",
2165+
] {
2166+
assert_eq!(desc.parse::<Descriptor<DescriptorPublicKey>>().unwrap_err().to_string(), DescriptorKeyParseError::UnexpectedXPrivateKey.to_string());
2167+
2168+
let (descriptor, key_map) =
2169+
Descriptor::<DescriptorPublicKey>::parse_descriptor(secp, desc).unwrap();
2170+
assert_eq!(key_map.len(), 1);
2171+
assert_eq!(descriptor.to_string_with_secret(&key_map), desc);
2172+
}
2173+
}
2174+
21552175
#[test]
21562176
fn checksum_for_nested_sh() {
21572177
let descriptor_str = "sh(wpkh(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL))";

0 commit comments

Comments
 (0)