|
Title: What is up with this SIGHASH_SINGLE and nOut out of range? Post by: piotr_n on July 23, 2013, 12:53:59 PM Since yesterday there have been like a few txs, committed into the blockchain, which IMO are literally broken.
This broke my client BTW, but I fixed it already, and I know this cannot be reverted anymore, so I only wonder.. From what I understand using SIGHASH_SINGLE that has more outputs than inputs does not make any sense - so the code even prints an error then: Code: unsigned int nOut = nIn; So what is up with it? Is it like someone just overlooked this piece of code - or is it "error => hash=1" by design? EDIT: If you don't know what I am talking about, checks these three txs: Code: 315ac7d4c26d69668129cc352851d9389b4a6868f1509c6c8b66bead11e2619f Title: Re: What is up with this SIGHASH_SINGLE and nOut out of range? Post by: blueadept on July 23, 2013, 01:03:39 PM https://bitcointalk.org/index.php?topic=260595.0
Title: Re: What is up with this SIGHASH_SINGLE and nOut out of range? Post by: jackjack on July 23, 2013, 01:06:12 PM Why would SIGHASH_SINGLE require nIn==nOut? It only require nIn<=nOut Title: Re: What is up with this SIGHASH_SINGLE and nOut out of range? Post by: piotr_n on July 23, 2013, 01:10:43 PM https://bitcointalk.org/index.php?topic=260595.0 Thx.And sorry for double post. It only require nIn<=nOut That's right.But what if nIn=2 and nOut=1 - like here? Well, now I know: use the hash value of 1, but don't dare to fail the script! Title: Re: What is up with this SIGHASH_SINGLE and nOut out of range? Post by: jackjack on July 23, 2013, 01:21:05 PM https://bitcointalk.org/index.php?topic=260595.0 Thx.And sorry for double post. It only require nIn<=nOut That's right.But what if nIn=2 and nOut=1 - like here? Well, now I know: use the hash value of 1, but don't dare to fail the script! Wow I didn't know... By the protocol? Title: Re: What is up with this SIGHASH_SINGLE and nOut out of range? Post by: piotr_n on July 23, 2013, 01:22:45 PM https://bitcointalk.org/index.php?topic=260595.0 Thx.And sorry for double post. It only require nIn<=nOut That's right.But what if nIn=2 and nOut=1 - like here? Well, now I know: use the hash value of 1, but don't dare to fail the script! Wow I didn't know... Is it allowed by bitcoin-qt? By the protocol? I don't find anything in the linked thread And now we all need to live with it - that's another argument on why a documentation other than satoshi's code is useless :) Title: Re: What is up with this SIGHASH_SINGLE and nOut out of range? Post by: jackjack on July 23, 2013, 01:25:57 PM https://bitcointalk.org/index.php?topic=260595.0 Thx.And sorry for double post. It only require nIn<=nOut That's right.But what if nIn=2 and nOut=1 - like here? Well, now I know: use the hash value of 1, but don't dare to fail the script! Wow I didn't know... Is it allowed by bitcoin-qt? By the protocol? I don't find anything in the linked thread And now we all need to live with it. Live with it ok, but we need to know what is the good behavior for future implementations! Doesn't this look like a crazy hack/bug? The only piece of protocol we have is this: Quote Procedure for Hashtype SIGHASH_SINGLE That requires adding empty outputs until len=index+1, then processingThe output of txCopy is resized to the size of the current input index+1. All other txCopy outputs aside from the output that is the same as the current input index are set to a blank script and a value of (long) -1. All other txCopy inputs aside from the current input are set to have an nSequence index of zero. Is it what bitcoin-qt does? Title: Re: What is up with this SIGHASH_SINGLE and nOut out of range? Post by: piotr_n on July 23, 2013, 01:28:28 PM Live with it ok, but we need to know what is the good behavior! Thus my question.Doesn't this look like a crazy hack/bug? For me it seems like someone has overlooked it at some point; "return 1" was supposed to indicate an error, but it was somehow overlooked and now it implicitly gets cast to (uint256)1 which is a pretty valid hash value, so C++ compiler does not complain and so nobody has noticed.. Don't you have the same problem in Armory? I cannot imagine reproducing the same behavior in python, just by repeating such a mistake. Title: Re: What is up with this SIGHASH_SINGLE and nOut out of range? Post by: Mike Hearn on July 23, 2013, 02:00:32 PM I believe Peter created these transactions, and I think that was a good idea. Go Peter!
Speaking to all readers of this thread (not specifically piotr_n) - if your implementation broke because of these transactions, two things to consider: 1) Your implementation almost certainly contains other chain splitting bugs beyond this one. 2) You should be using Matt's blocktester which would have revealed the issue, as Matt discovered this bug >1 year ago when he did the bitcoinj full verification mode. Title: Re: What is up with this SIGHASH_SINGLE and nOut out of range? Post by: jgarzik on July 23, 2013, 02:04:59 PM Live with it ok, but we need to know what is the good behavior! Thus my question.Doesn't this look like a crazy hack/bug? For me it seems like someone has overlooked it at some point; "return 1" was supposed to indicate an error, but it was somehow overlooked and now it implicitly gets cast to (uint256)1 which is a pretty valid hash value, so C++ compiler does not complain and so nobody has noticed.. Don't you have the same problem in Armory? I cannot imagine reproducing the same behavior in python, just by repeating such a mistake. This was noticed a long time ago. It is just now being re-noticed by others :) I discovered this over a year ago while rewriting everything into python: https://github.com/jgarzik/python-bitcoinlib/ https://github.com/jgarzik/python-bitcoinlib/blob/master/bitcoin/scripteval.py Can you spot the problem? :) Title: Re: What is up with this SIGHASH_SINGLE and nOut out of range? Post by: piotr_n on July 23, 2013, 02:05:43 PM I believe Peter created these transactions, and I think that was a good idea. Go Peter! Yes.As much as I don't get along with him in general, thank you @Peter for bringing these txs to the network! I was always worried about different implementation/language related technicals that could be exploited to create a hard fork, so I was designing my node to be less strict in any checking, for such cases. Since it is not a mining node, it should not matter much, but Peter has actually managed to find a case where my implementation was more strict.. And now I have at least one less to go :) Title: Re: What is up with this SIGHASH_SINGLE and nOut out of range? Post by: piotr_n on July 23, 2013, 02:08:13 PM Live with it ok, but we need to know what is the good behavior! Thus my question.Doesn't this look like a crazy hack/bug? For me it seems like someone has overlooked it at some point; "return 1" was supposed to indicate an error, but it was somehow overlooked and now it implicitly gets cast to (uint256)1 which is a pretty valid hash value, so C++ compiler does not complain and so nobody has noticed.. Don't you have the same problem in Armory? I cannot imagine reproducing the same behavior in python, just by repeating such a mistake. This was noticed a long time ago. It is just now being re-noticed by others :) I discovered this over a year ago while rewriting everything into python: https://github.com/jgarzik/python-bitcoinlib/ https://github.com/jgarzik/python-bitcoinlib/blob/master/bitcoin/scripteval.py Can you spot the problem? :) Unless your goal is to make sure that the clients you develop keep the monopoly. :) Then its probably a good strategy, to not make such an info public. Title: Re: What is up with this SIGHASH_SINGLE and nOut out of range? Post by: piotr_n on July 23, 2013, 03:53:20 PM Just as a general comment, to anyone reading this: please always, whenever you discover such a case, do whatever you can to public any possible sources of hard forks that can be a consequence of an existing implementation, that is not an obvious design decision, but rather a hard to spot, implementation specific whatever (a bug or a feature).
As they say it: knowledge is power. And knowledge of how you can attack a 1+bn USD worth of a currency is a power worth exploiting. So if you are honest people, and I think you are, please do not withhold such an important info, but rather advertise it as much as you can - embedding test cases in the block chain is the perfect ultimate solution. Title: Re: What is up with this SIGHASH_SINGLE and nOut out of range? Post by: jackjack on July 23, 2013, 09:13:57 PM Just as a general comment, to anyone reading this: please always, whenever you discover such a case, do whatever you can to public any possible sources of hard forks that can be a consequence of an existing implementation, that is not an obvious design decision, but rather a hard to spot, implementation specific whatever (a bug or a feature). +1I think such transactions must be treated as non-standard Title: Re: What is up with this SIGHASH_SINGLE and nOut out of range? Post by: Peter Todd on July 23, 2013, 09:24:55 PM Just as a general comment, to anyone reading this: please always, whenever you discover such a case, do whatever you can to public any possible sources of hard forks that can be a consequence of an existing implementation, that is not an obvious design decision, but rather a hard to spot, implementation specific whatever (a bug or a feature). We do that already. This issue with SIGHASH_SINGLE was documented in the unittests here: https://github.com/bitcoin/bitcoin/blob/master/src/test/data/tx_valid.json#L39 (I did not find it first) The other main source of test cases is Matt Corallo's block tester: https://github.com/TheBlueMatt/test-scripts You're welcome to go through those test cases and summarize the more subtle ones, but frankly I don't see the point: any change, no matter how small, triggers a hard fork. Perfect compliance with those tests is a necessary but far from sufficient requirement. Title: Re: What is up with this SIGHASH_SINGLE and nOut out of range? Post by: piotr_n on July 24, 2013, 05:17:28 PM We do that already. This issue with SIGHASH_SINGLE was documented in the unittests here: https://github.com/bitcoin/bitcoin/blob/master/src/test/data/tx_valid.json#L39 (I did not find it first) Thanks! The vectors from the json file helped me a lot today. And the tx_invalid.json also turned out to be useful.The other main source of test cases is Matt Corallo's block tester: https://github.com/TheBlueMatt/test-scripts Who would have thought that cutting off the signature from a message that gets hashed is such a complex operation (talking about the length prefix here). Though, the Matt's block tester - I did not even try to build it, not too mention figuring out how to use it.. Maybe some day. You're welcome to go through those test cases and summarize the more subtle ones, but frankly I don't see the point: any change, no matter how small, triggers a hard fork. Perfect compliance with those tests is a necessary but far from sufficient requirement. Sure - but at least this many less to go :)BTW, it would be nice to a have a test tx that triggers this code: Code: // In case concatenating two scripts ends up with two codeseparators, Title: Re: What is up with this SIGHASH_SINGLE and nOut out of range? Post by: Peter Todd on July 24, 2013, 05:53:27 PM BTW, it would be nice to a have a test tx that triggers this code: Code: // In case concatenating two scripts ends up with two codeseparators, Working on it. FWIW I'd suggest you consider removing script evaluation from your library entirely for now. Keep the rest of the scripting code - you need it to create transactions after all - but make it clear that the only way you know that a transaction is valid is if a miner mines it. This is more in-line with the guarantees SPV clients can provide anyway, and will save you an enormous amount of work that is probably wasted until we have better testing infrastructure for third party clients to use. Title: Re: What is up with this SIGHASH_SINGLE and nOut out of range? Post by: piotr_n on July 24, 2013, 06:01:48 PM FWIW I'd suggest you consider removing script evaluation from your library entirely for now. Keep the rest of the scripting code - you need it to create transactions after all - but make it clear that the only way you know that a transaction is valid is if a miner mines it. This is more in-line with the guarantees SPV clients can provide anyway, and will save you an enormous amount of work that is probably wasted until we have better testing infrastructure for third party clients to use. Oh, no!Thanks for the advise to make my life easy, but switching the validation off, after I had spent so much effort on validating the txs, is like admitting that I failed on the most important part. I know that there are still things that will make my code fail, eventually somewhere in a future, but that is exactly where I need people like you, who try to break it now, while I seem to be the only user, thus with no big consequences. So please keep up the good job, and let me handle whatever you break ;) Title: Re: What is up with this SIGHASH_SINGLE and nOut out of range? Post by: genjix on July 24, 2013, 07:13:44 PM yeah it returns one, but the tx is still valid in the blockchain. i know it's a big wtf.
the actual hash looks like 00 00 00 ... 00 01 (last byte is 0x01 but all the rest is 0x00). Title: Re: What is up with this SIGHASH_SINGLE and nOut out of range? Post by: Peter Todd on July 24, 2013, 07:25:37 PM Oh, no! Thanks for the advise to make my life easy, but switching the validation off, after I had spent so much effort on validating the txs, is like admitting that I failed on the most important part. I know that there are still things that will make my code fail, eventually somewhere in a future, but that is exactly where I need people like you, who try to break it now, while I seem to be the only user, thus with no big consequences. So please keep up the good job, and let me handle whatever you break ;) Successful people are willing to cut their losses and refocus their efforts. You can always retrieve the code from revision control history later when the testing infrastructure improves; without good testing capabilities you're wasting your time. Quote The general who advances without coveting fame and retreats without fearing disgrace, whose only thought is to protect his country and do good service for his sovereign, is the jewel of the kingdom. Title: Re: What is up with this SIGHASH_SINGLE and nOut out of range? Post by: Sergio_Demian_Lerner on July 24, 2013, 08:44:05 PM yeah it returns one, but the tx is still valid in the blockchain. i know it's a big wtf. the actual hash looks like 00 00 00 ... 00 01 (last byte is 0x01 but all the rest is 0x00). Now we've found the first and only way to test the execution of the line: Code: // Drop the signature, since there's no way for a signature to sign itself (for which I thought it would be impossible to ever test in a live transaction) Since the hash "00 .. 01" does not depend on the script hash that spends the output, you can create a scriptpub that actually includes the signature of the hash "00 .. 01" in the script. For example: <sig("00 .. 01")> OP_DROP OP_DUP OP_HASH160 <pubKeyHash> OP_EQUALVERIFY OP_CHECKSIG Will result in <sig("00 .. 01")> being stripped from the script when the output transaction is redeem by a SIGHASH_SINGLE signature with an out-of-bounds nOut. Nevertheless since it won't affect in any way the outcome of the signature checking, you'll only be checking if that execution path aborts with an exception or continues normally. Title: Re: What is up with this SIGHASH_SINGLE and nOut out of range? Post by: Sergio_Demian_Lerner on July 25, 2013, 12:44:07 AM BTW, we should be happy Satoshi choose 1 as the error code and not 0, because 0 is a weak message for a ECDSA hash and can be easily forged. This would result in anybody being able to steal each other coins.
(or maybe he knew about this fact) |