Fix crash bug with duplicate inputs within a transaction #14247

TheBlueMatt · 2018-09-17T21:57:04Z

No description provided.

Introduced by bitcoin#9049

laanwj · 2018-09-17T22:10:03Z

tested ACK 9b4a36e

jamesob · 2018-09-17T22:10:31Z

utACK 9b4a36e

sdaftuar · 2018-09-17T22:18:12Z

ACK

maflcko · 2018-09-17T22:34:15Z

utACK 9b4a36e. Checked that the test fails without the fix and passes with the fix.

ryanofsky

Slightly tested ACK 9b4a36e, just ran python test with & without fix.

ryanofsky · 2018-09-17T22:52:15Z

src/validation.cpp

    // Check transactions
    for (const auto& tx : block.vtx)
-        if (!CheckTransaction(*tx, state, false))
+        if (!CheckTransaction(*tx, state, true))


~~Just noting it looks like this argument can be removed entirely in a future cleanup PR.~~

Update: Removing the argument was attempted in #14258, but actually turns out not to be a good idea because the optimization will probably be reintroduced in the future (#14258 (comment)).

gmaxwell · 2018-09-17T22:59:48Z

ACK

…nsaction 0d49c82 [qa] backport: Test for duplicate inputs within a transaction (Suhas Daftuar) 833180f Fix crash bug with duplicate inputs within a transaction (Suhas Daftuar) Pull request description: This is a backport of #14247. Tree-SHA512: 4d3b6244d501a48f56a728c571dac9a346019a671434edac943f4f535ef8f94ec6bfd569a0585ad5e23a6e488ecd7e0079510cbb10a2a22f576eb36d73accb0c

9b4a36e [qa] Test for duplicate inputs within a transaction (Suhas Daftuar) b8f8019 Fix crash bug with duplicate inputs within a transaction (Suhas Daftuar) Pull request description: Tree-SHA512: 8c7ea34c7fa44188d86c04a690a7cbf8e9deda71ab1f7ca6d11de1f2abb3dd7222627071f86d0d39689a8b302ba9af142f0202466a67e30cd54aed3a08d4eb14

…nsaction 9bd08fd [qa] backport: Test for duplicate inputs within a transaction (Suhas Daftuar) d1dee20 Fix crash bug with duplicate inputs within a transaction (Suhas Daftuar) Pull request description: This is a backport of #14247 to 0.16. Tree-SHA512: f11b2b0f2d8089bbac7542f78a0f14fc15c693604cb1168ef5ea71852a206da7eb53b6e420376ed1380583961176ba2d283e409e19d783c7a68c3407933a89b0

Github-Pull: #14247 Tree-SHA512: fa0b11e245b66a9334ca3aa8c99d541f5835e5c60d372c852fc44a0ee8b930112e967ade535a3ca0a51bf40c8e9becfb5aeab3807abaddd7afb259cfa3844b32

Github-Pull: #14247 Tree-SHA512: 149dfe70406c8e013fd3e1ffad61dd1b4980a630f4f752a3520d366dd9b3848125dc0865eef80c8e04388310172d6a92e4414fa90059b2e14aa77a2c6d89d83d

Introduced by #9049 Github-Pull: #14247 Tree-SHA512: 54ccf896e4c816ba8532644affc984a091ed801d8387bb01a836953c9ec4a345359d98fb58dd5f929617afd42bce0cc40293fecf943a1584207c82dd78da0ea5

Introduced by #9049 Github-Pull: #14247 Tree-SHA512: 2815312a3da8ef4a93dbc26b71d9147e34d1fed794aa7188ec12670579d2e45380212cf1e23526a7b5e339a185a73637fc2f342e0699b687c920244bc2edc124

practicalswift · 2018-09-18T15:21:07Z

utACK 9b4a36e

Very nice find @sdaftuar! How did you find this issue?

luke-jr · 2018-09-18T18:09:18Z

