Merge #9753: Add static_assert to prevent VARINT(<signed value>)

499d95e27 Add static_assert to prevent VARINT(<signed value>) (Russell Yanofsky)

Pull request description:

  Using VARINT with signed types is dangerous because negative values will appear to serialize correctly, but then deserialize as positive values mod 128.

  This commit changes the VARINT macro to trigger a compile error by default if called with an signed value, and it updates existing broken uses of VARINT to pass a special flag that lets them keep working with no changes in behavior.

  There is some discussion about this issue here: https://github.com/bitcoin/bitcoin/pull/9693#issuecomment-278701473. I think another good change along these lines would be to make `GetSizeOfVarInt` and `WriteVarInt` throw exceptions if they are passed numbers less than 0 to serialize. But unlike this change, that would be a change in runtime behavior, and need more consideration.

Tree-SHA512: 082c65598cfac6dc1da042bdb47dbc9d5d789fc849fe52921cc238578588f4e5ff976c8b4b2ce42cb75290eb14f3b42ea76e26202c223c5b2aa63ef45c2ea3cc

1 files changed, 3 insertions, 3 deletions

diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 2077723af5..3f66c0c536 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -834,18 +834,18 @@ static void ApplyStats(CCoinsStats &stats, CHashWriter& ss, const uint256& hash,
 {
     assert(!outputs.empty());
     ss << hash;
-    ss << VARINT(outputs.begin()->second.nHeight * 2 + outputs.begin()->second.fCoinBase);
+    ss << VARINT(outputs.begin()->second.nHeight * 2 + outputs.begin()->second.fCoinBase, VarIntMode::NONNEGATIVE_SIGNED);
     stats.nTransactions++;
     for (const auto output : outputs) {
         ss << VARINT(output.first + 1);
         ss << output.second.out.scriptPubKey;
-        ss << VARINT(output.second.out.nValue);
+        ss << VARINT(output.second.out.nValue, VarIntMode::NONNEGATIVE_SIGNED);
         stats.nTransactionOutputs++;
         stats.nTotalAmount += output.second.out.nValue;
         stats.nBogoSize += 32 /* txid */ + 4 /* vout index */ + 4 /* height + coinbase */ + 8 /* amount */ +
                            2 /* scriptPubKey len */ + output.second.out.scriptPubKey.size() /* scriptPubKey */;
     }
-    ss << VARINT(0);
+    ss << VARINT(0u);
 }
 
 //! Calculate statistics about the unspent transaction output set