Re: [Bitcoin-development] SCRIPT_VERIFY_STRICTENC and CHECKSIG NOT
On Thu, Nov 06, 2014 at 05:38:20AM -0500, Peter Todd wrote: So right now git head will accept the following invalid transaction into the mempool: 010001140de229e08fda25cbc16ded2618cdacce49fcb18c0b6ccdace00040909adae49000493046022100f7828d81c849c5448ba5ba4ef55df6b4d0ba3ae3f1a59cff3291880c2c8e524f022100d2f5bc9dc2f0674eded31023cb47e61a596e10f8f1ddd44cf92d290c9db577c70144410778d430274f8c5ec1321338151e9f27f4c676a008bdf8638d07c0b6be9ab35c71a1518063243acd4dfe96b66e3f2ec8013c8e072cd09b3834a19f81f659cc3455ac9101102717a914e661a2229cc824329c9409f49d99cb5ac350c92887 which spends the redeemScript: 0778d430274f8c5ec1321338151e9f27f4c676a008bdf8638d07c0b6be9ab35c71a1518063243acd4dfe96b66e3f2ec8013c8e072cd09b3834a19f81f659cc3455 CHECKSIG NOT ...and while we're at it, bitcoin-ruby's forked yet again... -- 'peter'[:-1]@petertodd.org 152dc55f27338b58325f0432d2dc6edb90c8d449d9959583 signature.asc Description: Digital signature -- ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] SCRIPT_VERIFY_STRICTENC and CHECKSIG NOT
On Thu, Nov 6, 2014 at 2:38 AM, Peter Todd p...@petertodd.org wrote: However the implementation of the STRICTENC flag simply makes pubkey formats it doesn't recognize act as through the signature was invalid, rather than failing the transaction. Similar to the invalid due to too many sigops DoS attack I found before, this lets you fill up the mempool with garbage transactions that will never be mined. OTOH I don't see any way to exploit this in a v0.9.x IsStandard() transaction, so we haven't shipped code that actually has this vulnerability. (dunno about alt-implementations) Yeah, there's even a comment in script/interpreter.h currently about how STRICTENC is not softfork safe. I didn't realize that this would lead to the mempool accepting invalid transactions (I thought there was a second validity check with the actual consensus rules; if not, maybe we need to add that). I suggest we either change STRICTENC to simply fail unrecognized pubkeys immediately - similar to how non-standard signatures are treated - or fail the script if the pubkey is non-standard and signature verification succeeds. Sounds good to me, I disliked those semantics too. -- Pieter -- ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] SCRIPT_VERIFY_STRICTENC and CHECKSIG NOT
On Thu, Nov 6, 2014 at 2:47 AM, Pieter Wuille pieter.wui...@gmail.com wrote: I suggest we either change STRICTENC to simply fail unrecognized pubkeys immediately - similar to how non-standard signatures are treated - or fail the script if the pubkey is non-standard and signature verification succeeds. Sounds good to me, I disliked those semantics too. Of course: do we apply this rule to all pubkeys passed to CHECKMULTISIG (my preference...), or just the ones that are otherwise checked? This will likely make existing outputs hard to spend as well (I don't have numbers), are we okay with that? We probably can't make this a consensus rule, as it may make existing P2SH outputs/addresses unspendable. -- Pieter -- ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] SCRIPT_VERIFY_STRICTENC and CHECKSIG NOT
On Thu, Nov 06, 2014 at 02:47:29AM -0800, Pieter Wuille wrote:
On Thu, Nov 6, 2014 at 2:38 AM, Peter Todd p...@petertodd.org wrote:
However the implementation of the STRICTENC flag simply makes pubkey
formats it doesn't recognize act as through the signature was invalid,
rather than failing the transaction. Similar to the invalid due to too
many sigops DoS attack I found before, this lets you fill up the mempool
with garbage transactions that will never be mined. OTOH I don't see any
way to exploit this in a v0.9.x IsStandard() transaction, so we haven't
shipped code that actually has this vulnerability. (dunno about
alt-implementations)
Yeah, there's even a comment in script/interpreter.h currently about
how STRICTENC is not softfork safe.
Indeed.
I actually was thinking about SCRIPT_VERIFY_MINIMALDATA, CScript(), and
FindAndDelete() Specifically that if you were to change CScript() to
convert single-character PUSHDATA's to OP_number you'd be making a
consensus-critical change due to how FindAndDelete() is called with a a
CScript() signature. You didn't make that mistake, and I couldn't find a
way to exploit it anyway, but it reminded me of this STRICTENC stuff.
I didn't realize that this would
lead to the mempool accepting invalid transactions (I thought there
was a second validity check with the actual consensus rules; if not,
maybe we need to add that).
It should be enough to just duplicate the CheckInputs() call in
the AcceptToMemoryPool() function:
if (!CheckInputs(tx, state, view, true, STANDARD_SCRIPT_VERIFY_FLAGS, true))
{
return error(AcceptToMemoryPool: : ConnectInputs failed %s,
hash.ToString());
}
if (!CheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS,
true))
{
return error(AcceptToMemoryPool: : BUG FOUND Standard verify flags
passed yet mandatory flags failed. %s, hash.ToString());
}
I suggest we either change STRICTENC to simply fail unrecognized pubkeys
immediately - similar to how non-standard signatures are treated - or
fail the script if the pubkey is non-standard and signature verification
succeeds.
Sounds good to me, I disliked those semantics too.
Ok, then given we have to support hybrid encoding for awhile longer
anyway - I noticed your secp256k1 library supports it - lets do the
latter as a least invasive measure. I can't think of any case where
that'd be triggered other than delibrately. Doing that should make
STRICTENC a soft-fork-safe change, and we can decide at a later date if
we want to get rid of hybrid-encoded pubkeys in a further tightening of
the rules.
--
'peter'[:-1]@petertodd.org
19b3c625f667bd0b93011c0a37950545a6a8fccf0b08ae73
signature.asc
Description: Digital signature
--
___
Bitcoin-development mailing list
Bitcoin-development@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] SCRIPT_VERIFY_STRICTENC and CHECKSIG NOT
On Thu, 6 Nov 2014 05:45:09 -0500 Peter Todd p...@petertodd.org wrote: On Thu, Nov 06, 2014 at 05:38:20AM -0500, Peter Todd wrote: So right now git head will accept the following invalid transaction into the mempool: 010001140de229e08fda25cbc16ded2618cdacce49fcb18c0b6ccdace00040909adae49000493046022100f7828d81c849c5448ba5ba4ef55df6b4d0ba3ae3f1a59cff3291880c2c8e524f022100d2f5bc9dc2f0674eded31023cb47e61a596e10f8f1ddd44cf92d290c9db577c70144410778d430274f8c5ec1321338151e9f27f4c676a008bdf8638d07c0b6be9ab35c71a1518063243acd4dfe96b66e3f2ec8013c8e072cd09b3834a19f81f659cc3455ac9101102717a914e661a2229cc824329c9409f49d99cb5ac350c92887 which spends the redeemScript: 0778d430274f8c5ec1321338151e9f27f4c676a008bdf8638d07c0b6be9ab35c71a1518063243acd4dfe96b66e3f2ec8013c8e072cd09b3834a19f81f659cc3455 CHECKSIG NOT ...and while we're at it, bitcoin-ruby's forked yet again... It is? How do you reckon? The webbtc node just received a block: http://webbtc.com/block/006370724f73798f4c00c8da97f675c4dcf4605e9882913c You mean it would be forked off if this change was released? pgpDzb0nNBhAW.pgp Description: OpenPGP digital signature -- ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
