feat: limit open-ended vectors (#6473)

Description
---
Limited open-ended vectors to guard against malicious network messages
in:
- ProofOfWork::pow_data
- OutputFeatures::coinbase_extra
- BlockTemplateData::aux_chain_hashes
- Controller::current_blob

Motivation and Context
---
This is a _defense-in-depth_ exercise.

How Has This Been Tested?
---
Existing unit tests pass.

What process can a PR reviewer use to test or verify this change?
---
Code review

<!-- Checklist -->
<!-- 1. Is the title of your PR in the form that would make nice release
notes? The title, excluding the conventional commit
tag, will be included exactly as is in the CHANGELOG, so please think
about it carefully. -->


Breaking Changes
---

- [ ] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [x] Other - Please specify

<!-- Does this include a breaking change? If so, include this line as a
footer -->
Serialization will not be compatible

applications/minotari_app_grpc/src/conversions/block_header.rs

@@ -47,7 +47,7 @@ impl From<BlockHeader> for grpc::BlockHeader {
             nonce: h.nonce,
             pow: Some(grpc::ProofOfWork {
                 pow_algo: pow_algo.as_u64(),
-                pow_data: h.pow.pow_data,
+                pow_data: h.pow.pow_data.to_vec(),
             }),
             validator_node_mr: h.validator_node_mr.to_vec(),
             validator_node_size: h.validator_node_size,

applications/minotari_app_grpc/src/conversions/new_block_template.rs

@@ -43,7 +43,7 @@ impl TryFrom<NewBlockTemplate> for grpc::NewBlockTemplate {
             total_script_offset: Vec::from(block.header.total_script_offset.as_bytes()),
             pow: Some(grpc::ProofOfWork {
                 pow_algo: block.header.pow.pow_algo.as_u64(),
-                pow_data: block.header.pow.pow_data,
+                pow_data: block.header.pow.pow_data.to_vec(),
             }),
         };
         Ok(Self {

applications/minotari_app_grpc/src/conversions/output_features.rs

@@ -23,12 +23,14 @@
 use std::convert::{TryFrom, TryInto};
 
 use tari_core::transactions::transaction_components::{
+    CoinBaseExtra,
     OutputFeatures,
     OutputFeaturesVersion,
     OutputType,
     RangeProofType,
     SideChainFeature,
 };
+use tari_utilities::ByteArray;
 
 use crate::tari_rpc as grpc;
 
@@ -58,7 +60,7 @@ impl TryFrom<grpc::OutputFeatures> for OutputFeatures {
             )?,
             OutputType::from_byte(output_type).ok_or_else(|| "Invalid or unrecognised output type".to_string())?,
             features.maturity,
-            features.coinbase_extra,
+            CoinBaseExtra::try_from(features.coinbase_extra).map_err(|e| e.to_string())?,
             sidechain_feature,
             RangeProofType::from_byte(range_proof_type)
                 .ok_or_else(|| "Invalid or unrecognised range proof type".to_string())?,
@@ -72,7 +74,7 @@ impl From<OutputFeatures> for grpc::OutputFeatures {
             version: features.version as u32,
             output_type: u32::from(features.output_type.as_byte()),
             maturity: features.maturity,
-            coinbase_extra: features.coinbase_extra,
+            coinbase_extra: features.coinbase_extra.to_vec(),
             sidechain_feature: features.sidechain_feature.map(Into::into),
             range_proof_type: u32::from(features.range_proof_type.as_byte()),
         }

applications/minotari_app_grpc/src/conversions/proof_of_work.rs

@@ -22,7 +22,7 @@
 
 use std::convert::TryFrom;
 
-use tari_core::proof_of_work::{PowAlgorithm, ProofOfWork};
+use tari_core::proof_of_work::{PowAlgorithm, PowData, ProofOfWork};
 
 use crate::tari_rpc as grpc;
 
@@ -33,7 +33,7 @@ impl TryFrom<grpc::ProofOfWork> for ProofOfWork {
     fn try_from(pow: grpc::ProofOfWork) -> Result<Self, Self::Error> {
         Ok(Self {
             pow_algo: PowAlgorithm::try_from(pow.pow_algo)?,
-            pow_data: pow.pow_data,
+            pow_data: PowData::try_from(pow.pow_data).map_err(|e| e.to_string())?,
         })
     }
 }

applications/minotari_console_wallet/src/automation/commands.rs

@@ -1935,7 +1935,7 @@ fn write_utxos_to_csv_file(
             commitment.to_hex(),
             utxo.features.output_type,
             utxo.features.maturity,
-            String::from_utf8(utxo.features.coinbase_extra.clone())
+            String::from_utf8(utxo.features.coinbase_extra.to_vec())
                 .unwrap_or_else(|_| utxo.features.coinbase_extra.to_hex()),
             utxo.script.to_hex(),
             utxo.covenant.to_bytes().to_hex(),

applications/minotari_merge_mining_proxy/src/block_template_data.rs

@@ -29,7 +29,7 @@ use chrono::Duration;
 use chrono::{DateTime, Utc};
 use minotari_node_grpc_client::grpc;
 use tari_common_types::types::FixedHash;
-use tari_core::proof_of_work::monero_rx::FixedByteArray;
+use tari_core::{proof_of_work::monero_rx::FixedByteArray, AuxChainHashes};
 use tokio::sync::RwLock;
 use tracing::trace;
 
@@ -222,7 +222,7 @@ pub struct BlockTemplateData {
     pub tari_difficulty: u64,
     pub tari_merge_mining_hash: FixedHash,
     #[allow(dead_code)]
-    pub aux_chain_hashes: Vec<monero::Hash>,
+    pub aux_chain_hashes: AuxChainHashes,
     pub new_block_template: grpc::NewBlockTemplate,
 }
 
@@ -237,7 +237,7 @@ pub struct BlockTemplateDataBuilder {
     monero_difficulty: Option<u64>,
     tari_difficulty: Option<u64>,
     tari_merge_mining_hash: Option<FixedHash>,
-    aux_chain_hashes: Vec<monero::Hash>,
+    aux_chain_hashes: AuxChainHashes,
     new_block_template: Option<grpc::NewBlockTemplate>,
 }
 
@@ -276,7 +276,7 @@ impl BlockTemplateDataBuilder {
         self
     }
 
-    pub fn aux_hashes(mut self, aux_chain_hashes: Vec<monero::Hash>) -> Self {
+    pub fn aux_hashes(mut self, aux_chain_hashes: AuxChainHashes) -> Self {
         self.aux_chain_hashes = aux_chain_hashes;
         self
     }
@@ -342,6 +342,7 @@ pub mod test {
     use tari_utilities::ByteArray;
 
     use super::*;
+    use crate::block_template_protocol::AuxChainMr;
 
     fn create_block_template_data() -> FinalBlockTemplateData {
         let header = BlockHeader::new(100);
@@ -362,16 +363,16 @@ pub mod test {
             .monero_difficulty(123456)
             .tari_difficulty(12345)
             .tari_merge_mining_hash(hash)
-            .aux_hashes(vec![monero::Hash::from_slice(hash.as_slice())])
+            .aux_hashes(AuxChainHashes::try_from(vec![monero::Hash::from_slice(hash.as_slice())]).unwrap())
             .new_block_template(new_block_template);
         let block_template_data = btdb.build().unwrap();
         FinalBlockTemplateData {
             template: block_template_data,
             target_difficulty: Difficulty::from_u64(12345).unwrap(),
             blockhashing_blob: "no blockhashing_blob data".to_string(),
             blocktemplate_blob: "no blocktemplate_blob data".to_string(),
-            aux_chain_hashes: vec![monero::Hash::from_slice(hash.as_slice())],
-            aux_chain_mr: hash.to_vec(),
+            aux_chain_hashes: AuxChainHashes::try_from(vec![monero::Hash::from_slice(hash.as_slice())]).unwrap(),
+            aux_chain_mr: AuxChainMr::try_from(hash.to_vec()).unwrap(),
         }
     }
 

applications/minotari_merge_mining_proxy/src/block_template_protocol.rs

@@ -26,15 +26,16 @@ use std::{cmp, convert::TryFrom, sync::Arc};
 use log::*;
 use minotari_app_utilities::parse_miner_input::BaseNodeGrpcClient;
 use minotari_node_grpc_client::grpc;
-use tari_common_types::{tari_address::TariAddress, types::FixedHash};
+use tari_common_types::{tari_address::TariAddress, types::FixedHash, MaxSizeBytes};
 use tari_core::{
     consensus::ConsensusManager,
     proof_of_work::{monero_rx, monero_rx::FixedByteArray, Difficulty},
     transactions::{
         generate_coinbase,
         key_manager::{create_memory_db_key_manager, MemoryDbKeyManager},
-        transaction_components::{encrypted_data::PaymentId, TransactionKernel, TransactionOutput},
+        transaction_components::{encrypted_data::PaymentId, CoinBaseExtra, TransactionKernel, TransactionOutput},
     },
+    AuxChainHashes,
 };
 use tari_utilities::{hex::Hex, ByteArray};
 
@@ -209,7 +210,7 @@ impl BlockTemplateProtocol<'_> {
                 .save_final_block_template_if_key_unique(
                     // `aux_chain_mr` is used as the key because it is stored in the ExtraData field in the Monero
                     // block
-                    final_template_data.aux_chain_mr.clone(),
+                    final_template_data.aux_chain_mr.to_vec(),
                     final_template_data.clone(),
                 )
                 .await;
@@ -328,7 +329,7 @@ impl BlockTemplateProtocol<'_> {
             total_fees.into(),
             block_reward.into(),
             tari_height,
-            self.config.coinbase_extra.as_bytes(),
+            &CoinBaseExtra::try_from(self.config.coinbase_extra.as_bytes().to_vec())?,
             &self.key_manager,
             &self.wallet_payment_address,
             true,
@@ -358,7 +359,7 @@ impl BlockTemplateProtocol<'_> {
 
 /// This is an interim solution to calculate the merkle root for the aux chains when multiple aux chains will be
 /// merge mined with Monero. It needs to be replaced with a more general solution in the future.
-pub fn calculate_aux_chain_merkle_root(hashes: Vec<monero::Hash>) -> Result<(monero::Hash, u32), MmProxyError> {
+pub fn calculate_aux_chain_merkle_root(hashes: AuxChainHashes) -> Result<(monero::Hash, u32), MmProxyError> {
     if hashes.is_empty() {
         Err(MmProxyError::MissingDataError(
             "No aux chain hashes provided".to_string(),
@@ -380,7 +381,7 @@ fn add_monero_data(
     let merge_mining_hash = FixedHash::try_from(tari_block_result.merge_mining_hash.clone())
         .map_err(|e| MmProxyError::ConversionError(e.to_string()))?;
 
-    let aux_chain_hashes = vec![monero::Hash::from_slice(merge_mining_hash.as_slice())];
+    let aux_chain_hashes = AuxChainHashes::try_from(vec![monero::Hash::from_slice(merge_mining_hash.as_slice())])?;
     let tari_difficulty = template_data.miner_data.target_difficulty;
     let block_template_data = BlockTemplateDataBuilder::new()
         .tari_block(
@@ -426,7 +427,8 @@ fn add_monero_data(
         blockhashing_blob,
         blocktemplate_blob,
         aux_chain_hashes,
-        aux_chain_mr: aux_chain_mr.to_bytes().to_vec(),
+        aux_chain_mr: AuxChainMr::try_from(aux_chain_mr.to_bytes().to_vec())
+            .map_err(|e| MmProxyError::ConversionError(e.to_string()))?,
     })
 }
 
@@ -443,15 +445,17 @@ impl NewBlockTemplateData {
     }
 }
 
+/// The AuxChainMerkleRoot is a 32 byte hash
+pub type AuxChainMr = MaxSizeBytes<32>;
 /// Final outputs for required for merge mining
 #[derive(Debug, Clone)]
 pub struct FinalBlockTemplateData {
     pub template: BlockTemplateData,
     pub target_difficulty: Difficulty,
     pub blockhashing_blob: String,
     pub blocktemplate_blob: String,
-    pub aux_chain_hashes: Vec<monero::Hash>,
-    pub aux_chain_mr: Vec<u8>,
+    pub aux_chain_hashes: AuxChainHashes,
+    pub aux_chain_mr: AuxChainMr,
 }
 
 /// Container struct for monero mining data inputs obtained from monerod

applications/minotari_merge_mining_proxy/src/error.rs

@@ -29,6 +29,7 @@ use hyper::header::InvalidHeaderValue;
 use minotari_app_utilities::parse_miner_input::ParseInputError;
 use minotari_wallet_grpc_client::BasicAuthError;
 use tari_common::{ConfigError, ConfigurationError};
+use tari_common_types::{MaxSizeBytesError, MaxSizeVecError};
 use tari_core::{
     consensus::ConsensusBuilderError,
     proof_of_work::{monero_rx::MergeMineError, DifficultyError},
@@ -117,6 +118,10 @@ pub enum MmProxyError {
     UnexpectedMissingData(String),
     #[error("Failed to get block template: {0}")]
     FailedToGetBlockTemplate(String),
+    #[error("Max sized vector error: {0}")]
+    MaxSizeBytesError(#[from] MaxSizeBytesError),
+    #[error("Max sized vector error: {0}")]
+    MaxSizeVecError(#[from] MaxSizeVecError),
 }
 
 impl From<tonic::Status> for MmProxyError {

applications/minotari_miner/src/errors.rs

@@ -21,6 +21,7 @@
 //  USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 use minotari_app_grpc::authentication::BasicAuthError;
 use minotari_app_utilities::parse_miner_input::ParseInputError;
+use tari_common_types::MaxSizeBytesError;
 use thiserror::Error;
 use tonic::codegen::http::uri::InvalidUri;
 
@@ -56,6 +57,8 @@ pub enum MinerError {
     ParseInputError(#[from] ParseInputError),
     #[error("Base node not responding to gRPC requests: {0}")]
     BaseNodeNotResponding(String),
+    #[error("Limit error {0}")]
+    MaxSizeBytesError(#[from] MaxSizeBytesError),
 }
 
 pub fn err_empty(name: &str) -> MinerError {

applications/minotari_miner/src/run_miner.rs

@@ -57,7 +57,7 @@ use tari_core::{
         generate_coinbase,
         key_manager::{create_memory_db_key_manager, MemoryDbKeyManager},
         tari_amount::MicroMinotari,
-        transaction_components::encrypted_data::PaymentId,
+        transaction_components::{encrypted_data::PaymentId, CoinBaseExtra},
     },
 };
 use tari_crypto::ristretto::RistrettoPublicKey;
@@ -422,7 +422,7 @@ async fn get_new_block_base_node(
         fee,
         reward,
         height,
-        config.coinbase_extra.as_bytes(),
+        &CoinBaseExtra::try_from(config.coinbase_extra.as_bytes().to_vec())?,
         key_manager,
         wallet_payment_address,
         true,

applications/minotari_miner/src/stratum/error.rs

@@ -20,8 +20,10 @@
 //  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_common_types::MaxSizeBytesError;
 use thiserror::Error;
 
+#[allow(clippy::enum_variant_names)]
 #[derive(Debug, Error)]
 pub enum Error {
     #[error("Connection error: {0}")]
@@ -47,11 +49,12 @@ pub enum Error {
     NotConnected,
     #[error("Can't parse int: {0}")]
     Parse(#[from] std::num::ParseIntError),
-
     #[error("General error: {0}")]
     General(String),
     #[error("Missing Data error: {0}")]
     MissingData(String),
+    #[error("Limit exceeded error: {0}")]
+    MaxSizeBytesError(#[from] MaxSizeBytesError),
 }
 
 impl<T> From<std::sync::PoisonError<T>> for Error {

applications/minotari_miner/src/stratum/stratum_controller/controller.rs

@@ -25,7 +25,8 @@ use borsh::BorshDeserialize;
 use futures::stream::StreamExt;
 use log::*;
 use minotari_app_grpc::tari_rpc::BlockHeader;
-use tari_utilities::hex::Hex;
+use tari_common_types::MaxSizeBytes;
+use tari_utilities::{hex::Hex, ByteArray};
 
 use crate::{
     miner::Miner,
@@ -36,14 +37,16 @@ use crate::{
 pub const LOG_TARGET: &str = "minotari::miner::stratum::controller";
 pub const LOG_TARGET_FILE: &str = "minotari::logging::miner::stratum::controller";
 
+type CurrentBlob = MaxSizeBytes<{ 4 * 1024 * 1024 }>; // 4 MiB
+
 pub struct Controller {
     rx: mpsc::Receiver<types::miner_message::MinerMessage>,
     pub tx: mpsc::Sender<types::miner_message::MinerMessage>,
     client_tx: Option<mpsc::Sender<types::client_message::ClientMessage>>,
     current_height: u64,
     current_job_id: u64,
     current_difficulty_target: u64,
-    current_blob: Vec<u8>,
+    current_blob: CurrentBlob,
     current_header: Option<BlockHeader>,
     keep_alive_time: SystemTime,
     num_mining_threads: usize,
@@ -59,7 +62,7 @@ impl Controller {
             current_height: 0,
             current_job_id: 0,
             current_difficulty_target: 0,
-            current_blob: Vec::new(),
+            current_blob: CurrentBlob::default(),
             current_header: None,
             keep_alive_time: SystemTime::now(),
             num_mining_threads,
@@ -79,7 +82,7 @@ impl Controller {
                 debug!(target: LOG_TARGET_FILE, "Miner received message: {:?}", message);
                 match message {
                     types::miner_message::MinerMessage::ReceivedJob(height, job_id, diff, blob) => {
-                        match self.should_we_update_job(height, job_id, diff, blob) {
+                        match self.should_we_update_job(height, job_id, diff, CurrentBlob::try_from(blob)?) {
                             Ok(should_we_update) => {
                                 if should_we_update {
                                     let header = self
@@ -190,7 +193,13 @@ impl Controller {
         }
     }
 
-    pub fn should_we_update_job(&mut self, height: u64, job_id: u64, diff: u64, blob: Vec<u8>) -> Result<bool, Error> {
+    pub fn should_we_update_job(
+        &mut self,
+        height: u64,
+        job_id: u64,
+        diff: u64,
+        blob: CurrentBlob,
+    ) -> Result<bool, Error> {
         if height != self.current_height ||
             job_id != self.current_job_id ||
             diff != self.current_difficulty_target ||
@@ -200,7 +209,7 @@ impl Controller {
             self.current_job_id = job_id;
             self.current_blob = blob.clone();
             self.current_difficulty_target = diff;
-            let mut buffer = blob.as_slice();
+            let mut buffer = blob.as_bytes();
             let tari_header: tari_core::blocks::BlockHeader = BorshDeserialize::deserialize(&mut buffer)
                 .map_err(|_| Error::General("Byte Blob is not a valid header".to_string()))?;
             self.current_header = Some(minotari_app_grpc::tari_rpc::BlockHeader::from(tari_header));