txscript: Consensus audit.

This commit contains fixes from the results of a thorough audit of
txscript to find any cases of script evaluation which doesn't match the
required consensus behavior.  These conditions are fairly obscure and
highly unlikely to happen in any real scripts, but they could have
nevertheless been used by a clever attacker with malicious intent to
cause a fork.

Test cases which exercise these conditions have been added to the
reference tests and will contributed upstream to improve the quality for
the entire ecosystem.

txscript/data/script_valid.json

@@ -74,6 +74,7 @@
 
 ["1 1", "VERIFY", "P2SH,STRICTENC"],
 ["1 0x05 0x01 0x00 0x00 0x00 0x00", "VERIFY", "P2SH,STRICTENC", "values >4 bytes can be cast to boolean"],
+["1 0x01 0x80", "IF 0 ENDIF", "P2SH,STRICTENC", "negative 0 is false"],
 
 ["10 0 11 TOALTSTACK DROP FROMALTSTACK", "ADD 21 EQUAL", "P2SH,STRICTENC"],
 ["'gavin_was_here' TOALTSTACK 11 FROMALTSTACK", "'gavin_was_here' EQUALVERIFY 11 EQUAL", "P2SH,STRICTENC"],
@@ -409,6 +410,7 @@
 
 ["0 0", "EQUAL", "P2SH,STRICTENC"],
 ["0 0", "EQUALVERIFY 1", "P2SH,STRICTENC"],
+["0 0 1", "EQUAL EQUAL", "P2SH,STRICTENC", "OP_0 and bools must have identical byte representations"],
 
 ["0", "1ADD", "P2SH,STRICTENC"],
 ["2", "1SUB", "P2SH,STRICTENC"],

txscript/data/tx_invalid.json

@@ -19,6 +19,12 @@
 [[["0000000000000000000000000000000000000000000000000000000000000100", 0, "DUP HASH160 0x14 0x5b6462475454710f3c22f5fdf0b40704c92f25c3 EQUALVERIFY CHECKSIGVERIFY 1 0x4c 0x47 0x3044022067288ea50aa799543a536ff9306f8e1cba05b9c6b10951175b924f96732555ed022026d7b5265f38d21541519e4a1e55044d5b9e17e15cdbaf29ae3792e99e883e7a01"]],
 "01000000010001000000000000000000000000000000000000000000000000000000000000000000006b4c473044022067288ea50aa799543a536ff9306f8e1cba05b9c6b10951175b924f96732555ed022026d7b5265f38d21541519e4a1e55044d5b9e17e15cdbaf29ae3792e99e883e7a012103ba8c8b86dea131c22ab967e6dd99bdae8eff7a1f75a2c35f1f944109e3fe5e22ffffffff010000000000000000015100000000", "P2SH"],
 
+["This is the nearly-standard transaction with CHECKSIGVERIFY 1 instead of CHECKSIG from tx_valid.json"],
+["but with the signature duplicated in the scriptPubKey with a different hashtype suffix"],
+["See FindAndDelete, which will only remove if the signature, including the hash type, matches"],
+[[["0000000000000000000000000000000000000000000000000000000000000100", 0, "DUP HASH160 0x14 0x5b6462475454710f3c22f5fdf0b40704c92f25c3 EQUALVERIFY CHECKSIGVERIFY 1 0x47 0x3044022067288ea50aa799543a536ff9306f8e1cba05b9c6b10951175b924f96732555ed022026d7b5265f38d21541519e4a1e55044d5b9e17e15cdbaf29ae3792e99e883e7a81"]],
+"01000000010001000000000000000000000000000000000000000000000000000000000000000000006a473044022067288ea50aa799543a536ff9306f8e1cba05b9c6b10951175b924f96732555ed022026d7b5265f38d21541519e4a1e55044d5b9e17e15cdbaf29ae3792e99e883e7a012103ba8c8b86dea131c22ab967e6dd99bdae8eff7a1f75a2c35f1f944109e3fe5e22ffffffff010000000000000000015100000000", "P2SH"],
+
 ["An invalid P2SH Transaction"],
 [[["0000000000000000000000000000000000000000000000000000000000000100", 0, "HASH160 0x14 0x7a052c840ba73af26755de42cf01cc9e0a49fef0 EQUAL"]],
 "010000000100010000000000000000000000000000000000000000000000000000000000000000000009085768617420697320ffffffff010000000000000000015100000000", "P2SH"],

