Check against MANDATORY flags prior to accepting to mempool #5253

1. 
   petertodd commented at 7:58 am on November 10, 2014: contributor

   Previously transactions were only tested again the STANDARD_SCRIPT_VERIFY_FLAGS prior to mempool acceptance, so any bugs in those flags that allowed actually-invalid transactions to pass would result in allowing invalid transactions into the mempool. Fortunately there is a second check in CreateNewBlock() that would prevent those transactions from being mined, resulting in an invalid block, however this could still be exploited as a DoS attack.

   An example transaction failing this test that was previously allowed into the mempool is:

   0100000001db7f1e5f08248867e5825fdb24e6d8ce4de652e27f6c22a26e3c9380866ea3e6000000008e0047304402203d82f29cdff31153f533039fe7c3dd854e899b7372b8f1d22c50839bbb9481490220425c983fc9ae879853da193b83aecac8b64825ab01583f4ec10a6b18ae51ced60144410778d430274f8c5ec1321338151e9f27f4c676a008bdf8638d07c0b6be9ab35c71a1518063243acd4dfe96b66e3f2ec8013c8e072cd09b3834a19f81f659cc3455ac91ffffffff01881300000000000017a914e661a2229cc824329c9409f49d99cb5ac350c9288700000000

   which spends:

   0778d430274f8c5ec1321338151e9f27f4c676a008bdf8638d07c0b6be9ab35c71a1518063243acd4dfe96b66e3f2ec8013c8e072cd09b3834a19f81f659cc3455 CHECKSIG NOT

   with a valid signature. The existing, broken, STRICTENC, implementation causes the CHECKSIG to return false even though the signature is valid against the mandatory flags.

   (FWIW @sipa is fixing this in #5247)

2. Check against MANDATORY flags prior to accepting to mempool

   Previously transactions were only tested again the
   STANDARD_SCRIPT_VERIFY_FLAGS prior to mempool acceptance, so any bugs in
   those flags that allowed actually-invalid transactions to pass would
   result in allowing invalid transactions into the mempool. Fortunately
   there is a second check in CreateNewBlock() that would prevent those
   transactions from being mined, resulting in an invalid block, however
   this could still be exploited as a DoS attack.

   7c041b3b91
3. 
   sipa commented at 12:04 pm on November 12, 2014: member

   utACK. I wonder how much impact this will have on transaction relay speed/cost, but probably not much, as it should use signature caching.
4. 
   petertodd commented at 12:16 pm on November 12, 2014: contributor

   @sipa Yeah, the signature cache is used here.
5. 
   sipa commented at 9:43 pm on November 12, 2014: member

   Yeah, should be. This is certainly a safe defense.
6. laanwj added the label TX fees and policy on Dec 5, 2014
7. 
   btcdrak commented at 8:29 pm on December 15, 2014: contributor

   I think this patch should be included in 0.10.
8. 
   laanwj commented at 8:31 am on December 16, 2014: member

   Probably naive reasoning, but as STANDARD_SCRIPT_VERIFY_FLAGS is defined as a superset of MANDATORY_SCRIPT_VERIFY_FLAGS

   0static const unsigned int STANDARD_SCRIPT_VERIFY_FLAGS = MANDATORY_SCRIPT_VERIFY_FLAGS |
   1    SCRIPT_VERIFY_STRICTENC |
   2    ...
   

   …doesn’t that mean that we already check against a stricter check before we even get there? Is this a precaution against bugs where some of the script flags weaken instead of strengthen the validation?

9. 
   petertodd commented at 11:51 am on December 16, 2014: contributor

   @laanwj That’s exactly what it’s for.
10. 
    sipa commented at 11:56 am on December 16, 2014: member

    @laanwj Indeed. Note that until recently, not all script evaluation flags were actually strengthenings. Belt-and-suspenders.
11. 
    laanwj commented at 1:20 pm on December 16, 2014: member

    That’s scary and entirely unexpected. utACK.
12. laanwj added this to the milestone 0.10.0 on Dec 16, 2014
13. 
    TheBlueMatt commented at 3:23 am on December 21, 2014: member

    utACK
14. 
    btcdrak commented at 3:53 pm on December 21, 2014: contributor

    utACK
15. laanwj merged this on Dec 22, 2014
16. laanwj closed this on Dec 22, 2014

    ---

17. laanwj referenced this in commit 203632d20b on Dec 22, 2014
18. MarcoFalke locked this on Sep 8, 2021

Contributors
petertodd sipa btcdrak laanwj TheBlueMatt

Labels
TX fees and policy

Milestone
0.10.0