From 291895fac725375790e681e7befdfd8489ce97bd Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Mon, 30 Jul 2018 22:35:31 -0600 Subject: [PATCH] Ensure sum of valueBalance and all vpub_new's does not exceed MAX_MONEY inside of CheckTransactionWithoutProofVerification. --- src/gtest/test_checktransaction.cpp | 54 +++++++++++++++++++++++++++++ src/main.cpp | 12 ++++++- 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/src/gtest/test_checktransaction.cpp b/src/gtest/test_checktransaction.cpp index c2a412bd2..f2552c0ed 100644 --- a/src/gtest/test_checktransaction.cpp +++ b/src/gtest/test_checktransaction.cpp @@ -710,6 +710,60 @@ class UNSAFE_CTransaction : public CTransaction { UNSAFE_CTransaction(const CMutableTransaction &tx) : CTransaction(tx, true) {} }; +extern ZCJoinSplit* params; + +TEST(checktransaction_tests, SaplingSproutInputSumsTooLarge) { + CMutableTransaction mtx = GetValidTransaction(); + mtx.vjoinsplit.resize(0); + mtx.fOverwintered = true; + mtx.nVersion = SAPLING_TX_VERSION; + mtx.nVersionGroupId = SAPLING_VERSION_GROUP_ID; + mtx.nExpiryHeight = 0; + + { + // create JSDescription + uint256 rt; + uint256 joinSplitPubKey; + std::array inputs = { + libzcash::JSInput(), + libzcash::JSInput() + }; + std::array outputs = { + libzcash::JSOutput(), + libzcash::JSOutput() + }; + std::array inputMap; + std::array outputMap; + + auto jsdesc = JSDescription::Randomized( + true, + *params, joinSplitPubKey, rt, + inputs, outputs, + inputMap, outputMap, + 0, 0, false); + + mtx.vjoinsplit.push_back(jsdesc); + } + + mtx.vShieldedSpend.push_back(SpendDescription()); + + mtx.vjoinsplit[0].vpub_new = (MAX_MONEY / 2) + 10; + + { + UNSAFE_CTransaction tx(mtx); + CValidationState state; + EXPECT_TRUE(CheckTransactionWithoutProofVerification(tx, state)); + } + + mtx.valueBalance = (MAX_MONEY / 2) + 10; + + { + UNSAFE_CTransaction tx(mtx); + MockCValidationState state; + EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-txintotal-toolarge", false)).Times(1); + CheckTransactionWithoutProofVerification(tx, state); + } +} // Test bad Overwinter version number in CheckTransactionWithoutProofVerification TEST(checktransaction_tests, OverwinterVersionNumberLow) { diff --git a/src/main.cpp b/src/main.cpp index 238612a20..bef466076 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1223,8 +1223,18 @@ bool CheckTransactionWithoutProofVerification(const CTransaction& tx, CValidatio REJECT_INVALID, "bad-txns-txintotal-toolarge"); } } - } + // Also check for Sapling + if (tx.valueBalance >= 0) { + // NB: positive valueBalance "adds" money to the transparent value pool, just as inputs do + nValueIn += tx.valueBalance; + + if (!MoneyRange(nValueIn)) { + return state.DoS(100, error("CheckTransaction(): txin total out of range"), + REJECT_INVALID, "bad-txns-txintotal-toolarge"); + } + } + } // Check for duplicate inputs set vInOutPoints;