Add validation for private key format and master key constraints

This commit adds additional validation checks when decoding extended private keys:

1. Verifies that byte 45 is zero as required by BIP-32 specification
2. For master keys (depth=0), ensures parent fingerprint is zero
3. For master keys (depth=0), ensures child number is zero

These checks improve security by rejecting malformed keys that could
potentially lead to unexpected behavior. Added corresponding error types
and unit tests to verify each validation rule.
This commit is contained in:
Erick Cestari 2025-03-10 10:02:50 -03:00
parent 694d926ed0
commit 8f74b823ab
1 changed files with 69 additions and 0 deletions

View File

@ -518,6 +518,12 @@ pub enum Error {
InvalidPublicKeyHexLength(usize),
/// Base58 decoded data was an invalid length.
InvalidBase58PayloadLength(InvalidBase58PayloadLengthError),
/// Invalid private key prefix (byte 45 must be 0)
InvalidPrivateKeyPrefix,
/// Non-zero parent fingerprint for a master key (depth 0)
NonZeroParentFingerprintForMasterKey,
/// Non-zero child number for a master key (depth 0)
NonZeroChildNumberForMasterKey,
}
impl From<Infallible> for Error {
@ -544,6 +550,11 @@ impl fmt::Display for Error {
InvalidPublicKeyHexLength(got) =>
write!(f, "PublicKey hex should be 66 or 130 digits long, got: {}", got),
InvalidBase58PayloadLength(ref e) => write_err!(f, "base58 payload"; e),
InvalidPrivateKeyPrefix =>
f.write_str("invalid private key prefix, byte 45 must be 0 as required by BIP-32"),
NonZeroParentFingerprintForMasterKey =>
f.write_str("non-zero parent fingerprint in master key"),
NonZeroChildNumberForMasterKey => f.write_str("non-zero child number in master key"),
}
}
}
@ -565,6 +576,9 @@ impl std::error::Error for Error {
| UnknownVersion(_)
| WrongExtendedKeyLength(_)
| InvalidPublicKeyHexLength(_) => None,
InvalidPrivateKeyPrefix => None,
NonZeroParentFingerprintForMasterKey => None,
NonZeroChildNumberForMasterKey => None,
}
}
}
@ -699,6 +713,23 @@ impl Xpriv {
return Err(Error::UnknownVersion([b0, b1, b2, b3]));
};
// Check that byte 45 is 0 as required by BIP-32 for private keys
if data[45] != 0 {
return Err(Error::InvalidPrivateKeyPrefix);
}
// For master keys (depth=0), both parent fingerprint and child number must be zero
let depth = data[4];
if depth == 0 {
if !data[5..9].iter().all(|&b| b == 0) {
return Err(Error::NonZeroParentFingerprintForMasterKey);
}
if !data[9..13].iter().all(|&b| b == 0) {
return Err(Error::NonZeroChildNumberForMasterKey);
}
}
Ok(Xpriv {
network,
depth: data[4],
@ -1235,6 +1266,44 @@ mod tests {
"xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y");
}
#[test]
fn test_reject_xpriv_with_non_zero_byte_at_index_45() {
let mut xpriv = base58::decode_check("xprv9wSp6B7kry3Vj9m1zSnLvN3xH8RdsPP1Mh7fAaR7aRLcQMKTR2vidYEeEg2mUCTAwCd6vnxVrcjfy2kRgVsFawNzmjuHc2YmYRmagcEPdU9").unwrap();
// Modify byte at index 45 to be non-zero (e.g., 1)
xpriv[45] = 1;
let result = Xpriv::decode(&xpriv);
assert!(result.is_err());
match result {
Err(Error::InvalidPrivateKeyPrefix) => {}
_ => panic!("Expected InvalidPrivateKeyPrefix error, got {:?}", result),
}
}
#[test]
fn test_reject_xpriv_with_zero_depth_and_non_zero_index() {
let result = "xprv9s21ZrQH4r4TsiLvyLXqM9P7k1K3EYhA1kkD6xuquB5i39AU8KF42acDyL3qsDbU9NmZn6MsGSUYZEsuoePmjzsB3eFKSUEh3Gu1N3cqVUN".parse::<Xpriv>();
assert!(result.is_err());
match result {
Err(Error::NonZeroChildNumberForMasterKey) => {}
_ => panic!("Expected NonZeroChildNumberForMasterKey error, got {:?}", result),
}
}
#[test]
fn test_reject_xpriv_with_zero_depth_and_non_zero_parent_fingerprint() {
let result = "xprv9s2SPatNQ9Vc6GTbVMFPFo7jsaZySyzk7L8n2uqKXJen3KUmvQNTuLh3fhZMBoG3G4ZW1N2kZuHEPY53qmbZzCHshoQnNf4GvELZfqTUrcv".parse::<Xpriv>();
assert!(result.is_err());
match result {
Err(Error::NonZeroParentFingerprintForMasterKey) => {}
_ => panic!("Expected NonZeroParentFingerprintForMasterKey error, got {:?}", result),
}
}
#[test]
#[cfg(feature = "serde")]
pub fn encode_decode_childnumber() {