Merge #863: Fix HasValidFee potential overflow

stevenroose · stevenroose · commit 18fadbafccef · 2020-04-16T17:51:43.000+01:00
98e42a0 Fix HasValidFee potential overflow (Steven Roose) Pull request description: Dmitry pointed out this potential overflow. They can't really happen because of the `CheckTransaction` check on explicit amounts that happens earlier in the verification chain. But it's a good idea to add the check here as well so that a potential relaxing of other rules cannot accidentally introduce an overflow risk. Tree-SHA512: 0c6abb7719d4cf84596da5cb31e700bee53d26d6ffc6da0ba8f1300b3357ca3d29fb46e2754d182e64cb5794fd58fa1efed5ebf9c1bd58d08e2fb7841725ca66
diff --git a/src/confidential_validation.cpp b/src/confidential_validation.cpp
@@ -33,9 +33,12 @@ bool HasValidFee(const CTransaction& tx) {
             if (fee == 0 || !MoneyRange(fee))
                 return false;
             totalFee[tx.vout[i].nAsset.GetAsset()] += fee;
+            if (!MoneyRange(totalFee)) {
+                return false;
+            }
         }
     }
-    return MoneyRange(totalFee);
+    return true;
 }
 
 CAmountMap GetFeeMap(const CTransaction& tx) {
diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp
@@ -306,6 +306,9 @@ bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoins
             return state.DoS(100, false, REJECT_INVALID, "bad-txns-in-ne-out", false, "value in != value out");
         }
         fee_map += GetFeeMap(tx);
+        if (!MoneyRange(fee_map)) {
+            return state.DoS(100, false, REJECT_INVALID, "bad-block-total-fee-outofrange");
+        }
     } else {
         const CAmount value_out = tx.GetValueOutMap()[CAsset()];
         if (nValueIn < value_out) {

Original file line number	Diff line number	Diff line change
`@@ -33,9 +33,12 @@ bool HasValidFee(const CTransaction& tx) {`
`33`	`33`	`if (fee == 0 \|\| !MoneyRange(fee))`
`34`	`34`	`return false;`
`35`	`35`	`totalFee[tx.vout[i].nAsset.GetAsset()] += fee;`
	`36`	`+ if (!MoneyRange(totalFee)) {`
	`37`	`+ return false;`
	`38`	`+ }`
`36`	`39`	`}`
`37`	`40`	`}`
`38`		`- return MoneyRange(totalFee);`
	`41`	`+ return true;`
`39`	`42`	`}`
`40`	`43`
`41`	`44`	`CAmountMap GetFeeMap(const CTransaction& tx) {`