Skip to content

Commit

Permalink
don't allow result of math op be 9 bytes
Browse files Browse the repository at this point in the history
  • Loading branch information
biryukovmaxim committed Nov 11, 2024
1 parent 863a9a1 commit 9d75993
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 13 deletions.
11 changes: 8 additions & 3 deletions crypto/txscript/src/data_stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub(crate) trait DataStack {
Vec<u8>: OpcodeData<T>;
fn pop_raw<const SIZE: usize>(&mut self) -> Result<[Vec<u8>; SIZE], TxScriptError>;
fn peek_raw<const SIZE: usize>(&self) -> Result<[Vec<u8>; SIZE], TxScriptError>;
fn push_item<T: Debug>(&mut self, item: T)
fn push_item<T: Debug>(&mut self, item: T) -> Result<(), TxScriptError>
where
Vec<u8>: OpcodeData<T>;
fn drop_items<const SIZE: usize>(&mut self) -> Result<(), TxScriptError>;
Expand Down Expand Up @@ -269,11 +269,16 @@ impl DataStack for Stack {
}

#[inline]
fn push_item<T: Debug>(&mut self, item: T)
fn push_item<T: Debug>(&mut self, item: T) -> Result<(), TxScriptError>
where
Vec<u8>: OpcodeData<T>,
{
Vec::push(self, OpcodeData::serialize(&item));
let v: Vec<u8> = OpcodeData::serialize(&item);
if v.len() > 8 {
return Err(TxScriptError::NumberTooBig("Binary representation of {item:?} is longer than 8 bytes".to_string()));
}
Vec::push(self, v);
Ok(())
}

#[inline]
Expand Down
2 changes: 1 addition & 1 deletion crypto/txscript/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ impl<'a, T: VerifiableTransaction, Reused: SigHashReusedValues> TxScriptEngine<'
return Err(TxScriptError::NullFail);
}

self.dstack.push_item(!failed);
self.dstack.push_item(!failed)?;
Ok(())
}

Expand Down
21 changes: 12 additions & 9 deletions crypto/txscript/src/opcodes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,10 @@ fn push_number<T: VerifiableTransaction, Reused: SigHashReusedValues>(
number: i64,
vm: &mut TxScriptEngine<T, Reused>,
) -> OpCodeResult {
vm.dstack.push_item(number);
if number == i64::MIN {
return Err(TxScriptError::NumberTooBig(format!("Binary representation of {number} exceeds 8 bytes limit")));
}
vm.dstack.push_item(number)?;
Ok(())
}

Expand All @@ -224,13 +227,13 @@ macro_rules! numeric_op {
if $vm.kip10_enabled {
let $pattern: [Kip10I64; $count] = $vm.dstack.pop_items()?;
let r = $block;
$vm.dstack.push_item(r);
$vm.dstack.push_item(r)?;
Ok(())
} else {
let $pattern: [i64; $count] = $vm.dstack.pop_items()?;
#[allow(clippy::useless_conversion)]
let r = $block;
$vm.dstack.push_item(r);
$vm.dstack.push_item(r)?;
Ok(())
}
};
Expand Down Expand Up @@ -543,7 +546,7 @@ opcode_list! {
opcode OpSize<0x82, 1>(self, vm) {
match vm.dstack.last() {
Some(last) => {
vm.dstack.push_item(i64::try_from(last.len()).map_err(|e| TxScriptError::NumberTooBig(e.to_string()))?);
vm.dstack.push_item(i64::try_from(last.len()).map_err(|e| TxScriptError::NumberTooBig(e.to_string()))?)?;
Ok(())
},
None => Err(TxScriptError::InvalidStackOperation(1, 0))
Expand Down Expand Up @@ -721,7 +724,7 @@ opcode_list! {
let hash_type = SigHashType::from_u8(typ).map_err(|e| TxScriptError::InvalidSigHashType(typ))?;
match vm.check_ecdsa_signature(hash_type, key.as_slice(), sig.as_slice()) {
Ok(valid) => {
vm.dstack.push_item(valid);
vm.dstack.push_item(valid)?;
Ok(())
},
Err(e) => {
Expand All @@ -730,7 +733,7 @@ opcode_list! {
}
}
None => {
vm.dstack.push_item(false);
vm.dstack.push_item(false)?;
Ok(())
}
}
Expand All @@ -744,7 +747,7 @@ opcode_list! {
let hash_type = SigHashType::from_u8(typ).map_err(|e| TxScriptError::InvalidSigHashType(typ))?;
match vm.check_schnorr_signature(hash_type, key.as_slice(), sig.as_slice()) {
Ok(valid) => {
vm.dstack.push_item(valid);
vm.dstack.push_item(valid)?;
Ok(())
},
Err(e) => {
Expand All @@ -753,7 +756,7 @@ opcode_list! {
}
}
None => {
vm.dstack.push_item(false);
vm.dstack.push_item(false)?;
Ok(())
}
}
Expand Down Expand Up @@ -3097,7 +3100,7 @@ mod test {
assert!(matches!(op_input_idx.execute(&mut vm), Err(TxScriptError::InvalidOpcode(_))));
} else {
let mut expected = vm.dstack.clone();
expected.push_item(current_idx as i64);
expected.push_item(current_idx as i64).unwrap();
op_input_idx.execute(&mut vm).unwrap();
assert_eq!(vm.dstack, expected);
vm.dstack.clear();
Expand Down
7 changes: 7 additions & 0 deletions crypto/txscript/test-data/script_tests-kip10.json
Original file line number Diff line number Diff line change
Expand Up @@ -4408,6 +4408,13 @@
"UNKNOWN_ERROR",
"We cannot do BOOLAND on 9-byte integers"
],
[
"-9223372036854775807",
"1SUB",
"",
"UNKNOWN_ERROR",
"Interpret 9 byte integer as bool"
],
[
"1",
"1 ENDIF",
Expand Down

0 comments on commit 9d75993

Please sign in to comment.