From 877f9af36421bab14df4dd2310267465910d683e Mon Sep 17 00:00:00 2001
From: "Tobin C. Harding" <me@tobin.cc>
Date: Mon, 23 Jan 2023 14:49:09 +1100
Subject: [PATCH] Add new hex parse error variant

Recently we used an error type that holds only one expected hex string
length when parsing but for `PublicKey`s we have two (66 and 130). Add a
new error variant to express the error. Requires adding a variant to
`bip32` for the same thing.

Fix: #1281
---
 bitcoin/src/bip32.rs      |  7 ++++++-
 bitcoin/src/crypto/key.rs | 32 +++++++++++++++++++++++++++-----
 2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/bitcoin/src/bip32.rs b/bitcoin/src/bip32.rs
index bb1cb928..2497d4ab 100644
--- a/bitcoin/src/bip32.rs
+++ b/bitcoin/src/bip32.rs
@@ -455,6 +455,8 @@ pub enum Error {
     Base58(base58::Error),
     /// Hexadecimal decoding error
     Hex(hex::Error),
+    /// `PublicKey` hex should be 66 or 130 digits long.
+    InvalidPublicKeyHexLength(usize),
 }
 
 impl fmt::Display for Error {
@@ -473,6 +475,7 @@ impl fmt::Display for Error {
                 write!(f, "encoded extended key data has wrong length {}", len),
             Error::Base58(ref e) => write_err!(f, "base58 encoding error"; e),
             Error::Hex(ref e) => write_err!(f, "Hexadecimal decoding error"; e),
+            Error::InvalidPublicKeyHexLength(got) => write!(f, "PublicKey hex should be 66 or 130 digits long, got: {}", got),
         }
     }
 }
@@ -492,7 +495,8 @@ impl std::error::Error for Error {
             | InvalidChildNumberFormat
             | InvalidDerivationPathFormat
             | UnknownVersion(_)
-            | WrongExtendedKeyLength(_) => None,
+            | WrongExtendedKeyLength(_)
+            | InvalidPublicKeyHexLength(_) => None,
         }
     }
 }
@@ -504,6 +508,7 @@ impl From<key::Error> for Error {
             key::Error::Secp256k1(e) => Error::Secp256k1(e),
             key::Error::InvalidKeyPrefix(_) => Error::Secp256k1(secp256k1::Error::InvalidPublicKey),
             key::Error::Hex(e) => Error::Hex(e),
+            key::Error::InvalidHexLength(got) => Error::InvalidPublicKeyHexLength(got),
         }
     }
 }
diff --git a/bitcoin/src/crypto/key.rs b/bitcoin/src/crypto/key.rs
index 6c164752..acad5fa1 100644
--- a/bitcoin/src/crypto/key.rs
+++ b/bitcoin/src/crypto/key.rs
@@ -30,7 +30,9 @@ pub enum Error {
     /// Invalid key prefix error
     InvalidKeyPrefix(u8),
     /// Hex decoding error
-    Hex(hex::Error)
+    Hex(hex::Error),
+    /// `PublicKey` hex should be 66 or 130 digits long.
+    InvalidHexLength(usize),
 }
 
 impl fmt::Display for Error {
@@ -39,7 +41,8 @@ impl fmt::Display for Error {
             Error::Base58(ref e) => write_err!(f, "key base58 error"; e),
             Error::Secp256k1(ref e) => write_err!(f, "key secp256k1 error"; e),
             Error::InvalidKeyPrefix(ref b) => write!(f, "key prefix invalid: {}", b),
-            Error::Hex(ref e) => write_err!(f, "key hex decoding error"; e)
+            Error::Hex(ref e) => write_err!(f, "key hex decoding error"; e),
+            Error::InvalidHexLength(got) => write!(f, "PublicKey hex should be 66 or 130 digits long, got: {}", got),
         }
     }
 }
@@ -53,8 +56,8 @@ impl std::error::Error for Error {
         match self {
             Base58(e) => Some(e),
             Secp256k1(e) => Some(e),
-            InvalidKeyPrefix(_) => None,
             Hex(e) => Some(e),
+            InvalidKeyPrefix(_) | InvalidHexLength(_) => None,
         }
     }
 }
@@ -159,7 +162,8 @@ impl PublicKey {
                 Error::Base58(_) => "base58 error",
                 Error::Secp256k1(_) => "secp256k1 error",
                 Error::InvalidKeyPrefix(_) => "invalid key prefix",
-                Error::Hex(_) => "hex decoding error"
+                Error::Hex(_) => "hex decoding error",
+                Error::InvalidHexLength(_) => "invalid hex string length",
             };
             io::Error::new(io::ErrorKind::InvalidData, reason)
         })
@@ -287,7 +291,7 @@ impl FromStr for PublicKey {
         match s.len() {
             66 => PublicKey::from_slice(&<[u8; 33]>::from_hex(s)?),
             130 => PublicKey::from_slice(&<[u8; 65]>::from_hex(s)?),
-            len => Err(Error::Hex(hex::Error::InvalidLength(66, len))),
+            len => Err(Error::InvalidHexLength(len)),
         }
     }
 }
@@ -851,4 +855,22 @@ mod tests {
         let _ = PublicKey::new(kp);
         let _ = PublicKey::new_uncompressed(kp);
     }
+
+    #[test]
+    fn public_key_from_str_wrong_length() {
+        // Sanity checks, we accept string length 130 digits.
+        let s = "042e58afe51f9ed8ad3cc7897f634d881fdbe49a81564629ded8156bebd2ffd1af191923a2964c177f5b5923ae500fca49e99492d534aa3759d6b25a8bc971b133";
+        assert_eq!(s.len(), 130);
+        assert!(PublicKey::from_str(s).is_ok());
+        // And 66 digits.
+        let s = "032e58afe51f9ed8ad3cc7897f634d881fdbe49a81564629ded8156bebd2ffd1af";
+        assert_eq!(s.len(), 66);
+        assert!(PublicKey::from_str(s).is_ok());
+
+        let s = "aoeusthb";
+        assert_eq!(s.len(), 8);
+        let res = PublicKey::from_str(s);
+        assert!(res.is_err());
+        assert_eq!(res.unwrap_err(), Error::InvalidHexLength(8));
+    }
 }