<p>I think that's an incorrect impression. The code involved in CVE-2018-17144 was relatively recent (the UTXO handling logic was written in 0.8 (released feb 2013), and underwent a few small changes after, and a large improvement in 0.15). If by "original code" you refer to the code written by Satoshi, this issue is unrelated, as fairly little of that code is left.</p>

<p>The issue wasn't that the code was not understood. It's that it wasn't clear which piece of code was responsible for what.</p>

<p>For performance reasons, block validation is split up in multiple stages; some run when the block is first received, some run later when it's actually a candidate for becoming part of the best chain ("activation"). The general principle is that you try to make the work wasted on verifying an invalid block proportional to the work an attacker has to do. To accomplish that, you verify the things that are cheap to fake first, and things that are expensive to fake later.</p>

<p>One of those checks was whether there were double spends within a transaction. Initially, this check was done when the block was received directly, and a sanity check during activation. In 0.14, in a performance improvement, the check in the initial block was removed, as it became the critical path for validation of compact blocks. This was wrong; instead of removing the code, it should have been moved to activation time, but it appeared that the activation code was already doing this check. In 0.15, when the UTXO handling code was rewritten, this sanity check at activation time was removed as well, because it didn't appear to be actually part of the validation code.</p>

<p>The mistake in my view was that the split up of responsibility for this type of double-spending check was non-obvious and should have been documented. I was largely responsible for this. Several procedures are in place to help prevent issues of this magnitude, including a sometimes annoyingly slow review process, and functional tests (which do exercise double-spending, ... just not double-spending-within-one-transaction). In this case, those procedures failed.</p>

<p>The rest of my answer is my personal opinion, as I don't think I can speak for anyone else.</p>

<blockquote>
  <p>are there parts of the bitcoin source code that are 'untouchable'?</p>
</blockquote>

<p>No. Certainly some parts are more dangerous to modify, but everything is a question of cost vs. benefit. The changes in 0.15 were a very justified performance improvement, and got a lot of eyes, but not enough. For the future I think this means the bar for review, and tests, vs the possible gain needs to be revisited for changes touching this UTXO handling code.</p>

<blockquote>
  <p>How much magic is there in the source code that contributors need to just figure out exists?</p>
</blockquote>

<p>I don't think there is any magic. Certainly not all logic is equally easy to understand, but that is true for any software project.</p>

<blockquote>
  <p>If bitcoin broke, would the core team jump at the opportunity to recreate bitcoin </p>
</blockquote>

<p>My personal opinion is that rewriting something from scratch is orders of magnitude harder to do correctly than making small incremental changes that are individually-reviewable. Note that the bar for correctness in consensus code is extremely high: not only must it accept all valid blocks, it must also reject all invalid blocks. This sort of consistency between codebases is extremely hard to maintain. Most reimplementations of Bitcoin's consensus rules have had one or a few serious mistakes in their history, and I wouldn't be surprised if most of them still have not exactly the same logic in all cases.</p>

<blockquote>
  <p>bitcoin with all the new features (mast, tap, schnorr) etc.</p>
</blockquote>

<p>Thankfully, those things are very well-isolated changes with a simple interface to the rest of the system. Script validation is pretty much just a "call a function with the output and input without much of any other context, and return true or false". There are also obvious ways to introduce backward-compatible changes in it: simply add additional "return false" statements in it that don't modify any other variables.</p>

<blockquote>
  <p>Is describing the source code as having a cancerous centre a huge overstatement, or is it playfully accurate?</p>
</blockquote>

<p>I wouldn't know what that centre would be, so I think it's an overstatement.</p>

<blockquote>
  <p>Is the idea of current contributors working around the old code like walking on eggshells accurate?</p>
</blockquote>

<p>Touching consensus code is always walking on eggshells, but that doesn't mean it shouldn't or can't be done. I also don't think it would be any different if the code had a different history - consensus correctness is just a hard problem.</p>

CC BY-SA 4.0
2019-05-07T21:42:18.807Z
I think you also miss a bit of history here:  In the original code this kind of double spend was only caught in block construction/connection and so there was a bug where the mempool could could contain these transactions (which was mostly harmless except they could show up unconfirmed in the wallet).  This test was specifically added to protect the mempool and nothing else.  But its importance was undocumentedly (and unintentionally?) increased by later changes.
G. Maxwell
CC BY-SA 4.0
2019-05-07T21:44:35.913Z
The mistake here depended on the fact that people "knew" the history and original purpose of the code and didn't know that far away consensus rules had been weakened.I think it's informative to observe that "bitcoin-abc" rewrote double spend validation for their "ctor" change and managed to preserve the same bug and not detect it in the new test cases they added. The fact that double spending within a transaction isn't the same as general double spending is exactly the sort of cryptographic subtly that comes up and is fiendishly hard to get right.
G. Maxwell
CC BY-SA 4.0
2019-05-07T19:10:56.703Z
2019-05-07T22:51:29.570Z
2019-05-07T22:51:29.570Z
Pieter Wuille
<p>My understanding is that CVE-2018–17144 (inflation bug introduced in 0.15) was partially a result of how convoluted the original code is.</p>

<p>Regardless of the specific CVE-2018–17144 example, are there parts of the bitcoin source code that are 'untouchable'? How much magic is there in the source code that contributors need to just figure out exists? </p>

