Assert CPubKey::ValidLength to the pubkey's header-relevent size

Empact · furszy · commit fe31b31eecdd · 2020-06-18T01:04:24.000-03:00
Previously this was an inline test where the specificity was probably judged overly specific. As a class method it makes sense to maintain consistency. And replace some magic values with their constant equivalents. Excludes changes to the following functions we don't have: - ExtractPubKey (bitcoin#6415) - IsCompressedPubKey (bitcoin#8499)
diff --git a/src/pubkey.h b/src/pubkey.h
@@ -69,6 +69,11 @@ class CPubKey
     }
 
 public:
+
+    bool static ValidSize(const std::vector<unsigned char> &vch) {
+      return vch.size() > 0 && GetLen(vch[0]) == vch.size();
+    }
+
     //! Construct an invalid public key.
     CPubKey()
     {
diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
@@ -64,17 +64,17 @@ static inline void popstack(std::vector<valtype>& stack)
 }
 
 bool static IsCompressedOrUncompressedPubKey(const valtype &vchPubKey) {
-    if (vchPubKey.size() < 33) {
+    if (vchPubKey.size() < CPubKey::COMPRESSED_PUBLIC_KEY_SIZE) {
         //  Non-canonical public key: too short
         return false;
     }
     if (vchPubKey[0] == 0x04) {
-        if (vchPubKey.size() != 65) {
+        if (vchPubKey.size() != CPubKey::PUBLIC_KEY_SIZE) {
             //  Non-canonical public key: invalid length for uncompressed key
             return false;
         }
     } else if (vchPubKey[0] == 0x02 || vchPubKey[0] == 0x03) {
-        if (vchPubKey.size() != 33) {
+        if (vchPubKey.size() != CPubKey::COMPRESSED_PUBLIC_KEY_SIZE) {
             //  Non-canonical public key: invalid length for compressed key
             return false;
         }
diff --git a/src/script/standard.cpp b/src/script/standard.cpp
@@ -124,7 +124,7 @@ bool Solver(const CScript& scriptPubKey, txnouttype& typeRet, std::vector<std::v
             // Template matching opcodes:
             if (opcode2 == OP_PUBKEYS)
             {
-                while (vch1.size() >= 33 && vch1.size() <= 65)
+                while (CPubKey::ValidSize(vch1))
                 {
                     vSolutionsRet.push_back(vch1);
                     if (!script1.GetOp(pc1, opcode1, vch1))
@@ -138,7 +138,7 @@ bool Solver(const CScript& scriptPubKey, txnouttype& typeRet, std::vector<std::v
 
             if (opcode2 == OP_PUBKEY)
             {
-                if (vch1.size() < 33 || vch1.size() > 65)
+                if (!CPubKey::ValidSize(vch1))
                     break;
                 vSolutionsRet.push_back(vch1);
             }

Original file line number	Diff line number	Diff line change
`@@ -69,6 +69,11 @@ class CPubKey`
`69`	`69`	`}`
`70`	`70`
`71`	`71`	`public:`
	`72`	`+`
	`73`	`+ bool static ValidSize(const std::vector<unsigned char> &vch) {`
	`74`	`+ return vch.size() > 0 && GetLen(vch[0]) == vch.size();`
	`75`	`+ }`
	`76`	`+`
`72`	`77`	`//! Construct an invalid public key.`
`73`	`78`	`CPubKey()`
`74`	`79`	`{`
Original file line number	Diff line number	Diff line change
`@@ -64,17 +64,17 @@ static inline void popstack(std::vector<valtype>& stack)`
`64`	`64`	`}`
`65`	`65`
`66`	`66`	`bool static IsCompressedOrUncompressedPubKey(const valtype &vchPubKey) {`
`67`		`- if (vchPubKey.size() < 33) {`
	`67`	`+ if (vchPubKey.size() < CPubKey::COMPRESSED_PUBLIC_KEY_SIZE) {`
`68`	`68`	`// Non-canonical public key: too short`
`69`	`69`	`return false;`
`70`	`70`	`}`
`71`	`71`	`if (vchPubKey[0] == 0x04) {`
`72`		`- if (vchPubKey.size() != 65) {`
	`72`	`+ if (vchPubKey.size() != CPubKey::PUBLIC_KEY_SIZE) {`
`73`	`73`	`// Non-canonical public key: invalid length for uncompressed key`
`74`	`74`	`return false;`
`75`	`75`	`}`
`76`	`76`	`} else if (vchPubKey[0] == 0x02 \|\| vchPubKey[0] == 0x03) {`
`77`		`- if (vchPubKey.size() != 33) {`
	`77`	`+ if (vchPubKey.size() != CPubKey::COMPRESSED_PUBLIC_KEY_SIZE) {`
`78`	`78`	`// Non-canonical public key: invalid length for compressed key`
`79`	`79`	`return false;`
`80`	`80`	`}`
Original file line number	Diff line number	Diff line change
`@@ -124,7 +124,7 @@ bool Solver(const CScript& scriptPubKey, txnouttype& typeRet, std::vector<std::v`
`124`	`124`	`// Template matching opcodes:`
`125`	`125`	`if (opcode2 == OP_PUBKEYS)`
`126`	`126`	`{`
`127`		`- while (vch1.size() >= 33 && vch1.size() <= 65)`
	`127`	`+ while (CPubKey::ValidSize(vch1))`
`128`	`128`	`{`
`129`	`129`	`vSolutionsRet.push_back(vch1);`
`130`	`130`	`if (!script1.GetOp(pc1, opcode1, vch1))`
`@@ -138,7 +138,7 @@ bool Solver(const CScript& scriptPubKey, txnouttype& typeRet, std::vector<std::v`
`138`	`138`
`139`	`139`	`if (opcode2 == OP_PUBKEY)`
`140`	`140`	`{`
`141`		`- if (vch1.size() < 33 \|\| vch1.size() > 65)`
	`141`	`+ if (!CPubKey::ValidSize(vch1))`
`142`	`142`	`break;`
`143`	`143`	`vSolutionsRet.push_back(vch1);`
`144`	`144`	`}`