txscript/opcode.go

@@ -841,7 +841,7 @@ func opcodeInvalid(op *parsedOpcode, vm *Engine) error {
 // that 0, when encoded as a number according to the numeric encoding consensus
 // rules, is an empty array.
 func opcodeFalse(op *parsedOpcode, vm *Engine) error {
-	vm.dstack.PushByteArray([]byte(""))
+	vm.dstack.PushByteArray(nil)
 	return nil
 }
 
@@ -1784,7 +1784,7 @@ func opcodeCheckSig(op *parsedOpcode, vm *Engine) error {
 		return err
 	}
 
-	sigBytes, err := vm.dstack.PopByteArray()
+	fullSigBytes, err := vm.dstack.PopByteArray()
 	if err != nil {
 		return err
 	}
@@ -1792,7 +1792,7 @@ func opcodeCheckSig(op *parsedOpcode, vm *Engine) error {
 	// The signature actually needs needs to be longer than this, but at
 	// least 1 byte is needed for the hash type below.  The full length is
 	// checked depending on the script flags and upon parsing the signature.
-	if len(sigBytes) < 1 {
+	if len(fullSigBytes) < 1 {
 		vm.dstack.PushBool(false)
 		return nil
 	}
@@ -1809,8 +1809,8 @@ func opcodeCheckSig(op *parsedOpcode, vm *Engine) error {
 	// the data stack.  This is required because the more general script
 	// validation consensus rules do not have the new strict encoding
 	// requirements enabled by the flags.
-	hashType := SigHashType(sigBytes[len(sigBytes)-1])
+	hashType := SigHashType(fullSigBytes[len(fullSigBytes)-1])
-	sigBytes = sigBytes[:len(sigBytes)-1]
+	sigBytes := fullSigBytes[:len(fullSigBytes)-1]
 	if err := vm.checkHashTypeEncoding(hashType); err != nil {
 		return err
 	}
@@ -1826,7 +1826,7 @@ func opcodeCheckSig(op *parsedOpcode, vm *Engine) error {
 
 	// Remove the signature since there is no way for a signature to sign
 	// itself.
-	subScript = removeOpcodeByData(subScript, sigBytes)
+	subScript = removeOpcodeByData(subScript, fullSigBytes)
 
 	// Generate the signature hash based on the signature hash type.
 	hash := calcSignatureHash(subScript, hashType, &vm.tx, vm.txIdx)

txscript/stack.go

@@ -10,6 +10,10 @@ import "encoding/hex"
 func asBool(t []byte) bool {
 	for i := range t {
 		if t[i] != 0 {
+			// Negative 0 is also considered false.
+			if i == len(t)-1 && t[i] == 0x80 {
+				return false
+			}
 			return true
 		}
 	}
@@ -21,7 +25,7 @@ func fromBool(v bool) []byte {
 	if v {
 		return []byte{1}
 	}
-	return []byte{0}
+	return nil
 }
 
 // stack represents a stack of immutable objects to be used with bitcoin
@@ -334,6 +338,9 @@ func (s *stack) RollN(n int32) error {
 func (s *stack) String() string {
 	var result string
 	for _, stack := range s.stk {
+		if len(stack) == 0 {
+			result += "00000000  <empty>\n"
+		}
 		result += hex.Dump(stack)
 	}
 

txscript/stack_test.go

@@ -125,7 +125,7 @@ func TestStack(t *testing.T) {
 		},
 		{
 			"pop bool",
-			[][]byte{{0}},
+			[][]byte{nil},
 			func(s *stack) error {
 				val, err := s.PopBool()
 				if err != nil {
@@ -475,7 +475,7 @@ func TestStack(t *testing.T) {
 				return nil
 			},
 			nil,
-			[][]byte{{0}},
+			[][]byte{nil},
 		},
 		{
 			"PushBool PopBool",
@@ -867,7 +867,7 @@ func TestStack(t *testing.T) {
 		},
 		{
 			"Peek bool 2",
-			[][]byte{{0}},
+			[][]byte{nil},
 			func(s *stack) error {
 				// Peek bool is otherwise pretty well tested,
 				// just check it works.
@@ -881,7 +881,7 @@ func TestStack(t *testing.T) {
 				return nil
 			},
 			nil,
-			[][]byte{{0}},
+			[][]byte{nil},
 		},
 		{
 			"Peek int",