For reference, this issue was assigned CVE-2018-17144

ftrader · 2018-09-18T18:30:44Z

Could @TheBlueMatt / @sdaftuar outline the responsible disclosure procedures, if any, that were attempted to other projects before publishing these pull requests?

TheBlueMatt · 2018-09-18T19:45:15Z

The bug was disclosed to other projects simultaneously to it being disclosed to us. We tried to coordinate timelines for release but sadly didn't want to wait much past EST business hours to get people patched so ran out of time.

zquestz · 2018-09-18T20:19:52Z

Where can I find a detailed description of the impact of this bug? When is the CVE getting published so I can get more clarity on the exact impact? I am seeing all sorts of reports on Twitter and would like to get an accurate assessment of the issue.

PierreRochard · 2018-09-18T21:31:34Z

Where can I find a detailed description of the impact of this bug? When is the CVE getting published so I can get more clarity on the exact impact? I am seeing all sorts of reports on Twitter and would like to get an accurate assessment of the issue.

Release notes: https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-0.16.3.md#denial-of-service-vulnerability

More color in today's OpTech newsletter ( https://mailchi.mp/cc96f82565eb/bitcoin-optech-newsletter-13?e=cbe2b70569 ):

Upgrade to Bitcoin Core 0.16.3 to fix denial-of-service vulnerability: a bug introduced in Bitcoin Core 0.14.0 and affecting all subsequent versions through to 0.16.2 will cause Bitcoin Core to crash when attempting to validate a block containing a transaction that attempts to spend the same input twice. Such blocks would be invalid and so can only be created by miners willing to lose the allowed income from having created a block (at least 12.5 XBT or $80,000 USD).

Patches for master and 0.16 branches were submitted for public review yesterday, the 0.16.3 release has been tagged containing the patch, and binaries will be available for download as soon as a sufficient number of well-known contributors have reproduced the deterministic build—probably later today (Tuesday). Immediate upgrade is highly recommended.

Introduced by bitcoin#9049 Github-Pull: bitcoin#14247 Tree-SHA512: 54ccf896e4c816ba8532644affc984a091ed801d8387bb01a836953c9ec4a345359d98fb58dd5f929617afd42bce0cc40293fecf943a1584207c82dd78da0ea5

deadalnix · 2018-09-19T00:23:53Z

We tried to coordinate timelines for release [...]

Tell us more about this.

sickpig · 2018-09-19T07:46:33Z

We tried to coordinate timelines for release but sadly didn't want to wait much past EST business hours to get people patched so ran out of time.

I was among the people who received the anonymous report, even though BU was not affected, and I'm not aware of any kind of effort to "coordinate timelines".

As far as I can tell the list of people to which the bug was disclosed was not exhaustive, not by a long shot, so make sure to have a proper way to tell projects/devs that their code is at risk is a must IMHO.

It would be great to know what were the actions taken to put in places the aforementioned coordination process. The aim is to improve the process in case of new disclosures that could happen in the future.

Sjors · 2018-09-19T08:57:28Z

Post merge slightly tested 9b4a36e, ran python test without fix which produced the assert.

biblepay · 2018-09-19T09:44:48Z

Thanks.

