diff --git a/src/lib.rs b/src/lib.rs index a237437..415694a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -78,7 +78,7 @@ pub struct Experimental { /// Used to fine-tune different variables during execution. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Options { - /// Require data pushes be minimally encoded. + /// Require executed data pushes and consumed numbers to be minimally encoded. pub require_minimal: bool, //TODO(stevenroose) double check all fRequireMinimal usage in Core /// Verify OP_CHECKLOCKTIMEVERIFY. pub verify_cltv: bool, @@ -228,11 +228,9 @@ impl Exec { // We want to make sure the script is valid so we don't have to throw parsing errors // while executing. - let instructions = if opt.require_minimal { - script.instructions_minimal() - } else { - script.instructions() - }; + // Minimal-push policy applies only when a push is executed. Parse + // syntax here so nonminimal pushes in skipped branches remain valid. + let instructions = script.instructions(); if let Some(err) = instructions.clone().find_map(|res| res.err()) { return Err(Error::InvalidScript(err)); } @@ -245,11 +243,7 @@ impl Exec { // We box alocate the script to get a static Instructions iterator. // We will manually drop this allocation in the ops::Drop impl. let script = Box::leak(script.into_boxed_script()) as &'static Script; - let instructions = if opt.require_minimal { - script.instructions_minimal() - } else { - script.instructions() - }; + let instructions = script.instructions(); //TODO(stevenroose) make this more efficient let witness_size = @@ -501,6 +495,15 @@ impl Exec { return self.fail(ExecError::PushSize); } if exec { + if self.opt.require_minimal + && self.script[self.current_position..] + .instructions_minimal() + .next() + .expect("already parsed instruction") + .is_err() + { + return self.fail(ExecError::MinimalData); + } self.stack.pushstr(p.as_bytes()); } } diff --git a/tests/minimal_pushes.rs b/tests/minimal_pushes.rs new file mode 100644 index 0000000..fa38a75 --- /dev/null +++ b/tests/minimal_pushes.rs @@ -0,0 +1,217 @@ +use bitcoin::{hashes::Hash, ScriptBuf, TapLeafHash, Transaction}; +use bitcoin_scriptexec::{Error, Exec, ExecCtx, ExecError, Options, TxTemplate}; + +const CONTEXTS: [ExecCtx; 3] = [ExecCtx::Legacy, ExecCtx::SegwitV0, ExecCtx::Tapscript]; + +fn executor( + ctx: ExecCtx, + minimal: bool, + script: Vec, + witness: Vec>, +) -> Result { + Exec::new( + ctx, + Options { + require_minimal: minimal, + verify_minimal_if: false, + ..Options::default() + }, + TxTemplate { + tx: Transaction { + version: bitcoin::transaction::Version::TWO, + lock_time: bitcoin::absolute::LockTime::ZERO, + input: vec![], + output: vec![], + }, + prevouts: vec![], + input_idx: 0, + taproot_annex_scriptleaf: Some((TapLeafHash::all_zeros(), None)), + }, + ScriptBuf::from_bytes(script), + witness, + ) +} + +fn execute(ctx: ExecCtx, minimal: bool, script: Vec, witness: Vec>) -> Exec { + let mut exec = executor(ctx, minimal, script, witness) + .expect("syntactically valid scripts must construct independently of minimal-push policy"); + while exec.exec_next().is_ok() {} + exec +} + +fn nonminimal_pushes() -> Vec> { + vec![ + vec![0x4c, 0], // PUSHDATA1 instead of OP_0 + vec![1, 1], // data push instead of OP_1 + vec![1, 0x81], // data push instead of OP_1NEGATE + vec![0x4c, 1, 17], + vec![0x4d, 1, 0, 17], + vec![0x4e, 1, 0, 0, 0, 17], + ] +} + +#[test] +fn executed_nonminimal_pushes_obey_policy_at_execution() { + for ctx in CONTEXTS { + for minimal in [false, true] { + for mut script in nonminimal_pushes() { + script.extend([0x75, 0x51]); // DROP TRUE + let exec = execute(ctx, minimal, script, vec![]); + let result = exec.result().unwrap(); + if minimal { + assert_eq!(result.error, Some(ExecError::MinimalData)); + assert!(!result.success); + assert!( + exec.stack().is_empty(), + "a rejected push must not mutate the stack" + ); + } else { + assert_eq!(result.error, None); + assert!(result.success); + } + } + } + } +} + +#[test] +fn nonminimal_pushes_in_unexecuted_branches_are_accepted() { + for ctx in CONTEXTS { + for minimal in [false, true] { + for push in nonminimal_pushes() { + // Both a false IF body and the ELSE arm of a true IF are skipped. + for (prefix, suffix) in [ + (vec![0x00, 0x63], vec![0x68, 0x51]), + (vec![0x51, 0x63, 0x51, 0x67], vec![0x68]), + (vec![0x00, 0x63, 0x63], vec![0x68, 0x68, 0x51]), + ] { + let mut script = prefix; + script.extend(&push); + script.extend(suffix); + let exec = execute(ctx, minimal, script, vec![]); + assert_eq!(exec.result().unwrap().error, None); + assert!(exec.result().unwrap().success); + } + } + } + } +} + +#[test] +fn minimal_pushes_at_encoding_boundaries_are_accepted() { + let mut pushes = vec![vec![0], vec![0x4f], vec![0x51], vec![0x60], vec![1, 17]]; + for (size, mut prefix) in [ + (75, vec![75]), + (76, vec![0x4c, 76]), + (255, vec![0x4c, 255]), + (256, vec![0x4d, 0, 1]), + (520, vec![0x4d, 8, 2]), + ] { + prefix.extend(vec![17; size]); + pushes.push(prefix); + } + for ctx in CONTEXTS { + for minimal in [false, true] { + for push in &pushes { + let mut script = push.clone(); + script.extend([0x75, 0x51]); + let exec = execute(ctx, minimal, script, vec![]); + assert_eq!(exec.result().unwrap().error, None); + assert!(exec.result().unwrap().success); + } + } + } +} + +#[test] +fn malformed_pushes_remain_constructor_errors_even_in_dead_branches() { + for ctx in CONTEXTS { + for minimal in [false, true] { + for script in [ + vec![0x4c], + vec![0x4c, 2, 17], + vec![2, 17], + vec![0x00, 0x63, 0x4d, 1], + ] { + assert!(matches!( + executor(ctx, minimal, script, vec![]), + Err(Error::InvalidScript( + bitcoin::script::Error::EarlyEndOfScript + )) + )); + } + } + } +} + +#[test] +fn numeric_minimality_is_still_checked_when_a_number_is_consumed() { + for ctx in CONTEXTS { + for minimal in [false, true] { + for (number, expected) in [(vec![0], 1), (vec![0x80], 1), (vec![1, 0], 2)] { + let exec = execute( + ctx, + minimal, + vec![0x8b, 0x50 + expected, 0x9c], + vec![number.clone()], + ); + assert_eq!( + exec.result().unwrap().error, + minimal.then_some(ExecError::MinimalData) + ); + assert_eq!(exec.result().unwrap().success, !minimal); + // Merely dropping the same data must not require numeric canonicalization. + let dropped = execute(ctx, minimal, vec![0x75, 0x51], vec![number]); + assert!(dropped.result().unwrap().success); + } + let overflow = execute(ctx, minimal, vec![0x8b], vec![vec![1, 0, 0, 0, 0]]); + assert_eq!( + overflow.result().unwrap().error, + Some(ExecError::ScriptIntNumericOverflow) + ); + } + } +} + +#[test] +fn tapscript_minimal_if_remains_consensus_enforced_with_either_option() { + for minimal in [false, true] { + for condition in [vec![], vec![1], vec![0], vec![2], vec![0x80], vec![1, 0]] { + let valid = condition.is_empty() || condition == [1]; + // IF TRUE ELSE TRUE ENDIF; verify_minimal_if is deliberately false. + let exec = execute( + ExecCtx::Tapscript, + minimal, + vec![0x63, 0x51, 0x67, 0x51, 0x68], + vec![condition], + ); + assert_eq!(exec.result().unwrap().success, valid); + assert_eq!( + exec.result().unwrap().error, + (!valid).then_some(ExecError::TapscriptMinimalIf) + ); + } + } +} + +#[test] +fn oversized_push_precedes_minimality_and_fails_in_skipped_branches() { + let mut push = vec![0x4e, 9, 2, 0, 0]; // nonminimal PUSHDATA4 of 521 bytes + push.extend(vec![17; 521]); + for ctx in CONTEXTS { + for minimal in [false, true] { + for skipped in [false, true] { + let mut script = if skipped { vec![0, 0x63] } else { vec![] }; + script.extend(&push); + script.extend(if skipped { + vec![0x68, 0x51] + } else { + vec![0x75, 0x51] + }); + let exec = execute(ctx, minimal, script, vec![]); + assert_eq!(exec.result().unwrap().error, Some(ExecError::PushSize)); + assert!(!exec.result().unwrap().success); + } + } + } +}