diff --git a/src/lib.rs b/src/lib.rs index a237437..5e72764 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -792,8 +792,10 @@ impl Exec { OP_PICK | OP_ROLL => { // (xn ... x2 x1 x0 n - xn ... x2 x1 x0 xn) // (xn ... x2 x1 x0 n - ... x2 x1 x0 xn) + self.stack.needn(2)?; let x = self.stack.topnum(-1, self.opt.require_minimal)?; - if x < 0 || x >= self.stack.len() as i64 { + // The selector itself is not part of the selectable pool. + if x < 0 || x >= (self.stack.len() - 1) as i64 { return Err(ExecError::InvalidStackOperation); } self.stack.pop().unwrap(); diff --git a/tests/stack_indices.rs b/tests/stack_indices.rs new file mode 100644 index 0000000..c6235e1 --- /dev/null +++ b/tests/stack_indices.rs @@ -0,0 +1,126 @@ +//! Regression tests for the operand-stack bounds shared by OP_PICK and OP_ROLL. + +use bitcoin::{ + absolute, + hashes::Hash, + opcodes::all::{OP_PICK, OP_ROLL}, + transaction, Opcode, ScriptBuf, TapLeafHash, Transaction, +}; +use bitcoin_scriptexec::{Exec, ExecCtx, ExecError, Options, TxTemplate}; + +const CONTEXTS: [ExecCtx; 3] = [ExecCtx::Legacy, ExecCtx::SegwitV0, ExecCtx::Tapscript]; + +fn executor(ctx: ExecCtx, opcode: Opcode, stack: Vec>) -> Exec { + Exec::new( + ctx, + Options::default(), + TxTemplate { + tx: Transaction { + version: transaction::Version::TWO, + lock_time: absolute::LockTime::ZERO, + input: vec![], + output: vec![], + }, + prevouts: vec![], + input_idx: 0, + taproot_annex_scriptleaf: Some((TapLeafHash::all_zeros(), None)), + }, + ScriptBuf::from_bytes(vec![opcode.to_u8()]), + stack, + ) + .expect("single-opcode fixture") +} + +fn assert_rejected(stack: Vec>, expected: ExecError) { + for ctx in CONTEXTS { + for opcode in [OP_PICK, OP_ROLL] { + let mut exec = executor(ctx, opcode, stack.clone()); + // A panic is a test failure: invalid input must return a normal + // execution error, including when the index equals the pool size. + let result = exec.exec_next().expect_err("invalid stack index"); + assert_eq!(result.error, Some(expected.clone()), "{ctx:?} {opcode:?}"); + assert_eq!(result.opcode, Some(opcode)); + assert!(!result.success); + } + } +} + +#[test] +fn first_and_last_stack_indices_are_valid() { + for ctx in CONTEXTS { + for (opcode, selector, expected) in [ + (OP_PICK, 0, vec![0x11, 0x22, 0x33, 0x33]), + (OP_PICK, 2, vec![0x11, 0x22, 0x33, 0x11]), + (OP_ROLL, 0, vec![0x11, 0x22, 0x33]), + (OP_ROLL, 2, vec![0x22, 0x33, 0x11]), + ] { + let mut stack = vec![vec![0x11], vec![0x22], vec![0x33]]; + stack.push(if selector == 0 { + vec![] + } else { + vec![selector] + }); + let mut exec = executor(ctx, opcode, stack); + exec.exec_next().expect("valid stack index"); + assert_eq!( + exec.stack().iter_str().collect::>(), + expected + .into_iter() + .map(|value| vec![value]) + .collect::>(), + "{ctx:?} {opcode:?} index {selector}", + ); + } + for opcode in [OP_PICK, OP_ROLL] { + let mut exec = executor(ctx, opcode, vec![vec![0x42], vec![]]); + exec.exec_next().expect("only pool element is selectable"); + let expected_len = if opcode == OP_PICK { 2 } else { 1 }; + assert_eq!( + exec.stack().iter_str().collect::>(), + vec![vec![0x42]; expected_len] + ); + } + } +} + +#[test] +fn exact_pool_length_is_rejected_without_panicking() { + assert_rejected(vec![vec![0x42], vec![1]], ExecError::InvalidStackOperation); + assert_rejected( + vec![vec![0x11], vec![0x22], vec![0x33], vec![3]], + ExecError::InvalidStackOperation, + ); +} + +#[test] +fn above_pool_length_is_rejected() { + assert_rejected(vec![vec![0x42], vec![2]], ExecError::InvalidStackOperation); + assert_rejected( + vec![vec![0x11], vec![0x22], vec![0x33], vec![4]], + ExecError::InvalidStackOperation, + ); +} + +#[test] +fn negative_stack_index_is_rejected() { + assert_rejected( + vec![vec![0x42], vec![0x81]], + ExecError::InvalidStackOperation, + ); +} + +#[test] +fn missing_operands_are_rejected_without_panicking() { + for stack in [vec![], vec![vec![]], vec![vec![0x42]], vec![vec![1; 5]]] { + assert_rejected(stack, ExecError::InvalidStackOperation); + } +} + +#[test] +fn malformed_stack_indices_keep_their_numeric_errors() { + assert_rejected(vec![vec![0x42], vec![0]], ExecError::MinimalData); + assert_rejected( + vec![vec![0x42], vec![1; 5]], + ExecError::ScriptIntNumericOverflow, + ); +}