- Fixed a bug, that can cause a node to crash if a block includes duplicate transactions. (bitcoin#14247)

Merged the following PR from bitcoin repository: bitcoin/bitcoin#14247 . Merged the actual fix only, the functional test is not yet merged in: bitcoin: Merge #14247: Fix crash bug with duplicate inputs within a transaction b8f801964f59586508ea8da6cf3decd76bc0e571 Fix crash bug with duplicate inputs within a transaction (Suhas Daftuar) Pull request description: Tree-SHA512: 8c7ea34c7fa44188d86c04a690a7cbf8e9deda71ab1f7ca6d11de1f2abb3dd7222627071f86d0d39689a8b302ba9af142f0202466a67e30cd54aed3a08d4eb14

bitcoin/bitcoin#14247

Backports bitcoin/bitcoin#14247

practicalswift · 2018-09-20T14:11:54Z

I would like to thank the researcher who invested time into finding this issue. Impressive finding! Thanks!

isghe · 2018-09-20T21:44:32Z

Is this bug reproducible in regtest or testnet? Thanks

Sjors · 2018-09-21T10:27:26Z

@isghe the bug is reproducible on regtest using the Python test from this PR. You need to change CheckTransaction(*tx, state, true) back to CheckTransaction(*tx, state, false) and then run test/functional/p2p_invalid_block.py. The test contains the kind of invalid block that can crash an (unpatched) node. You'll see a crash caused by an assert.

CVE-2018-17144 Full Disclosure, which includes the inflation bug not mentioned above: https://bitcoincore.org/en/2018/09/20/notice/

I assume we'll add a test case for the inflation bug once the dust has settled?

promag · 2018-09-21T10:47:43Z

I assume we'll add a test case for the inflation bug once the dust has settled?

@gmaxwell already suggested it, also agree.

ftrader · 2018-09-22T04:04:38Z

The discoverer of CVE-2018-17144 has come forward (for those wishing to express their gratitude, there is an included donation address in the post):

https://medium.com/@awemany/600-microseconds-b70f87b0b2a6

Fix from bitcoin#14247

Fix CVE-2018-17144 bitcoin/bitcoin#14247

Follow bitcoin possible issue bitcoin/bitcoin#14247 Even if there were other tests prevening that in code at several levels (mempool)

Original Bitcoin Fix: bitcoin/bitcoin#14247

isghe · 2018-09-26T23:18:32Z

It looks they created with success 0.1 tBTC from nothing, exploiting this bug in testnet3 block 1414411, hash 00000000eba3f43a8624750f39e4520a1678c0dbdf8707bfa4854a12fbf086c5

sdaftuar added 2 commits September 17, 2018 15:50

Fix crash bug with duplicate inputs within a transaction

b8f8019

Introduced by bitcoin#9049

[qa] Test for duplicate inputs within a transaction

9b4a36e

This was referenced Sep 17, 2018

[0.17] Fix crash bug with duplicate inputs within a transaction #14248

Merged

[0.16] Fix crash bug with duplicate inputs within a transaction #14249

Merged

laanwj added the Consensus label Sep 17, 2018

ryanofsky reviewed Sep 17, 2018

View reviewed changes

laanwj merged commit 9b4a36e into bitcoin:master Sep 17, 2018

dagurval mentioned this pull request Sep 18, 2018

[qa] Test for duplicate inputs within a transaction bitcoinxt/bitcoinxt#486

Merged

instagibbs mentioned this pull request Sep 18, 2018

we do check for duplicate inputs in CheckBlock #14257

Closed

UdjinM6 mentioned this pull request Sep 18, 2018

Fix crash bug with duplicate inputs within a transaction dashpay/dash#2302

Merged

cryptapus mentioned this pull request Sep 18, 2018

Myriadcoin 0.14.3.0 myriadteam/myriadcoin#110

Merged

globaltoken added a commit to globaltoken/globaltoken that referenced this pull request Sep 19, 2018

Fixed important Bug, that occured from BTC Core 0.14 - 0.16

5a201f2

- Fixed a bug, that can cause a node to crash if a block includes duplicate transactions. (bitcoin#14247)

dgpv mentioned this pull request Sep 19, 2018

Is Litecoin affected by CVE-2018-17144 ? litecoin-project/litecoin#537

Closed

h4x3rotab mentioned this pull request Sep 19, 2018

Fix CVE-2018-17144 BTCGPU/BTCGPU#351

Merged

dartdart26 mentioned this pull request Sep 19, 2018

Fix crash bug with duplicate inputs within a transaction (merge from bitcoin) projectpai/paicoin#96

Merged

lateminer pushed a commit to lateminer/bitcoin-abc that referenced this pull request Sep 19, 2018

Fix crash bug with duplicate inputs within a transaction

f8062c3

bitcoin/bitcoin#14247

sidhujag added a commit to syscoin/syscoin that referenced this pull request Sep 19, 2018

ix crash bug with duplicate inputs within a transaction bitcoin#14247

8c462d0

jtimon mentioned this pull request Sep 19, 2018

[backport] Fix crash bug with duplicate inputs within a transaction ElementsProject/elements#413

Merged

expocash added a commit to expocash/expocash that referenced this pull request Sep 20, 2018

CheckBlock() must validate duplicate inputs within a tx

0c37829

Backports bitcoin/bitcoin#14247

cryptomeow pushed a commit to cryptomeow/bitcoinfibre that referenced this pull request Sep 22, 2018

Reinstate duplicate inputs check within a transaction

dffb8c2

Fix from bitcoin#14247

cryptomeow mentioned this pull request Sep 22, 2018

Reinstate duplicate inputs check within a transaction bitcoinfibre/bitcoinfibre#11

Closed

litebitcoins pushed a commit to litebitcoins/litebitcoin that referenced this pull request Sep 23, 2018

Fix crash bug with duplicate inputs within a transaction

4cc80c8

Fix CVE-2018-17144 bitcoin/bitcoin#14247

tpruvot added a commit to LUX-Core/lux that referenced this pull request Sep 23, 2018

CVE-2018-17144: ensure tx dups are double checked

954c382

Follow bitcoin possible issue bitcoin/bitcoin#14247 Even if there were other tests prevening that in code at several levels (mempool)

Stouse49 added a commit to goldcoin/goldcoin that referenced this pull request Sep 24, 2018

Fix crash bug with duplicate inputs within a transaction

947f668

Original Bitcoin Fix: bitcoin/bitcoin#14247

leafcutterant mentioned this pull request Sep 24, 2018

Is ElectrumX affected by the Bitcoin Core DDoS + inflation bug? kyuupichan/electrumx#606

Closed

isghe mentioned this pull request Sep 25, 2018

Are v0.17rc1-v0.17rc3 affected by CVE-2018-17144? #14323

Closed

bitcoin locked and limited conversation to collaborators Sep 26, 2018

Fix crash bug with duplicate inputs within a transaction #14247

Fix crash bug with duplicate inputs within a transaction #14247

Uh oh!

Conversation

TheBlueMatt commented Sep 17, 2018

Uh oh!

laanwj commented Sep 17, 2018

Uh oh!

jamesob commented Sep 17, 2018

Uh oh!

sdaftuar commented Sep 17, 2018

Uh oh!

maflcko commented Sep 17, 2018

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

ryanofsky Sep 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gmaxwell commented Sep 17, 2018

Uh oh!

practicalswift commented Sep 18, 2018

Uh oh!

luke-jr commented Sep 18, 2018

Uh oh!

ftrader commented Sep 18, 2018

Uh oh!

TheBlueMatt commented Sep 18, 2018

Uh oh!

zquestz commented Sep 18, 2018

Uh oh!

PierreRochard commented Sep 18, 2018

Uh oh!

deadalnix commented Sep 19, 2018

Uh oh!

sickpig commented Sep 19, 2018

Uh oh!

Sjors commented Sep 19, 2018

Uh oh!

biblepay commented Sep 19, 2018

Uh oh!

practicalswift commented Sep 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

isghe commented Sep 20, 2018

Uh oh!

Sjors commented Sep 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

promag commented Sep 21, 2018

Uh oh!

ftrader commented Sep 22, 2018

Uh oh!

isghe commented Sep 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

18 participants

ryanofsky Sep 17, 2018 •

edited

Loading

practicalswift commented Sep 20, 2018 •

edited

Loading

Sjors commented Sep 21, 2018 •

edited

Loading

isghe commented Sep 26, 2018 •

edited

Loading