Add header-sensitive size checking via CPubKey::ValidSize, and prefer key/sig size const references #12319

 171 | @@ -164,7 +172,7 @@ class CPubKey
 172 |       */
 173 |      bool IsValid() const
 174 |      {
 175 | -        return size() > 0;
 176 | +        return size() >= COMPRESSED_SIZE && size() <= SIZE;

devrandom commented at 5:35 PM on February 3, 2018:

I would leave the original code on this line, since size() (and GetLen()) specifically returns 0 to signal an invalid header byte.

Keep in mind that size() can only return three potential values (zero or one of the two sizes), so any size in between will never be returned.

Empact commented at 10:23 AM on February 4, 2018:

Roger! Thanks for pointing that out.

  60 | @@ -61,17 +61,17 @@ static inline void popstack(std::vector<valtype>& stack)
  61 |  }
  62 |  
  63 |  bool static IsCompressedOrUncompressedPubKey(const valtype &vchPubKey) {

devrandom commented at 5:37 PM on February 3, 2018:

Might be good to move these two static methods under CPubKey, but maybe that's a job for another PR.

Empact commented at 11:49 AM on February 4, 2018:

Make sense! This commit moves these methods: https://github.com/Empact/bitcoin/commit/f3c5b2d1c6b0348b5751c15cffdd1238209ea463

I'll plan to submit it once this is merged.

  34 | @@ -35,6 +35,10 @@ const char* GetTxnOutputType(txnouttype t)
  35 |      return nullptr;
  36 |  }
  37 |  
  38 | +static constexpr bool ValidPubKeySize(std::vector<unsigned char>::size_type size) {

devrandom commented at 5:38 PM on February 3, 2018:

Might be good to have this under CPubKey

  55 |       */
  56 |      static_assert(
  57 | -        PUBLIC_KEY_SIZE >= COMPRESSED_PUBLIC_KEY_SIZE,
  58 | -        "COMPRESSED_PUBLIC_KEY_SIZE is larger than PUBLIC_KEY_SIZE");
  59 | +        SIZE >= COMPRESSED_SIZE,
  60 | +        "COMPRESSED_SIZE is larger than SIZE");

devrandom commented at 5:40 PM on February 3, 2018:

maybe reword to "COMPRESSED_SIZE must be smaller than SIZE"

  40 |       */
  41 |      static_assert(
  42 | -        PRIVATE_KEY_SIZE >= COMPRESSED_PRIVATE_KEY_SIZE,
  43 | -        "COMPRESSED_PRIVATE_KEY_SIZE is larger than PRIVATE_KEY_SIZE");
  44 | +        SIZE >= COMPRESSED_SIZE,
  45 | +        "COMPRESSED_SIZE is larger than SIZE");

devrandom commented at 5:41 PM on February 3, 2018:

maybe reword