Fix crash bug with duplicate inputs within a transaction #14247

pull TheBlueMatt wants to merge 2 commits into bitcoin:master from TheBlueMatt:2018-09-checktransaction changing 2 files +11 −1
  1. TheBlueMatt commented at 9:57 PM on September 17, 2018: contributor

    No description provided.

  2. Fix crash bug with duplicate inputs within a transaction
    Introduced by #9049
    b8f801964f
  3. [qa] Test for duplicate inputs within a transaction 9b4a36effc
  4. TheBlueMatt cross-referenced this on Sep 17, 2018 from issue [0.17] Fix crash bug with duplicate inputs within a transaction by TheBlueMatt
  5. TheBlueMatt cross-referenced this on Sep 17, 2018 from issue [0.16] Fix crash bug with duplicate inputs within a transaction by TheBlueMatt
  6. laanwj commented at 10:10 PM on September 17, 2018: member

    tested ACK 9b4a36effcf642f3844c6696b757266686ece11a

  8. sdaftuar commented at 10:18 PM on September 17, 2018: member

    ACK

  9. laanwj added the label Consensus on Sep 17, 2018
  10. MarcoFalke commented at 10:34 PM on September 17, 2018: member

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

  11. in src/validation.cpp:3125 in 9b4a36effc 
    3121 | @@ -3122,7 +3122,7 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P
3122 |  
3123 |      // Check transactions
3124 |      for (const auto& tx : block.vtx)
3125 | -        if (!CheckTransaction(*tx, state, false))
3126 | +        if (!CheckTransaction(*tx, state, true))
    ryanofsky commented at 10:52 PM on September 17, 2018:

    ~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 (https://github.com/bitcoin/bitcoin/pull/14258#issuecomment-422795765).

  12. ryanofsky commented at 10:55 PM on September 17, 2018: contributor

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

  13. gmaxwell commented at 10:59 PM on September 17, 2018: contributor

    ACK

  14. laanwj referenced this in commit c64128df58 on Sep 17, 2018
  15. laanwj merged this on Sep 17, 2018
  16. laanwj closed this on Sep 17, 2018
  17. laanwj referenced this in commit d926a87fde on Sep 17, 2018
  18. laanwj referenced this in commit 696b936aa3 on Sep 17, 2018
  19. dagurval cross-referenced this on Sep 18, 2018 from issue [qa] Test for duplicate inputs within a transaction by dagurval
  20. laanwj referenced this in commit a765d575dd on Sep 18, 2018
  21. laanwj referenced this in commit f37124f2a7 on Sep 18, 2018
  22. laanwj referenced this in commit 71c5900381 on Sep 18, 2018
  23. laanwj referenced this in commit 52965fbaef on Sep 18, 2018
  24. laanwj referenced this in commit 4b8a3f5d23 on Sep 18, 2018
  25. practicalswift commented at 3:21 PM on September 18, 2018: contributor

    utACK 9b4a36effcf642f3844c6696b757266686ece11a

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

  26. instagibbs cross-referenced this on Sep 18, 2018 from issue we do check for duplicate inputs in CheckBlock by instagibbs
  27. luke-jr commented at 6:09 PM on September 18, 2018: member

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

  28. ftrader commented at 6:30 PM on September 18, 2018: none

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

  29. TheBlueMatt commented at 7:45 PM on September 18, 2018: contributor

    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.

  30. zquestz commented at 8:19 PM on September 18, 2018: none

    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.

  31. PierreRochard commented at 9:31 PM on September 18, 2018: contributor

    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.

  32. UdjinM6 cross-referenced this on Sep 18, 2018 from issue Fix crash bug with duplicate inputs within a transaction by UdjinM6
  33. cryptapus referenced this in commit d018a9b73e on Sep 18, 2018
  34. cryptapus cross-referenced this on Sep 18, 2018 from issue Myriadcoin 0.14.3.0 by cryptapus
  35. deadalnix commented at 12:23 AM on September 19, 2018: none

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

    Tell us more about this.

  36. sickpig commented at 7:46 AM on September 19, 2018: none

    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.

  37. Sjors commented at 8:57 AM on September 19, 2018: member

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

  38. biblepay commented at 9:44 AM on September 19, 2018: none

    Thanks.

  39. globaltoken referenced this in commit 5a201f2af0 on Sep 19, 2018
  40. dgpv cross-referenced this on Sep 19, 2018 from issue Is Litecoin affected by CVE-2018-17144 ? by dgpv
  41. h4x3rotab cross-referenced this on Sep 19, 2018 from issue Fix CVE-2018-17144 by h4x3rotab
  42. dartdart26 referenced this in commit b1fa9eb160 on Sep 19, 2018
  43. dartdart26 cross-referenced this on Sep 19, 2018 from issue Fix crash bug with duplicate inputs within a transaction (merge from bitcoin) by dartdart26
  44. lateminer referenced this in commit f8062c3b57 on Sep 19, 2018
  45. sidhujag referenced this in commit 8c462d09fb on Sep 19, 2018
  46. jtimon cross-referenced this on Sep 19, 2018 from issue [backport] Fix crash bug with duplicate inputs within a transaction by instagibbs
  47. expocash referenced this in commit 0c37829493 on Sep 20, 2018
  48. practicalswift commented at 2:11 PM on September 20, 2018: contributor

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

  49. isghe commented at 9:44 PM on September 20, 2018: contributor

    Is this bug reproducible in regtest or testnet? Thanks

  50. Sjors commented at 10:27 AM on September 21, 2018: member

    @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?

  51. promag commented at 10:47 AM on September 21, 2018: member

    I assume we'll add a test case for the inflation bug once the dust has settled? @gmaxwell already suggested it, also agree.

  52. ftrader commented at 4:04 AM on September 22, 2018: none

    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

  53. cryptomeow referenced this in commit dffb8c270a on Sep 22, 2018
  54. cryptomeow cross-referenced this on Sep 22, 2018 from issue Reinstate duplicate inputs check within a transaction by cryptomeow
  55. litebitcoins referenced this in commit 4cc80c81cf on Sep 23, 2018
  56. tpruvot referenced this in commit 954c382f9a on Sep 23, 2018
  57. Stouse49 referenced this in commit 947f668d9b on Sep 24, 2018
  58. leafcutterant cross-referenced this on Sep 24, 2018 from issue Is ElectrumX affected by the Bitcoin Core DDoS + inflation bug? by leafcutterant
  59. isghe cross-referenced this on Sep 25, 2018 from issue Are v0.17rc1-v0.17rc3 affected by CVE-2018-17144? by isghe
  60. isghe commented at 11:18 PM on September 26, 2018: contributor

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

  61. bitcoin locked this on Sep 26, 2018
Contributors
TheBlueMattlaanwjjamesobsdaftuarMarcoFalkeryanofskygmaxwellpracticalswiftluke-jrftraderzquestzPierreRocharddeadalnixsickpigSjorsbiblepayisghepromag
Labels
Consensus
Linked (view graph)
#14248 [0.17] Fix crash bug with duplicate inputs within a transaction#14249 [0.16] Fix crash bug with duplicate inputs within a transaction#14257 we do check for duplicate inputs in CheckBlock#14258 Remove redundant parameter fCheckDuplicateInputs from CheckTransaction(...)#14323 Are v0.17rc1-v0.17rc3 affected by CVE-2018-17144?
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-20 06:54 UTC