Implement excessive sighashing protection policy with tight sighash estimation

jl2012 commented at 7:17 pm on September 18, 2016: contributor

This implements a static estimation of sighash size for a transaction. A transaction with more than 90bytes of sighash per weight is non-standard. This is equivalent to 36MB for an 100kB non-segwit transaction, or 360MB for a block in the worst case. All transactions below 100kB with legitimate use of CHECK(MULTI)SIG should remain standard with this limit.

The estimation of sighash is based on the following 3 assumptions: a. OP_CODESEPARATOR and FindAndDelete are disabled by SCRIPT_VERIFY_CONST_SCRIPTCODE. This ensures that the scriptCode serialized in SignatureHash is always the same as the original script passing to the EvalScript. (part of this PR) b. SignatureHash is performed once only for each SIGHASH type. (#8654) c. Only 6 sighash types are allowed: ALL, NONE, SINGLE, and combinations with ANYONECANPAY (already enforced as policy with STRICTENC)

Todo: unit tests

jl2012 commented at 7:21 pm on September 18, 2016: contributor

@TheBlueMatt As you suggested, we could be more aggressive when disabling FindAndDelete. So eventually we may retire this function after a softfork.

jl2012 force-pushed on Sep 18, 2016

in src/primitives/transaction.cpp: in 04d9e6f5fa outdated

162+        size -= scriptSigSize;
163+        // If the scriptSig size is larger than 252, 2 bytes compactSize encoding is deducted.
164+        if (scriptSigSize > 252)
165+            size -= 2;
166+        /*
167+         * // scriptSig larger than 10000 btyes is invalid

instagibbs commented at 1:50 pm on September 19, 2016:

dead code

jl2012 commented at 7:35 pm on September 29, 2016:

removed

laanwj added the label P2P on Sep 21, 2016

jl2012 commented at 6:03 pm on September 21, 2016: contributor

A draft BIP is made for the detailed rationale of this PR: https://github.com/jl2012/bips/blob/sighash/bip-sighash.mediawiki

jl2012 force-pushed on Sep 29, 2016

jl2012 force-pushed on Sep 30, 2016

jl2012 renamed this:
~~Implement excessive sighashing protection policy~~
Implement excessive sighashing protection policy with tight sighash estimation
on Sep 30, 2016

jl2012 commented at 10:36 am on September 30, 2016: contributor

Unit tests are completed and related BIP updated

jl2012 force-pushed on Oct 27, 2016

jl2012 force-pushed on Oct 28, 2016

Add constant scriptCode policy in non-segwit scripts

This disables OP_CODESEPARATOR in non-segwit scripts (even in an unexecuted branch), and makes a positive FindAndDelete result invalid. This ensures that the scriptCode serialized in SignatureHash is always the same as the script passing to the EvalScript.

98416d6462

Add sighash limitation policy

This implements a static estimation of sighash size for a transaction. A transaction with more than 90bytes of sighash per weight is non-standard. This is equivalent to 36MB for an 100kB non-segwit transaction, or 360MB for a block in the worst case. All transactions below 100kB with legitimate use of CHECK(MULTI)SIG should remain standard with this limit.

e81ecd2ebe

Add transaction tests for constant scriptCode 67f4eba3ec

jl2012 force-pushed on Dec 22, 2016

Test sighash limit policy aa8c27515c

jl2012 force-pushed on Dec 22, 2016

jonasschnelli added this to the milestone 0.14.0 on Dec 22, 2016

laanwj removed this from the milestone 0.14.0 on Jan 12, 2017

laanwj commented at 7:27 pm on January 12, 2017: member

Removing 0.14 tag as discussed in today’s meeting

TheBlueMatt commented at 6:19 pm on September 29, 2017: member

Strong Concept ACK on at least making CODESEPARATOR and FindAndDelete non-standard. Can we push forward on that independantly to get it done sooner rather than later, then at least the sighash limits can be reviewed separately and are more straight-forward? Any plans to rebase this @jl2012?

jl2012 commented at 9:18 pm on September 29, 2017: contributor

@TheBlueMatt : sure, I’ll make another PR just for the SCRIPT_VERIFY_CONST_SCRIPTCODE policy

sipa commented at 5:09 pm on March 6, 2018: member

@jl2012 What’s the status here?

jl2012 commented at 6:56 pm on March 6, 2018: contributor

@sipa: this requires #8654 and #11423. But #8654 needs to maintain a cache of 256 32-bytes hashes per input which might impact validation. It could be reduced to 6 hashes/input if we softfork away those 250 non-std nHashType.

The alternative is #8756, which does not require #8654 and #11423. But the counting is more conservative (overestimating)

in src/script/interpreter.h:112 in aa8c27515c

105@@ -106,6 +106,10 @@ enum
106     // Public keys in segregated witness scripts must be compressed
107     //
108     SCRIPT_VERIFY_WITNESS_PUBKEYTYPE = (1U << 15),
109+
110+    // Making OP_CODESEPARATOR and FindAndDelete non-standard in non-segwit scripts
111+    //
112+    SCRIPT_VERIFY_CONST_SCRIPTCODE = (1U << 16),

MarcoFalke commented at 7:37 pm on June 5, 2018:

Does adding a new constant require the tests to be updated as well? See ValidateCheckInputsForAllFlags

MarcoFalke added the label Needs rebase on Jun 6, 2018

jl2012 closed this on Sep 4, 2018

fanquake removed this from the "In progress" column in a project

laanwj removed the label Needs rebase on Oct 24, 2019

DrahtBot locked this on Dec 16, 2021

Implement excessive sighashing protection policy with tight sighash estimation #8755