From bb10f8484cf690c945138dabfe530d29f2440fc9 Mon Sep 17 00:00:00 2001 From: Ori Newman Date: Thu, 11 Oct 2018 11:21:14 +0300 Subject: [PATCH] [DEV-199] Apply the minimum-if-policy for all transactions (#88) * [DEV-199] apply minimum if * [DEV-199] refactor popIfBool * [DEV-199] use popIfBool in OP_NOTIF --- txscript/data/script_tests.json | 8 ++++++-- txscript/error.go | 5 +++++ txscript/error_test.go | 1 + txscript/opcode.go | 29 +++++++++++++++++++++++++++-- txscript/reference_test.go | 2 ++ 5 files changed, 41 insertions(+), 4 deletions(-) diff --git a/txscript/data/script_tests.json b/txscript/data/script_tests.json index 3867e7c51..618b4b214 100644 --- a/txscript/data/script_tests.json +++ b/txscript/data/script_tests.json @@ -72,7 +72,7 @@ ["1 1", "VERIFY", "", "OK"], ["1 0x05 0x01 0x00 0x00 0x00 0x00", "VERIFY", "", "OK", "values >4 bytes can be cast to boolean"], -["1 0x01 0x80", "IF 0 ENDIF", "", "OK", "negative 0 is false"], +["0x01 0x80", "VERIFY TRUE", "", "VERIFY", "negative 0 is false"], ["10 0 11", "TOALTSTACK DROP FROMALTSTACK ADD 21 EQUAL", "", "OK"], ["'gavin_was_here'", "TOALTSTACK 11 FROMALTSTACK 'gavin_was_here' EQUALVERIFY 11 EQUAL", "", "OK"], @@ -687,7 +687,7 @@ ["", "NOP", "", "EMPTY_STACK", "Checks EMPTY_STACK error"], -["'abc'", "IF INVERT ELSE 1 ENDIF", "", "DISABLED_OPCODE", "INVERT disabled"], +["'abc'", "INVERT VERIFY TRUE", "", "DISABLED_OPCODE", "INVERT disabled"], ["1 2 0", "IF AND ELSE 1 ENDIF NOP", "", "DISABLED_OPCODE", "AND disabled"], ["1 2 0", "IF OR ELSE 1 ENDIF NOP", "", "DISABLED_OPCODE", "OR disabled"], ["1 2 0", "IF XOR ELSE 1 ENDIF NOP", "", "DISABLED_OPCODE", "XOR disabled"], @@ -1064,6 +1064,10 @@ ["0 0x02 0x0000", "CHECKMULTISIGVERIFY 1", "", "UNKNOWN_ERROR"], ["0x02 0x0000 0", "CHECKMULTISIGVERIFY 1", "", "UNKNOWN_ERROR"], +["Check MINIMALIF"], +["2", "IF TRUE ELSE FALSE", "", "MINIMALIF"], +["2", "NOTIF TRUE ELSE FALSE", "", "MINIMALIF"], + ["Order of CHECKMULTISIG evaluation tests, inverted by swapping the order of"], ["pubkeys/signatures so they fail due to the STRICTENC rules on validly encoded"], diff --git a/txscript/error.go b/txscript/error.go index a99606a52..c03c17452 100644 --- a/txscript/error.go +++ b/txscript/error.go @@ -222,6 +222,10 @@ const ( // reached. ErrUnsatisfiedLockTime + // ErrMinimalIf is returned if the operand of an OP_IF/OP_NOTIF + // is not either an empty vector or [0x01]. + ErrMinimalIf + // numErrorCodes is the maximum error code number used in tests. This // entry MUST be the last entry in the enum. numErrorCodes @@ -270,6 +274,7 @@ var errorCodeStrings = map[ErrorCode]string{ ErrDiscourageUpgradableNOPs: "ErrDiscourageUpgradableNOPs", ErrNegativeLockTime: "ErrNegativeLockTime", ErrUnsatisfiedLockTime: "ErrUnsatisfiedLockTime", + ErrMinimalIf: "ErrMinimalIf", } // String returns the ErrorCode as a human-readable name. diff --git a/txscript/error_test.go b/txscript/error_test.go index 7085d98dd..9e82f9a89 100644 --- a/txscript/error_test.go +++ b/txscript/error_test.go @@ -57,6 +57,7 @@ func TestErrorCodeStringer(t *testing.T) { {ErrDiscourageUpgradableNOPs, "ErrDiscourageUpgradableNOPs"}, {ErrNegativeLockTime, "ErrNegativeLockTime"}, {ErrUnsatisfiedLockTime, "ErrUnsatisfiedLockTime"}, + {ErrMinimalIf, "ErrMinimalIf"}, {0xffff, "Unknown ErrorCode (65535)"}, } diff --git a/txscript/opcode.go b/txscript/opcode.go index 4df4ddaf5..ce3c47f4d 100644 --- a/txscript/opcode.go +++ b/txscript/opcode.go @@ -925,6 +925,31 @@ func opcodeNop(op *parsedOpcode, vm *Engine) error { return nil } +// popIfBool enforces the "minimal if" policy. In order to +// eliminate an additional source of nuisance malleability, we +// require the following: for OP_IF and OP_NOTIF, the top stack item MUST +// either be an empty byte slice, or [0x01]. Otherwise, the item at the top of +// the stack will be popped and interpreted as a boolean. +func popIfBool(vm *Engine) (bool, error) { + so, err := vm.dstack.PopByteArray() + if err != nil { + return false, err + } + + if len(so) == 1 && so[0] == 0x01 { + return true, nil + } + + if len(so) == 0 { + return false, nil + } + + str := fmt.Sprintf("with OP_IF or OP_NOTIF top stack item MUST "+ + "be an empty byte array or 0x01, and is instead: %v", + so[0]) + return false, scriptError(ErrMinimalIf, str) +} + // opcodeIf treats the top item on the data stack as a boolean and removes it. // // An appropriate entry is added to the conditional stack depending on whether @@ -943,7 +968,7 @@ func opcodeNop(op *parsedOpcode, vm *Engine) error { func opcodeIf(op *parsedOpcode, vm *Engine) error { condVal := OpCondFalse if vm.isBranchExecuting() { - ok, err := vm.dstack.PopBool() + ok, err := popIfBool(vm) if err != nil { return err @@ -978,7 +1003,7 @@ func opcodeIf(op *parsedOpcode, vm *Engine) error { func opcodeNotIf(op *parsedOpcode, vm *Engine) error { condVal := OpCondFalse if vm.isBranchExecuting() { - ok, err := vm.dstack.PopBool() + ok, err := popIfBool(vm) if err != nil { return err } diff --git a/txscript/reference_test.go b/txscript/reference_test.go index f189d0480..bf08d45c6 100644 --- a/txscript/reference_test.go +++ b/txscript/reference_test.go @@ -204,6 +204,8 @@ func parseExpectedResult(expected string) ([]ErrorCode, error) { return []ErrorCode{ErrNegativeLockTime}, nil case "UNSATISFIED_LOCKTIME": return []ErrorCode{ErrUnsatisfiedLockTime}, nil + case "MINIMALIF": + return []ErrorCode{ErrMinimalIf}, nil } return nil, fmt.Errorf("unrecognized expected result in test data: %v",