diff --git a/bitcoin/src/blockdata/block.rs b/bitcoin/src/blockdata/block.rs index 7be62e471..9d194d625 100644 --- a/bitcoin/src/blockdata/block.rs +++ b/bitcoin/src/blockdata/block.rs @@ -19,6 +19,7 @@ use crate::consensus::{encode, Decodable, Encodable, Params}; use crate::internal_macros::{impl_consensus_encoding, impl_hashencode}; use crate::pow::{CompactTarget, Target, Work}; use crate::prelude::*; +use crate::script::PushBytes; use crate::{merkle_tree, VarInt}; hashes::hash_newtype! { @@ -378,7 +379,7 @@ impl Block { match push.map_err(|_| Bip34Error::NotPresent)? { script::Instruction::PushBytes(b) => { // Check that the number is encoded in the minimal way. - let h = script::read_scriptint(b.as_bytes()) + let h = PushBytes::read_scriptint(b) .map_err(|_e| Bip34Error::UnexpectedPush(b.as_bytes().to_vec()))?; if h < 0 { Err(Bip34Error::NegativeHeight) diff --git a/bitcoin/src/blockdata/script/instruction.rs b/bitcoin/src/blockdata/script/instruction.rs index 5b941cf8b..b4b1bcd98 100644 --- a/bitcoin/src/blockdata/script/instruction.rs +++ b/bitcoin/src/blockdata/script/instruction.rs @@ -61,6 +61,27 @@ impl<'a> Instruction<'a> { Instruction::PushBytes(bytes) => ScriptBuf::reserved_len_for_slice(bytes.len()), } } + + /// Reads an integer from an Instruction, + /// returning Some(i64) for valid opcodes or pushed bytes, otherwise None + pub fn read_int(&self) -> Option { + match self { + Instruction::Op(op) => { + let v = op.to_u8(); + match v { + // OP_PUSHNUM_1 ..= OP_PUSHNUM_16 + 0x51..=0x60 => Some(v as i64 - 0x50), + // OP_PUSHNUM_NEG1 + 0x4f => Some(-1), + _ => None, + } + } + Instruction::PushBytes(bytes) => match PushBytes::read_scriptint(bytes) { + Ok(v) => Some(v), + _ => None, + }, + } + } } /// Iterator over a script returning parsed opcodes. diff --git a/bitcoin/src/blockdata/script/mod.rs b/bitcoin/src/blockdata/script/mod.rs index d0662d999..855d543aa 100644 --- a/bitcoin/src/blockdata/script/mod.rs +++ b/bitcoin/src/blockdata/script/mod.rs @@ -155,54 +155,10 @@ pub fn write_scriptint(out: &mut [u8; 8], n: i64) -> usize { len } -/// Decodes an integer in script(minimal CScriptNum) format. -/// -/// Notice that this fails on overflow: the result is the same as in -/// bitcoind, that only 4-byte signed-magnitude values may be read as -/// numbers. They can be added or subtracted (and a long time ago, -/// multiplied and divided), and this may result in numbers which -/// can't be written out in 4 bytes or less. This is ok! The number -/// just can't be read as a number again. -/// This is a bit crazy and subtle, but it makes sense: you can load -/// 32-bit numbers and do anything with them, which back when mult/div -/// was allowed, could result in up to a 64-bit number. We don't want -/// overflow since that's surprising --- and we don't want numbers that -/// don't fit in 64 bits (for efficiency on modern processors) so we -/// simply say, anything in excess of 32 bits is no longer a number. -/// This is basically a ranged type implementation. -/// -/// This code is based on the `CScriptNum` constructor in Bitcoin Core (see `script.h`). -pub fn read_scriptint(v: &[u8]) -> Result { - let last = match v.last() { - Some(last) => last, - None => return Ok(0), - }; - if v.len() > 4 { - return Err(Error::NumericOverflow); - } - // Comment and code copied from Bitcoin Core: - // https://github.com/bitcoin/bitcoin/blob/447f50e4aed9a8b1d80e1891cda85801aeb80b4e/src/script/script.h#L247-L262 - // If the most-significant-byte - excluding the sign bit - is zero - // then we're not minimal. Note how this test also rejects the - // negative-zero encoding, 0x80. - if (*last & 0x7f) == 0 { - // One exception: if there's more than one byte and the most - // significant bit of the second-most-significant-byte is set - // it would conflict with the sign bit. An example of this case - // is +-255, which encode to 0xff00 and 0xff80 respectively. - // (big-endian). - if v.len() <= 1 || (v[v.len() - 2] & 0x80) == 0 { - return Err(Error::NonMinimalPush); - } - } - - Ok(scriptint_parse(v)) -} - /// Decodes an integer in script format without non-minimal error. /// /// The overflow error for slices over 4 bytes long is still there. -/// See [`read_scriptint`] for a description of some subtleties of +/// See [`push_bytes::PushBytes::read_scriptint`] for a description of some subtleties of /// this function. pub fn read_scriptint_non_minimal(v: &[u8]) -> Result { if v.is_empty() { diff --git a/bitcoin/src/blockdata/script/push_bytes.rs b/bitcoin/src/blockdata/script/push_bytes.rs index 1488ebb8e..0b2a6428c 100644 --- a/bitcoin/src/blockdata/script/push_bytes.rs +++ b/bitcoin/src/blockdata/script/push_bytes.rs @@ -19,8 +19,10 @@ mod primitive { }; use super::PushBytesError; + use crate::blockdata::script::Error; #[allow(unused)] use crate::prelude::*; + use crate::script::scriptint_parse; #[cfg(any(target_pointer_width = "16", target_pointer_width = "32"))] fn check_limit(_: usize) -> Result<(), PushBytesError> { Ok(()) } @@ -73,6 +75,50 @@ mod primitive { /// Returns the underlying mutbale bytes. pub fn as_mut_bytes(&mut self) -> &mut [u8] { &mut self.0 } + + /// Decodes an integer in script(minimal CScriptNum) format. + /// + /// Notice that this fails on overflow: the result is the same as in + /// bitcoind, that only 4-byte signed-magnitude values may be read as + /// numbers. They can be added or subtracted (and a long time ago, + /// multiplied and divided), and this may result in numbers which + /// can't be written out in 4 bytes or less. This is ok! The number + /// just can't be read as a number again. + /// This is a bit crazy and subtle, but it makes sense: you can load + /// 32-bit numbers and do anything with them, which back when mult/div + /// was allowed, could result in up to a 64-bit number. We don't want + /// overflow since that's surprising --- and we don't want numbers that + /// don't fit in 64 bits (for efficiency on modern processors) so we + /// simply say, anything in excess of 32 bits is no longer a number. + /// This is basically a ranged type implementation. + /// + /// This code is based on the `CScriptNum` constructor in Bitcoin Core (see `script.h`). + pub fn read_scriptint(&self) -> Result { + let last = match self.as_bytes().last() { + Some(last) => last, + None => return Ok(0), + }; + if self.len() > 4 { + return Err(Error::NumericOverflow); + } + // Comment and code copied from Bitcoin Core: + // https://github.com/bitcoin/bitcoin/blob/447f50e4aed9a8b1d80e1891cda85801aeb80b4e/src/script/script.h#L247-L262 + // If the most-significant-byte - excluding the sign bit - is zero + // then we're not minimal. Note how this test also rejects the + // negative-zero encoding, 0x80. + if (*last & 0x7f) == 0 { + // One exception: if there's more than one byte and the most + // significant bit of the second-most-significant-byte is set + // it would conflict with the sign bit. An example of this case + // is +-255, which encode to 0xff00 and 0xff80 respectively. + // (big-endian). + if self.len() <= 1 || (self[self.len() - 2] & 0x80) == 0 { + return Err(Error::NonMinimalPush); + } + } + + Ok(scriptint_parse(self.as_bytes())) + } } macro_rules! delegate_index { diff --git a/bitcoin/src/blockdata/script/tests.rs b/bitcoin/src/blockdata/script/tests.rs index 9b4e1f71d..965e61890 100644 --- a/bitcoin/src/blockdata/script/tests.rs +++ b/bitcoin/src/blockdata/script/tests.rs @@ -310,23 +310,51 @@ fn scriptint_round_trip() { -((1 << 31) - 1), ]; for &i in test_vectors.iter() { - assert_eq!(Ok(i), read_scriptint(&build_scriptint(i))); - assert_eq!(Ok(-i), read_scriptint(&build_scriptint(-i))); + assert_eq!( + Ok(i), + PushBytes::read_scriptint( + <&PushBytes>::try_from(build_scriptint(i).as_slice()).unwrap() + ) + ); + assert_eq!( + Ok(-i), + PushBytes::read_scriptint( + <&PushBytes>::try_from(build_scriptint(-i).as_slice()).unwrap() + ) + ); assert_eq!(Ok(i), read_scriptint_non_minimal(&build_scriptint(i))); assert_eq!(Ok(-i), read_scriptint_non_minimal(&build_scriptint(-i))); } - assert!(read_scriptint(&build_scriptint(1 << 31)).is_err()); - assert!(read_scriptint(&build_scriptint(-(1 << 31))).is_err()); + assert!(PushBytes::read_scriptint( + <&PushBytes>::try_from(build_scriptint(1 << 31).as_slice()).unwrap() + ) + .is_err()); + assert!(PushBytes::read_scriptint( + <&PushBytes>::try_from(build_scriptint(-(1 << 31)).as_slice()).unwrap() + ) + .is_err()); assert!(read_scriptint_non_minimal(&build_scriptint(1 << 31)).is_err()); assert!(read_scriptint_non_minimal(&build_scriptint(-(1 << 31))).is_err()); } #[test] fn non_minimal_scriptints() { - assert_eq!(read_scriptint(&[0x80, 0x00]), Ok(0x80)); - assert_eq!(read_scriptint(&[0xff, 0x00]), Ok(0xff)); - assert_eq!(read_scriptint(&[0x8f, 0x00, 0x00]), Err(Error::NonMinimalPush)); - assert_eq!(read_scriptint(&[0x7f, 0x00]), Err(Error::NonMinimalPush)); + assert_eq!( + PushBytes::read_scriptint(<[_; 2] as AsRef>::as_ref(&[0x80, 0x00])), + Ok(0x80) + ); + assert_eq!( + PushBytes::read_scriptint(<[_; 2] as AsRef>::as_ref(&[0xff, 0x00])), + Ok(0xff) + ); + assert_eq!( + PushBytes::read_scriptint(<[_; 3] as AsRef>::as_ref(&[0x8f, 0x00, 0x00])), + Err(Error::NonMinimalPush) + ); + assert_eq!( + PushBytes::read_scriptint(<[_; 2] as AsRef>::as_ref(&[0x7f, 0x00])), + Err(Error::NonMinimalPush) + ); assert_eq!(read_scriptint_non_minimal(&[0x80, 0x00]), Ok(0x80)); assert_eq!(read_scriptint_non_minimal(&[0xff, 0x00]), Ok(0xff)); diff --git a/fuzz/fuzz_targets/bitcoin/deserialize_script.rs b/fuzz/fuzz_targets/bitcoin/deserialize_script.rs index a32f01f0f..c11178f90 100644 --- a/fuzz/fuzz_targets/bitcoin/deserialize_script.rs +++ b/fuzz/fuzz_targets/bitcoin/deserialize_script.rs @@ -23,7 +23,7 @@ fn do_test(data: &[u8]) { // reserialized as numbers. (For -1 through 16, this will use special ops; for // others it'll just reserialize them as pushes.) if bytes.len() == 1 && bytes[0] != 0x80 && bytes[0] != 0x00 { - if let Ok(num) = script::read_scriptint(bytes.as_bytes()) { + if let Ok(num) = script::PushBytes::read_scriptint(bytes) { b = b.push_int(num); } else { b = b.push_slice(bytes);