Remove in-band error signalling from SignatureHash, fixing the SIGHAS…

…H_SINGLE bug.
eiabea · Jul 19, 2016 · 67f0243 · 67f0243
1 parent 85cc6f5
commit 67f0243
Show file tree

Hide file tree

Showing 6 changed files with 19 additions and 20 deletions.
diff --git a/src/main.cpp b/src/main.cpp
@@ -999,12 +999,12 @@ bool CheckTransactionWithoutProofVerification(const CTransaction& tx, CValidatio
                                  REJECT_INVALID, "bad-txns-prevout-null");
 
         if (tx.vjoinsplit.size() > 0) {
-            // TODO: #966.
-            static const uint256 one(uint256S("0000000000000000000000000000000000000000000000000000000000000001"));
             // Empty output script.
             CScript scriptCode;
-            uint256 dataToBeSigned = SignatureHash(scriptCode, tx, NOT_AN_INPUT, SIGHASH_ALL);
-            if (dataToBeSigned == one) {
+            uint256 dataToBeSigned;
+            try {
+                dataToBeSigned = SignatureHash(scriptCode, tx, NOT_AN_INPUT, SIGHASH_ALL);
+            } catch (std::logic_error ex) {
                 return state.DoS(100, error("CheckTransaction(): error computing signature hash"),
                                  REJECT_INVALID, "error-computing-signature-hash");
             }

diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
@@ -1095,17 +1095,16 @@ class CTransactionSignatureSerializer {
 
 uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType)
 {
-    static const uint256 one(uint256S("0000000000000000000000000000000000000000000000000000000000000001"));
     if (nIn >= txTo.vin.size() && nIn != NOT_AN_INPUT) {
         //  nIn out of range
-        return one;
+        throw logic_error("input index is out of range");
     }
 
     // Check for invalid use of SIGHASH_SINGLE
     if ((nHashType & 0x1f) == SIGHASH_SINGLE) {
         if (nIn >= txTo.vout.size()) {
             //  nOut out of range
-            return one;
+            throw logic_error("no matching output for SIGHASH_SINGLE");
         }
     }
 
@@ -1136,7 +1135,12 @@ bool TransactionSignatureChecker::CheckSig(const vector<unsigned char>& vchSigIn
     int nHashType = vchSig.back();
     vchSig.pop_back();
 
-    uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType);
+    uint256 sighash;
+    try {
+        sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType);
+    } catch (logic_error ex) {
+        return false;
+    }
 
     if (!VerifySignature(vchSig, pubkey, sighash))
         return false;

diff --git a/src/script/sign.cpp b/src/script/sign.cpp
@@ -25,7 +25,13 @@ bool TransactionSignatureCreator::CreateSig(std::vector<unsigned char>& vchSig,
     if (!keystore->GetKey(address, key))
         return false;
 
-    uint256 hash = SignatureHash(scriptCode, *txTo, nIn, nHashType);
+    uint256 hash;
+    try {
+        hash = SignatureHash(scriptCode, *txTo, nIn, nHashType);
+    } catch (logic_error ex) {
+        return false;
+    }
+
     if (!key.Sign(hash, vchSig))
         return false;
     vchSig.push_back((unsigned char)nHashType);

diff --git a/src/test/sighash_tests.cpp b/src/test/sighash_tests.cpp
@@ -146,13 +146,10 @@ void static RandomTransaction(CMutableTransaction &tx, bool fSingle) {
         unsigned char joinSplitPrivKey[crypto_sign_SECRETKEYBYTES];
         crypto_sign_keypair(tx.joinSplitPubKey.begin(), joinSplitPrivKey);
 
-        // TODO: #966.
-        static const uint256 one(uint256S("0000000000000000000000000000000000000000000000000000000000000001"));
         // Empty output script.
         CScript scriptCode;
         CTransaction signTx(tx);
         uint256 dataToBeSigned = SignatureHash(scriptCode, signTx, NOT_AN_INPUT, SIGHASH_ALL);
-        BOOST_CHECK(dataToBeSigned != one);
 
         assert(crypto_sign_detached(&tx.joinSplitSig[0], NULL,
                                     dataToBeSigned.begin(), 32,

diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp
@@ -402,13 +402,10 @@ BOOST_AUTO_TEST_CASE(test_simple_joinsplit_invalidity)
         BOOST_CHECK(!CheckTransactionWithoutProofVerification(newTx, state));
         BOOST_CHECK(state.GetRejectReason() == "bad-txns-invalid-joinsplit-signature");
 
-        // TODO: #966.
-        static const uint256 one(uint256S("0000000000000000000000000000000000000000000000000000000000000001"));
         // Empty output script.
         CScript scriptCode;
         CTransaction signTx(newTx);
         uint256 dataToBeSigned = SignatureHash(scriptCode, signTx, NOT_AN_INPUT, SIGHASH_ALL);
-        BOOST_CHECK(dataToBeSigned != one);
 
         assert(crypto_sign_detached(&newTx.joinSplitSig[0], NULL,
                                     dataToBeSigned.begin(), 32,

diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
@@ -2656,15 +2656,10 @@ Value zc_raw_joinsplit(const json_spirit::Array& params, bool fHelp)
 
     mtx.vjoinsplit.push_back(jsdesc);
 
-    // TODO: #966.
-    static const uint256 one(uint256S("0000000000000000000000000000000000000000000000000000000000000001"));
     // Empty output script.
     CScript scriptCode;
     CTransaction signTx(mtx);
     uint256 dataToBeSigned = SignatureHash(scriptCode, signTx, NOT_AN_INPUT, SIGHASH_ALL);
-    if (dataToBeSigned == one) {
-        throw runtime_error("SignatureHash failed");
-    }
 
     // Add the signature
     assert(crypto_sign_detached(&mtx.joinSplitSig[0], NULL,