Skip to content

Commit 3e17ea4

Browse files
committed
txscript: reject OP_CODESEPARATOR in unexecuted branches for non-segwit
In this commit, we fix a policy-level divergence with Bitcoin Core when handling OP_CODESEPARATOR inside unexecuted OP_IF branches in non-segwit scripts with the ScriptVerifyConstScriptCode flag. Bitcoin Core's EvalScript (interpreter.cpp:474-476) places the SCRIPT_VERIFY_CONST_SCRIPTCODE check for OP_CODESEPARATOR before the fExec branch-execution gate, causing it to fire unconditionally on every OP_CODESEPARATOR encountered during script iteration -- even inside OP_FALSE OP_IF ... OP_ENDIF envelopes. Previously, btcd's equivalent check lived inside the opcodeCodeSeparator handler, which was never reached for opcodes in unexecuted branches due to the early return in executeOpcode that skips non-conditional opcodes when isBranchExecuting() is false. This meant a script like: OP_FALSE OP_IF OP_CODESEPARATOR OP_ENDIF <validation> would be rejected by Bitcoin Core's mempool but accepted by btcd's. The fix moves the check before the branch-execution gate in executeOpcode, matching Bitcoin Core's structure. This follows the existing pattern in btcd where isOpcodeDisabled and isOpcodeAlwaysIllegal checks already fire regardless of branch execution state. Note: SCRIPT_VERIFY_CONST_SCRIPTCODE is purely a policy flag (included in STANDARD_SCRIPT_VERIFY_FLAGS but not MANDATORY_SCRIPT_VERIFY_FLAGS), so this was not a consensus divergence. Both implementations would accept such transactions if mined in a block. Found via differential fuzzing by Bruno from bitcoinfuzz.
1 parent 3eacced commit 3e17ea4

File tree

2 files changed

+122
-0
lines changed

2 files changed

+122
-0
lines changed

txscript/engine.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,18 @@ func (vm *Engine) executeOpcode(op *opcode, data []byte) error {
485485
return scriptError(ErrElementTooBig, str)
486486
}
487487

488+
// With ScriptVerifyConstScriptCode, OP_CODESEPARATOR in non-segwit
489+
// script is rejected even in an unexecuted branch. This mirrors
490+
// Bitcoin Core's behavior where the check fires unconditionally
491+
// before the branch execution gate.
492+
if op.value == OP_CODESEPARATOR && vm.taprootCtx == nil &&
493+
vm.witnessProgram == nil &&
494+
vm.hasFlag(ScriptVerifyConstScriptCode) {
495+
496+
str := "OP_CODESEPARATOR used in non-segwit script"
497+
return scriptError(ErrCodeSeparator, str)
498+
}
499+
488500
// Nothing left to do when this is not a conditional opcode and it is
489501
// not in an executing branch.
490502
if !vm.isBranchExecuting() && !isOpcodeConditional(op.value) {

txscript/engine_test.go

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,3 +426,113 @@ func TestCheckSignatureEncoding(t *testing.T) {
426426
}
427427
}
428428
}
429+
430+
// TestCodeSepUnexecutedBranch ensures that OP_CODESEPARATOR is rejected in
431+
// non-segwit scripts even when inside an unexecuted OP_IF branch, when the
432+
// ScriptVerifyConstScriptCode flag is set. This matches Bitcoin Core's
433+
// behavior where the SCRIPT_VERIFY_CONST_SCRIPTCODE check fires
434+
// unconditionally before the branch execution gate.
435+
func TestCodeSepUnexecutedBranch(t *testing.T) {
436+
t.Parallel()
437+
438+
// A minimal transaction for script execution.
439+
tx := &wire.MsgTx{
440+
Version: 1,
441+
TxIn: []*wire.TxIn{{
442+
PreviousOutPoint: wire.OutPoint{
443+
Hash: chainhash.Hash([32]byte{
444+
0xc9, 0x97, 0xa5, 0xe5,
445+
0x6e, 0x10, 0x41, 0x02,
446+
0xfa, 0x20, 0x9c, 0x6a,
447+
0x85, 0x2d, 0xd9, 0x06,
448+
0x60, 0xa2, 0x0b, 0x2d,
449+
0x9c, 0x35, 0x24, 0x23,
450+
0xed, 0xce, 0x25, 0x85,
451+
0x7f, 0xcd, 0x37, 0x04,
452+
}),
453+
Index: 0,
454+
},
455+
SignatureScript: mustParseShortForm("TRUE"),
456+
Sequence: 4294967295,
457+
}},
458+
TxOut: []*wire.TxOut{{
459+
Value: 1000000000,
460+
PkScript: nil,
461+
}},
462+
LockTime: 0,
463+
}
464+
465+
tests := []struct {
466+
name string
467+
script string
468+
flags ScriptFlags
469+
wantErr bool
470+
errCode ErrorCode
471+
}{
472+
{
473+
// OP_CODESEPARATOR inside an unexecuted branch with
474+
// the const scriptcode flag set should be rejected.
475+
name: "codesep in unexecuted IF with const scriptcode",
476+
script: "0 IF CODESEPARATOR ENDIF TRUE",
477+
flags: ScriptVerifyConstScriptCode,
478+
wantErr: true,
479+
errCode: ErrCodeSeparator,
480+
},
481+
{
482+
// Without the flag, OP_CODESEPARATOR in an unexecuted
483+
// branch should be fine (consensus behavior).
484+
name: "codesep in unexecuted IF without const scriptcode",
485+
script: "0 IF CODESEPARATOR ENDIF TRUE",
486+
flags: 0,
487+
wantErr: false,
488+
},
489+
{
490+
// OP_CODESEPARATOR in an executed branch with the
491+
// const scriptcode flag should also be rejected.
492+
name: "codesep in executed branch with const scriptcode",
493+
script: "CODESEPARATOR TRUE",
494+
flags: ScriptVerifyConstScriptCode,
495+
wantErr: true,
496+
errCode: ErrCodeSeparator,
497+
},
498+
{
499+
// Nested unexecuted branches should still be caught.
500+
name: "codesep in nested unexecuted IF with const scriptcode",
501+
script: "0 IF 1 IF CODESEPARATOR ENDIF ENDIF TRUE",
502+
flags: ScriptVerifyConstScriptCode,
503+
wantErr: true,
504+
errCode: ErrCodeSeparator,
505+
},
506+
}
507+
508+
for _, test := range tests {
509+
t.Run(test.name, func(t *testing.T) {
510+
pkScript := mustParseShortForm(test.script)
511+
vm, err := NewEngine(
512+
pkScript, tx, 0, test.flags, nil, nil,
513+
-1, nil,
514+
)
515+
if err != nil {
516+
t.Fatalf("failed to create engine: %v", err)
517+
}
518+
519+
err = vm.Execute()
520+
521+
switch {
522+
case test.wantErr && err == nil:
523+
t.Fatal("expected error but execution " +
524+
"succeeded")
525+
526+
case !test.wantErr && err != nil:
527+
t.Fatalf("unexpected error: %v", err)
528+
529+
case test.wantErr && err != nil:
530+
if !IsErrorCode(err, test.errCode) {
531+
t.Fatalf("expected error code "+
532+
"%v, got: %v",
533+
test.errCode, err)
534+
}
535+
}
536+
})
537+
}
538+
}

0 commit comments

Comments
 (0)