fix(core)!: fix potential panic for sidechain merkle root with incorrect length

applications/tari_app_grpc/src/conversions/output_features.rs

@@ -22,7 +22,10 @@
 
 use std::convert::{TryFrom, TryInto};
 
-use tari_common_types::types::{Commitment, PublicKey};
+use tari_common_types::{
+    array::copy_into_fixed_array,
+    types::{Commitment, PublicKey},
+};
 use tari_core::transactions::transaction::{
     AssetOutputFeatures,
     MintNonFungibleFeatures,
@@ -176,10 +179,8 @@ impl TryFrom<grpc::SideChainCheckpointFeatures> for SideChainCheckpointFeatures
                 PublicKey::from_bytes(c).map_err(|err| format!("committee member was not a valid public key: {}", err))
             })
             .collect::<Result<_, _>>()?;
+        let merkle_root = copy_into_fixed_array(&value.merkle_root).map_err(|_| "Invalid merkle_root length")?;
 
-        Ok(Self {
-            merkle_root: value.merkle_root.as_bytes().to_vec(),
-            committee,
-        })
+        Ok(Self { merkle_root, committee })
     }
 }

applications/tari_console_wallet/src/automation/commands.rs

@@ -35,7 +35,7 @@ use log::*;
 use sha2::Sha256;
 use strum_macros::{Display, EnumIter, EnumString};
 use tari_common::GlobalConfig;
