Skip to content

Commit

Permalink
Limit open-ended vectors
Browse files Browse the repository at this point in the history
Limited open-ended vectors to guard against malicious network messages in:
- ProofOfWork::pow_data
- OutputFeatures::coinbase_extra
- BlockTemplateData::aux_chain_hashes
- Controller::current_blob
  • Loading branch information
hansieodendaal committed Aug 17, 2024
1 parent a67b79d commit e56eace
Show file tree
Hide file tree
Showing 54 changed files with 1,095 additions and 832 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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())?,
Expand All @@ -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()),
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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())?,
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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,
}

Expand All @@ -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>,
}

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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);
Expand All @@ -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(),
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand All @@ -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(
Expand Down Expand Up @@ -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()))?,
})
}

Expand All @@ -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
Expand Down
5 changes: 5 additions & 0 deletions applications/minotari_merge_mining_proxy/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions applications/minotari_miner/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions applications/minotari_miner/src/run_miner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 4 additions & 1 deletion applications/minotari_miner/src/stratum/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}")]
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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 ||
Expand All @@ -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));
Expand Down
Loading

0 comments on commit e56eace

Please sign in to comment.