[RDS-2923] Check overflows + config fix (#32)

Author: kostekIV

crates/redstone/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "redstone"
-version = "2.0.0-pre"
+version = "2.0.1"
 edition = "2021"
 authors = ["RedStone <https://redstone.finance>"]
 description = "A Rust implementation of deserializing&decrypting RedStone payload"
@@ -59,7 +59,7 @@ derive-getters = "0.5.0"
 getrandom = { version = "^0.2.15", default-features = false, features = ["js"] }
 
 [dev-dependencies]
-criterion = "0.5.1"
+criterion = { version = "0.5.1", default-features = false }
 itertools = { version = "^0.13.0" }
 rand = "0.8.5"
 

crates/redstone/src/contract/verification.rs

@@ -62,7 +62,7 @@ const MAX_SIGNER_COUNT: usize = u8::MAX as usize;
 
 /// Verifies if:
 /// * if `last_write_time` is not None if between `last_write_time` and `time_now`
-/// passed strictly more than `min_time_between_updates`.
+///     passed strictly more than `min_time_between_updates`.
 pub fn verify_write_timestamp(
     time_now: TimestampMillis,
     last_write_time: Option<TimestampMillis>,

crates/redstone/src/core/validator.rs

@@ -1,12 +1,8 @@
 use alloc::vec::Vec;
 
 use crate::{
-    core::config::Config,
-    network::error::Error,
-    protocol::constants::{MAX_TIMESTAMP_AHEAD_MS, MAX_TIMESTAMP_DELAY_MS},
-    types::Value,
-    utils::filter::FilterSome,
-    FeedId, SignerAddress, TimestampMillis,
+    core::config::Config, network::error::Error, types::Value, utils::filter::FilterSome, FeedId,
+    SignerAddress, TimestampMillis,
 };
 /// A trait defining validation operations for data feeds and signers.
 ///
@@ -122,12 +118,13 @@ impl Validator for Config {
         timestamp: TimestampMillis,
     ) -> Result<TimestampMillis, Error> {
         if !timestamp
-            .add(MAX_TIMESTAMP_DELAY_MS)
+            .add(*self.max_timestamp_delay_ms())
             .is_same_or_after(*self.block_timestamp())
         {
             return Err(Error::TimestampTooOld(index, timestamp));
         }
-        if !timestamp.is_same_or_before(self.block_timestamp().add(MAX_TIMESTAMP_AHEAD_MS)) {
+        if !timestamp.is_same_or_before(self.block_timestamp().add(*self.max_timestamp_ahead_ms()))
+        {
             return Err(Error::TimestampTooFuture(index, timestamp));
         }
 

crates/redstone/src/crypto/mod.rs

@@ -12,18 +12,22 @@ const ECDSA_N_DIV_2: U256 = U256([
     9223372036854775807,
 ]);
 
+const SIGNATURE_SIZE_BS: usize = 65;
+
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub enum CryptoError {
     RecoveryByte(u8),
     Signature(Vec<u8>),
     RecoverPreHash,
+    InvalidSignatureLen(usize),
 }
 impl CryptoError {
     pub fn code(&self) -> u16 {
         match self {
             CryptoError::RecoveryByte(byte) => *byte as u16,
             CryptoError::Signature(vec) => vec.len() as u16,
             CryptoError::RecoverPreHash => 0,
+            CryptoError::InvalidSignatureLen(len) => *len as u16,
         }
     }
 }
@@ -43,12 +47,16 @@ pub trait Crypto {
         message: A,
         signature: B,
     ) -> Result<SignerAddress, CryptoError> {
-        check_signature_malleability(signature.as_ref())?;
-        let recovery_byte = signature.as_ref()[64]; // 65-byte representation
+        let signature = signature.as_ref();
+        if signature.len() != SIGNATURE_SIZE_BS {
+            return Err(CryptoError::InvalidSignatureLen(signature.len()));
+        }
+        check_signature_malleability(signature)?;
+        let recovery_byte = signature[64]; // 65-byte representation
         let msg_hash = Self::keccak256(message);
         let key = Self::recover_public_key(
             recovery_byte - (if recovery_byte >= 27 { 27 } else { 0 }),
-            &signature.as_ref()[..64],
+            &signature[..64],
             msg_hash,
         )?;
         let key_hash = Self::keccak256(&key.as_ref()[1..]); // skip first uncompressed-key byte
@@ -69,7 +77,7 @@ fn check_signature_malleability(sig: &[u8]) -> Result<(), CryptoError> {
 #[cfg(test)]
 #[allow(dead_code)] // this is test template for crypto implementations
 pub mod recovery_key_tests {
-    use alloc::borrow::ToOwned;
+    use alloc::{borrow::ToOwned, string::ToString};
 
     use crate::{helpers::hex::hex_to_bytes, Crypto, CryptoError};
 
@@ -145,13 +153,13 @@ pub mod recovery_key_tests {
         T: Crypto<KeccakOutput = [u8; 32]>,
     {
         let msg =
-        b"4254430000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000058f32c910a001924dc0bd5000000020000001";
+        hex_to_bytes("4254430000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000058f32c910a001924dc0bd5000000020000001".to_string());
 
         let signature =
-        b"6307247862e106f0d4b3cde75805ababa67325953145aa05bdd219d90a741e0eeba79b756bf3af6db6c26a8ed3810e3c584379476fd83096218e9deb95a7617e1b";
+        hex_to_bytes("6307247862e106f0d4b3cde75805ababa67325953145aa05bdd219d90a741e0eeba79b756bf3af6db6c26a8ed3810e3c584379476fd83096218e9deb95a7617e1b".to_string());
 
-        let result = T::recover_address(msg, signature);
-        assert_eq!(result, Err(CryptoError::RecoveryByte(74)));
+        let result = T::recover_address(&msg, &signature);
+        assert_eq!(result, Err(CryptoError::Signature(signature)));
     }
 
     fn u8_slice<const N: usize>(str: &str) -> [u8; N] {

crates/redstone/src/network/error.rs

@@ -1,5 +1,8 @@
 use alloc::{string::String, vec::Vec};
-use core::fmt::{Debug, Display, Formatter};
+use core::{
+    fmt::{Debug, Display, Formatter},
+    num::TryFromIntError,
+};
 
 use crate::{
     network::as_str::{AsAsciiStr, AsHexStr},
@@ -131,6 +134,12 @@ pub enum Error {
     ///
     /// Includes the value of a current update timestamp and the last update timestamp.
     CurrentTimestampMustBeGreaterThanLatestUpdateTimestamp(TimestampMillis, TimestampMillis),
+
+    /// Indicates error while converting from one type of the integer to the other.
+    NumberConversionFail,
+
+    /// Indicates error of usize overflow.
+    UsizeOverflow,
 }
 
 impl From<CryptoError> for Error {
@@ -139,6 +148,12 @@ impl From<CryptoError> for Error {
     }
 }
 
+impl From<TryFromIntError> for Error {
+    fn from(_: TryFromIntError) -> Self {
+        Self::NumberConversionFail
+    }
+}
+
 impl Error {
     pub fn code(&self) -> u16 {
         match self {
@@ -163,6 +178,8 @@ impl Error {
             Error::TimestampTooFuture(data_package_index, _) => 1050 + *data_package_index as u16,
             Error::DataTimestampMustBeGreaterThanBefore(_, _) => 1101,
             Error::CurrentTimestampMustBeGreaterThanLatestUpdateTimestamp(_, _) => 1102,
+            Error::NumberConversionFail => 1200,
+            Error::UsizeOverflow => 1300,
         }
     }
 }
@@ -233,16 +250,20 @@ impl Display for Error {
             ),
             Error::DataTimestampMustBeGreaterThanBefore(current, before) => {
                 write!(
-                    f,
-                    "Package timestamp: {current:?} must be greater than package timestamp before: {before:?}"
-                )
+                            f,
+                            "Package timestamp: {current:?} must be greater than package timestamp before: {before:?}"
+                        )
             }
             Error::CurrentTimestampMustBeGreaterThanLatestUpdateTimestamp(current, last) => {
                 write!(
-                    f,
-                    "Current update timestamp: {current:?} must be greater than latest update timestamp: {last:?}"
-                )
+                            f,
+                            "Current update timestamp: {current:?} must be greater than latest update timestamp: {last:?}"
+                        )
+            }
+            Error::NumberConversionFail => {
+                write!(f, "Number conversion failed")
             }
+            Error::UsizeOverflow => write!(f, "Usize overflow"),
         }
     }
 }

crates/redstone/src/protocol/payload.rs

@@ -1,8 +1,9 @@
+use alloc::vec::Vec;
+
 use crate::{
     core::validator::Validator, network::error::Error, protocol::data_package::DataPackage,
     TimestampMillis,
 };
-use alloc::vec::Vec;
 
 #[derive(Clone, Debug)]
 pub struct Payload {
@@ -14,7 +15,7 @@ impl Payload {
         &self,
         validator: &impl Validator,
     ) -> Result<TimestampMillis, Error> {
-        let Some(first_package) = self.data_packages.get(0) else {
+        let Some(first_package) = self.data_packages.first() else {
             return Err(Error::ArrayIsEmpty);
         };
 

crates/redstone/src/protocol/payload_decoder.rs

@@ -1,4 +1,5 @@
 use alloc::vec::Vec;
+use core::convert::TryInto;
 use core::marker::PhantomData;
 
 use crate::{
@@ -41,10 +42,12 @@ impl<Env: Environment, C: Crypto> PayloadDecoder<Env, C> {
     }
 
     fn trim_metadata(payload: &mut Vec<u8>) -> Result<usize, Error> {
-        let unsigned_metadata_size = payload.try_trim_end(UNSIGNED_METADATA_BYTE_SIZE_BS)?;
+        let unsigned_metadata_size = payload
+            .try_trim_end(UNSIGNED_METADATA_BYTE_SIZE_BS)?
+            .try_into()?;
         let _: Vec<u8> = payload.trim_end(unsigned_metadata_size);
 
-        let data_package_count = payload.try_trim_end(DATA_PACKAGES_COUNT_BS)?;
+        let data_package_count = payload.try_trim_end(DATA_PACKAGES_COUNT_BS)?.try_into()?;
 
         Ok(data_package_count)
     }
@@ -67,15 +70,21 @@ impl<Env: Environment, C: Crypto> PayloadDecoder<Env, C> {
         let data_point_count = payload.try_trim_end(DATA_POINTS_COUNT_BS)?;
         let value_size = payload.try_trim_end(DATA_POINT_VALUE_BYTE_SIZE_BS)?;
         let timestamp = payload.try_trim_end(TIMESTAMP_BS)?;
-        let size = data_point_count * (value_size + DATA_FEED_ID_BS)
-            + DATA_POINT_VALUE_BYTE_SIZE_BS
-            + TIMESTAMP_BS
-            + DATA_POINTS_COUNT_BS;
 
-        let signable_bytes: Vec<_> = tmp.trim_end(size);
+        let size: u64 = data_point_count
+            * (value_size + TryInto::<u64>::try_into(DATA_FEED_ID_BS)?)
+            + TryInto::<u64>::try_into(DATA_POINT_VALUE_BYTE_SIZE_BS)?
+            + TryInto::<u64>::try_into(TIMESTAMP_BS)?
+            + TryInto::<u64>::try_into(DATA_POINTS_COUNT_BS)?;
+
+        let signable_bytes: Vec<_> = tmp.trim_end(size.try_into()?);
         let signer_address = C::recover_address(signable_bytes, signature)?;
 
-        let data_points = Self::trim_data_points(payload, data_point_count, value_size)?;
+        let data_points = Self::trim_data_points(
+            payload,
+            data_point_count.try_into()?,
+            value_size.try_into()?,
+        )?;
 
         Ok(DataPackage {
             data_points,
@@ -268,7 +277,7 @@ mod tests {
         }
     }
 
-    #[should_panic(expected = "range end index 64 out of range for slice of length 0")]
+    #[should_panic(expected = "CryptographicError(InvalidSignatureLen(0))")]
     #[test]
     fn test_trim_data_packages_bigger_number() {
         test_trim_data_packages_of(3, "");

crates/redstone/src/utils/trim.rs

@@ -1,6 +1,6 @@
 use alloc::vec::Vec;
 
-use crate::{network::error::Error, FeedId, Value};
+use crate::{network::error::Error, FeedId};
 
 pub trait Trim<T>
 where
@@ -34,15 +34,6 @@ impl Trim<FeedId> for Vec<u8> {
     }
 }
 
-impl TryTrim<usize> for Vec<u8> {
-    fn try_trim_end(&mut self, len: usize) -> Result<usize, Error> {
-        let y: u64 = self.try_trim_end(len)?;
-
-        y.try_into()
-            .map_err(|_| Error::NumberOverflow(Value::from(y)))
-    }
-}
-
 impl TryTrim<u64> for Vec<u8> {
     fn try_trim_end(&mut self, len: usize) -> Result<u64, Error> {
         let y: Vec<u8> = self.trim_end(len);
@@ -86,12 +77,6 @@ mod tests {
 
         let (_, result) = test_try_trim_end(3);
         assert_eq!(result, Ok(256u64.pow(2) * 30));
-
-        let (_, result) = test_try_trim_end(3);
-        assert_eq!(result, Ok(256usize.pow(2) * 30));
-
-        let (_, result): (_, Vec<u8>) = test_trim_end(3);
-        assert_eq!(result.as_slice(), &REDSTONE_MARKER[6..]);
     }
 
     #[test]
@@ -101,10 +86,10 @@ mod tests {
         assert_eq!(rest.as_slice(), &REDSTONE_MARKER);
 
         let (_, result) = test_try_trim_end(0);
-        assert_eq!(result, Ok(0_usize));
+        assert_eq!(result, Ok(0));
 
         let (_, result) = test_try_trim_end(0);
-        assert_eq!(result, Ok(0_usize));
+        assert_eq!(result, Ok(0));
 
         let (_, result): (_, Vec<u8>) = test_trim_end(0);
         assert_eq!(result, Vec::<u8>::new());
@@ -132,7 +117,7 @@ mod tests {
         #[cfg(not(target_arch = "wasm32"))]
         {
             let (_, result) = test_try_trim_end(size);
-            assert_eq!(result, Ok(823907890102272usize));
+            assert_eq!(result, Ok(823907890102272));
         }
 
         let (_rest, result): (_, Vec<u8>) = test_trim_end(size);
@@ -150,17 +135,6 @@ mod tests {
         assert_eq!(x, 18446744073709551615);
     }
 
-    #[cfg(target_arch = "wasm32")]
-    #[test]
-    fn test_trim_end_u64_overflow_usize_wasm32() {
-        let (_, output): (_, Result<usize, _>) = test_try_trim_end(REDSTONE_MARKER_BS);
-
-        assert_eq!(
-            output,
-            Err(Error::NumberOverflow(823907890102272_u64.into()))
-        );
-    }
-
     #[test]
     fn test_trim_end_u64_overflow_u64() {
         let mut bytes = vec![1u8, 2, 3, 4, 5, 6, 7, 8, 9];