Discourage P2WSH with too big script or stack #8685

1. 
   jl2012 commented at 6:51 pm on September 8, 2016: contributor

   Idea by @sipa

   This implements a policy limit for P2WSH, with witnessScript <= 3600 bytes, witness stack item size <= 80 bytes, and witness stack items <= 100

   3600 bytes witnessScript and 100 stack items are adequate for a n-of-100 multisig using OP_CHECKSIG, OP_ADD, and OP_EQUAL

   The max size for ECDSA signature is 73 bytes and nothing should use more than that with the current scripting language.

   This is to prevent abuse of witness space, and reduce the risks of DoS attack with some unknown special and big scripts.

2. Discourage P2WSH with too big script or stack

   This implements a policy limit for P2WSH, with witnessScript <= 3600 bytes, witness stack item size <= 80 bytes, and witness stack items <= 100

   194343fceb
3. 
   petertodd commented at 6:53 pm on September 8, 2016: contributor

   Concept ACK

   As this is not consensus, we can always increase the limit later.

4. laanwj added this to the milestone 0.13.1 on Sep 8, 2016
5. in src/script/interpreter.cpp: in 194343fceb

   1356@@ -1354,6 +1357,16 @@ static bool VerifyWitnessProgram(const CScriptWitness& witness, int witversion,
   1357         return set_success(serror);
   1358     }
   1359 
   1360+    // Policy limit for stack size and stack element size
   1361+    if (flags & SCRIPT_VERIFY_DISCOURAGE_BIG_P2WSH) {
   1362+        if (stack.size() > 100)
   1363+            return set_error(serror, SCRIPT_ERR_DISCOURAGE_BIG_P2WSH);
   1364+        for (unsigned int i = 0; i < stack.size(); i++) {
   1365+            if (stack.at(i).size() > 80)
   

   ---

   ---

   

   instagibbs commented at 7:33 pm on September 8, 2016:

   are there really no near/mid-term uses of something larger than 80 bytes?

   ---

   

   sipa commented at 7:38 pm on September 8, 2016:

   Apart from the hashing operations, there is not even an opcode in the scripting language that can process them.
6. in src/script/interpreter.cpp: in 194343fceb

   1356@@ -1354,6 +1357,16 @@ static bool VerifyWitnessProgram(const CScriptWitness& witness, int witversion,
   1357         return set_success(serror);
   1358     }
   1359 
   1360+    // Policy limit for stack size and stack element size
   

   ---

   ---

   

   instagibbs commented at 7:49 pm on September 8, 2016:

   Might make sense to stick this inside the witversion==0 block if we want to tie both behaviors to specific versions or neither if we don’t. (in which case let’s just do it earlier in processing)
7. 
   sipa commented at 7:50 pm on September 8, 2016: member

   I don’t think this needs to be implemented as a script interpreter option. It can be done long before script execution is attempted.
8. MarcoFalke added the label Needs backport on Sep 8, 2016
9. 
   dcousens commented at 0:16 am on September 9, 2016: contributor

   I don’t think this needs to be implemented as a script interpreter option. It can be done long before script execution is attempted.

   The modularity of the flag is nice though. Has there been any thought on making policies more modular? Not relevant here

10. 
    jl2012 commented at 3:52 am on September 9, 2016: contributor

    @sipa @instagibbs @dcousens Another option is make it inside #8499, which creates a smaller diff and won’t touch the consensus critical files
11. 
    sipa commented at 4:28 am on September 9, 2016: member

    @jl2012 Yes, I prefer doing it there.
12. 
    jl2012 commented at 10:10 am on September 9, 2016: contributor

    Implemented in a different way at #8499
13. 
    jl2012 commented at 2:55 pm on September 13, 2016: contributor

    Closed in favor of #8499
14. jl2012 closed this on Sep 13, 2016

    ---

15. 
    laanwj commented at 2:01 pm on September 26, 2016: member

    This was closed unmerged. Removing needs backport tag.
16. laanwj removed the label Needs backport on Sep 26, 2016
17. MarcoFalke locked this on Sep 8, 2021

Contributors
jl2012 petertodd instagibbs sipa dcousens laanwj

Milestone
0.13.1