-use tari_common_types::{emoji::EmojiId, transaction::TxId, types::PublicKey};
+use tari_common_types::{array::copy_into_fixed_array, emoji::EmojiId, transaction::TxId, types::PublicKey};
 use tari_comms::{
     connectivity::{ConnectivityEvent, ConnectivityRequester},
     multiaddr::Multiaddr,
@@ -818,18 +818,17 @@ pub async fn command_runner(
                     _ => Err(CommandError::Argument),
                 }?;
 
-                let unique_ids: Vec<Vec<u8>> = parsed.args[1..]
+                let unique_ids = parsed.args[1..]
                     .iter()
                     .map(|arg| {
                         let s = arg.to_string();
                         if let Some(s) = s.strip_prefix("0x") {
-                            let r: Vec<u8> = Hex::from_hex(s).unwrap();
-                            r
+                            Hex::from_hex(s).map_err(|_| CommandError::Argument)
                         } else {
-                            s.into_bytes()
+                            Ok(s.into_bytes())
                         }
                     })
-                    .collect();
+                    .collect::<Result<Vec<Vec<u8>>, _>>()?;
 
                 let mut asset_manager = wallet.asset_manager.clone();
                 let asset = asset_manager.get_owned_asset_by_pub_key(&public_key).await?;
@@ -856,18 +855,14 @@ pub async fn command_runner(
 
                 let merkle_root = match parsed.args[1] {
                     ParsedArgument::Text(ref root) => {
-                        let s = root.to_string();
-                        match &s[0..2] {
-                            "0x" => {
-                                let s = s[2..].to_string();
-                                let r: Vec<u8> = Hex::from_hex(&s).unwrap();
-                                Ok(r)
-                            },
-                            _ => Ok(s.into_bytes()),
-                        }
+                        let bytes = match &root[0..2] {
+                            "0x" => Vec::<u8>::from_hex(&root[2..]).map_err(|_| CommandError::Argument)?,
+                            _ => Vec::<u8>::from_hex(root).map_err(|_| CommandError::Argument)?,
+                        };
+                        copy_into_fixed_array(&bytes).map_err(|_| CommandError::Argument)?
                     },
-                    _ => Err(CommandError::Argument),
-                }?;
+                    _ => return Err(CommandError::Argument),
+                };
 
                 let committee: Vec<PublicKey> = parsed.args[2..]
                     .iter()
@@ -879,7 +874,7 @@ pub async fn command_runner(
 
                 let mut asset_manager = wallet.asset_manager.clone();
                 let (tx_id, transaction) = asset_manager
-                    .create_initial_asset_checkpoint(&public_key, &merkle_root, &committee)
+                    .create_initial_asset_checkpoint(&public_key, merkle_root, &committee)
                     .await?;
                 let _result = transaction_service
                     .submit_transaction(

applications/tari_console_wallet/src/grpc/wallet_grpc_server.rs

@@ -71,7 +71,10 @@ use tari_app_grpc::{
         TransferResult,
     },
 };
-use tari_common_types::types::{BlockHash, PublicKey, Signature};
+use tari_common_types::{
+    array::copy_into_fixed_array,
+    types::{BlockHash, PublicKey, Signature},
+};
 use tari_comms::{types::CommsPublicKey, CommsNode};
 use tari_core::transactions::{
     tari_amount::MicroTari,
@@ -655,12 +658,11 @@ impl wallet_server::Wallet for WalletGrpcServer {
             .collect::<Result<_, _>>()
             .map_err(|err| Status::invalid_argument(format!("Committee did not contain valid pub keys:{}", err)))?;
 
+        let merkle_root = copy_into_fixed_array(&message.merkle_root)
+            .map_err(|_| Status::invalid_argument("Merkle root has an incorrect length"))?;
+
         let (tx_id, transaction) = asset_manager
-            .create_initial_asset_checkpoint(
-                &asset_public_key,
-                message.merkle_root.as_slice(),
-                committee_public_keys.as_slice(),
-            )
+            .create_initial_asset_checkpoint(&asset_public_key, merkle_root, committee_public_keys.as_slice())
             .await
             .map_err(|e| Status::internal(e.to_string()))?;
 
@@ -691,11 +693,14 @@ impl wallet_server::Wallet for WalletGrpcServer {
                 Status::invalid_argument(format!("Next committee did not contain valid pub keys:{}", err))
             })?;
 
+        let merkle_root = copy_into_fixed_array(&message.merkle_root)
+            .map_err(|_| Status::invalid_argument("Incorrect merkle root length"))?;
+
         let (tx_id, transaction) = asset_manager
             .create_follow_on_asset_checkpoint(
                 &asset_public_key,
                 message.unique_id.as_slice(),
-                message.merkle_root.as_slice(),
+                merkle_root,
                 committee_public_keys.as_slice(),
             )
             .await

base_layer/common_types/Cargo.toml

@@ -7,10 +7,12 @@ version = "0.27.3"
 edition = "2018"
 
 [dependencies]
-rand = "0.8"
 tari_crypto = { git = "https://github.com/tari-project/tari-crypto.git", branch = "main" }
-serde = { version = "1.0.106", features = ["derive"] }
-tokio = { version = "1.11", features = ["time", "sync"] }
-lazy_static = "1.4.0"
+tari_utilities = "^0.3"
+
 digest = "0.9.0"
+lazy_static = "1.4.0"
+rand = "0.8"
+serde = { version = "1.0.106", features = ["derive"] }
 thiserror = "1.0.29"
+tokio = { version = "1.11", features = ["time", "sync"] }

base_layer/common_types/src/array.rs

@@ -0,0 +1,32 @@
+//  Copyright 2022, The Tari Project
+//
+//  Redistribution and use in source and binary forms, with or without modification, are permitted provided that the
+//  following conditions are met:
+//
+//  1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following
+//  disclaimer.
+//
+//  2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the
+//  following disclaimer in the documentation and/or other materials provided with the distribution.
+//
+//  3. Neither the name of the copyright holder nor the names of its contributors may be used to endorse or promote
+//  products derived from this software without specific prior written permission.
+//
+//  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+//  INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+//  DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+//  SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+//  SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
+//  WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
+//  USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+use tari_utilities::ByteArrayError;
+
+pub fn copy_into_fixed_array<T: Default + Copy, const SZ: usize>(elems: &[T]) -> Result<[T; SZ], ByteArrayError> {
+    if elems.len() != SZ {
+        return Err(ByteArrayError::IncorrectLength);
+    }
+    let mut buf = [T::default(); SZ];
+    buf.copy_from_slice(&elems[0..SZ]);
+    Ok(buf)
+}

base_layer/common_types/src/lib.rs

@@ -20,6 +20,7 @@
 // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
 // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+pub mod array;
 pub mod chain_metadata;
 pub mod emoji;
 pub mod luhn;

base_layer/common_types/src/types/mod.rs

@@ -38,6 +38,8 @@ use tari_crypto::{
 pub const BLOCK_HASH_LENGTH: usize = 32;
 pub type BlockHash = Vec<u8>;
 
+pub type FixedHash = [u8; BLOCK_HASH_LENGTH];
+
 /// Define the explicit Signature implementation for the Tari base layer. A different signature scheme can be
 /// employed by redefining this type.
 pub type Signature = RistrettoSchnorr;

base_layer/core/src/proto/transaction.rs

@@ -27,7 +27,7 @@ use std::{
     sync::Arc,
 };
 
-use tari_common_types::types::{BlindingFactor, BulletRangeProof, Commitment, PublicKey};
+use tari_common_types::types::{BlindingFactor, BulletRangeProof, Commitment, PublicKey, BLOCK_HASH_LENGTH};
 use tari_crypto::{
     script::{ExecutionStack, TariScript},
     tari_utilities::{ByteArray, ByteArrayError},
@@ -400,7 +400,14 @@ impl TryFrom<proto::types::SideChainCheckpointFeatures> for SideChainCheckpointF
     type Error = String;
 
     fn try_from(value: proto::types::SideChainCheckpointFeatures) -> Result<Self, Self::Error> {
-        let merkle_root = value.merkle_root.as_bytes().to_vec();
+        if value.merkle_root.len() != BLOCK_HASH_LENGTH {
+            return Err(format!(
+                "Invalid side chain checkpoint merkle length {}",
+                value.merkle_root.len()
+            ));
+        }
+        let mut merkle_root = [0u8; BLOCK_HASH_LENGTH];
+        merkle_root.copy_from_slice(&value.merkle_root[0..BLOCK_HASH_LENGTH]);
         let committee = value
             .committee
             .into_iter()

base_layer/core/src/transactions/transaction/output_features.rs

@@ -29,7 +29,7 @@ use std::{
 };
 
 use serde::{Deserialize, Serialize};
-use tari_common_types::types::{Commitment, PublicKey};
+use tari_common_types::types::{Commitment, FixedHash, PublicKey};
 use tari_utilities::ByteArray;
 
 use super::OutputFeaturesVersion;
@@ -181,7 +181,7 @@ impl OutputFeatures {
     pub fn for_checkpoint(
         parent_public_key: PublicKey,
         unique_id: Vec<u8>,
-        merkle_root: Vec<u8>,
+        merkle_root: FixedHash,
         committee: Vec<PublicKey>,
         is_initial: bool,
     ) -> OutputFeatures {
@@ -317,7 +317,7 @@ mod test {
                 asset_owner_commitment: Default::default(),
             }),
             sidechain_checkpoint: Some(SideChainCheckpointFeatures {
-                merkle_root: vec![1u8; 32],
+                merkle_root: [1u8; 32],
                 committee: iter::repeat_with(PublicKey::default).take(50).collect(),
             }),
         }

base_layer/core/src/transactions/transaction/side_chain_checkpoint_features.rs

@@ -20,28 +20,24 @@
 //  WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
 //  USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-use std::io::{Error, ErrorKind, Read, Write};
+use std::io::{Error, Read, Write};
 
 use integer_encoding::VarInt;
 use serde::{Deserialize, Serialize};
-use tari_common_types::types::PublicKey;
+use tari_common_types::types::{FixedHash, PublicKey};
 use tari_crypto::keys::PublicKey as PublicKeyTrait;
 
 use crate::consensus::{ConsensusDecoding, ConsensusEncoding, ConsensusEncodingSized, MaxSizeVec};
 
 #[derive(Debug, Clone, Hash, PartialEq, Deserialize, Serialize, Eq)]
 pub struct SideChainCheckpointFeatures {
-    // TODO: This should be fixed size [u8;32]
-    pub merkle_root: Vec<u8>,
+    pub merkle_root: FixedHash,
     pub committee: Vec<PublicKey>,
 }
 
 impl ConsensusEncoding for SideChainCheckpointFeatures {
     fn consensus_encode<W: Write>(&self, writer: &mut W) -> Result<usize, Error> {
-        if self.merkle_root.len() != 32 {
-            return Err(Error::new(ErrorKind::InvalidInput, "merkle_root must be 32 bytes"));
-        }
-        writer.write_all(&self.merkle_root[0..32])?;
+        self.merkle_root.consensus_encode(writer)?;
         let mut written = 32;
         written += self.committee.consensus_encode(writer)?;
         Ok(written)
@@ -56,7 +52,7 @@ impl ConsensusEncodingSized for SideChainCheckpointFeatures {
 
 impl ConsensusDecoding for SideChainCheckpointFeatures {
     fn consensus_decode<R: Read>(reader: &mut R) -> Result<Self, Error> {
-        let merkle_root = <[u8; 32] as ConsensusDecoding>::consensus_decode(reader)?.to_vec();
+        let merkle_root = FixedHash::consensus_decode(reader)?;
 
         const MAX_COMMITTEE_KEYS: usize = 50;
         let committee = MaxSizeVec::<PublicKey, MAX_COMMITTEE_KEYS>::consensus_decode(reader)?;
@@ -78,7 +74,7 @@ mod test {
     #[test]
     fn it_encodes_and_decodes_correctly() {
         let subject = SideChainCheckpointFeatures {
-            merkle_root: vec![1u8; 32],
+            merkle_root: [1u8; 32],
             committee: iter::repeat_with(PublicKey::default).take(50).collect(),
         };
 
@@ -88,22 +84,11 @@ mod test {
     #[test]
     fn it_fails_for_too_many_committee_pks() {
         let subject = SideChainCheckpointFeatures {
-            merkle_root: vec![1u8; 32],
+            merkle_root: [1u8; 32],
             committee: iter::repeat_with(PublicKey::default).take(51).collect(),
         };
 
         let err = check_consensus_encoding_correctness(subject).unwrap_err();
         assert_eq!(err.kind(), ErrorKind::InvalidInput);
     }
-
-    #[test]
-    fn it_fails_for_incorrect_merkle_root_length() {
-        let subject = SideChainCheckpointFeatures {
-            merkle_root: vec![1u8; 31],
-            committee: vec![],
-        };
-
-        let err = check_consensus_encoding_correctness(subject).unwrap_err();
-        assert_eq!(err.kind(), ErrorKind::InvalidInput);
-    }
 }

base_layer/wallet/src/assets/asset_manager.rs

@@ -23,7 +23,7 @@
 use log::*;
 use tari_common_types::{
     transaction::TxId,
-    types::{Commitment, PublicKey},
+    types::{Commitment, FixedHash, PublicKey},
 };
 use tari_core::transactions::transaction::{OutputFeatures, OutputFlags, TemplateParameter, Transaction};
 
@@ -151,7 +151,7 @@ impl<T: OutputManagerBackend + 'static> AssetManager<T> {
     pub async fn create_initial_asset_checkpoint(
         &mut self,
         asset_pub_key: PublicKey,
-        merkle_root: Vec<u8>,
+        merkle_root: FixedHash,
         committee_pub_keys: Vec<PublicKey>,
     ) -> Result<(TxId, Transaction), WalletError> {
         let output = self
@@ -199,7 +199,7 @@ impl<T: OutputManagerBackend + 'static> AssetManager<T> {
         &mut self,
         asset_pub_key: PublicKey,
         unique_id: Vec<u8>,
-        merkle_root: Vec<u8>,
+        merkle_root: FixedHash,
         committee_pub_keys: Vec<PublicKey>,
     ) -> Result<(TxId, Transaction), WalletError> {
         let output = self