<p>If bitcoin broke, would the core team jump at the opportunity to recreate bitcoin with all the new features (mast, tap, schnorr) etc. without the 'cancerous centre'? Is describing the source code as having a cancerous centre a huge overstatement, or is it playfully accurate? Is the idea of current contributors working around the old code like walking on eggshells accurate?</p>

<bitcoin-core><security>
How much of the Bitcoin Source code is untouchable? [CVE-2018–17144]
Paul Kania
# How much of the Bitcoin Source code is untouchable? [CVE-2018–17144]

(Asked by: username_0 on 2019-05-07)

My understanding is that CVE-2018–17144 (inflation bug introduced in 0.15) was partially a result of how convoluted the original code is.

Regardless of the specific CVE-2018–17144 example, are there parts of the bitcoin source code that are 'untouchable'? How much magic is there in the source code that contributors need to just figure out exists?

If bitcoin broke, would the core team jump at the opportunity to recreate bitcoin with all the new features (mast, tap, schnorr) etc. without the 'cancerous centre'? Is describing the source code as having a cancerous centre a huge overstatement, or is it playfully accurate? Is the idea of current contributors working around the old code like walking on eggshells accurate?

**Tags:** bitcoin-core, security


---

## 1 answers

### Answer by username_1 on 2019-05-07. Score: 4 | Accepted answer

I think that's an incorrect impression. The code involved in CVE-2018-17144 was relatively recent (the UTXO handling logic was written in 0.8 (released feb 2013), and underwent a few small changes after, and a large improvement in 0.15). If by "original code" you refer to the code written by Satoshi, this issue is unrelated, as fairly little of that code is left.

The issue wasn't that the code was not understood. It's that it wasn't clear which piece of code was responsible for what.

For performance reasons, block validation is split up in multiple stages; some run when the block is first received, some run later when it's actually a candidate for becoming part of the best chain ("activation"). The general principle is that you try to make the work wasted on verifying an invalid block proportional to the work an attacker has to do. To accomplish that, you verify the things that are cheap to fake first, and things that are expensive to fake later.

One of those checks was whether there were double spends within a transaction. Initially, this check was done when the block was received directly, and a sanity check during activation. In 0.14, in a performance improvement, the check in the initial block was removed, as it became the critical path for validation of compact blocks. This was wrong; instead of removing the code, it should have been moved to activation time, but it appeared that the activation code was already doing this check. In 0.15, when the UTXO handling code was rewritten, this sanity check at activation time was removed as well, because it didn't appear to be actually part of the validation code.

The mistake in my view was that the split up of responsibility for this type of double-spending check was non-obvious and should have been documented. I was largely responsible for this. Several procedures are in place to help prevent issues of this magnitude, including a sometimes annoyingly slow review process, and functional tests (which do exercise double-spending, ... just not double-spending-within-one-transaction). In this case, those procedures failed.

The rest of my answer is my personal opinion, as I don't think I can speak for anyone else.

> are there parts of the bitcoin source code that are 'untouchable'?

No. Certainly some parts are more dangerous to modify, but everything is a question of cost vs. benefit. The changes in 0.15 were a very justified performance improvement, and got a lot of eyes, but not enough. For the future I think this means the bar for review, and tests, vs the possible gain needs to be revisited for changes touching this UTXO handling code.

> How much magic is there in the source code that contributors need to just figure out exists?

I don't think there is any magic. Certainly not all logic is equally easy to understand, but that is true for any software project.

> If bitcoin broke, would the core team jump at the opportunity to recreate bitcoin

My personal opinion is that rewriting something from scratch is orders of magnitude harder to do correctly than making small incremental changes that are individually-reviewable. Note that the bar for correctness in consensus code is extremely high: not only must it accept all valid blocks, it must also reject all invalid blocks. This sort of consistency between codebases is extremely hard to maintain. Most reimplementations of Bitcoin's consensus rules have had one or a few serious mistakes in their history, and I wouldn't be surprised if most of them still have not exactly the same logic in all cases.

> bitcoin with all the new features (mast, tap, schnorr) etc.

Thankfully, those things are very well-isolated changes with a simple interface to the rest of the system. Script validation is pretty much just a "call a function with the output and input without much of any other context, and return true or false". There are also obvious ways to introduce backward-compatible changes in it: simply add additional "return false" statements in it that don't modify any other variables.

> Is describing the source code as having a cancerous centre a huge overstatement, or is it playfully accurate?

I wouldn't know what that centre would be, so I think it's an overstatement.

> Is the idea of current contributors working around the old code like walking on eggshells accurate?

Touching consensus code is always walking on eggshells, but that doesn't mean it shouldn't or can't be done. I also don't think it would be any different if the code had a different history - consensus correctness is just a hard problem.

> **Comments:**
> **username_2**: I think you also miss a bit of history here:  In the original code this kind of double spend was only caught in block construction/connection and so there was a bug where the mempool could could contain these transactions (which was mostly harmless except they could show up unconfirmed in the wallet).  This test was specifically added to protect the mempool and nothing else.  But its importance was undocumentedly (and unintentionally?) increased by later changes.  
> **username_2**: The mistake here depended on the fact that people "knew" the history and original purpose of the code and didn't know that far away consensus rules had been weakened.I think it's informative to observe that "bitcoin-abc" rewrote double spend validation for their "ctor" change and managed to preserve the same bug and not detect it in the new test cases they added. The fact that double spending within a transaction isn't the same as general double spending is exactly the sort of cryptographic subtly that comes up and is fiendishly hard to get right.  

---

