Skip to content

Commit

Permalink
feat: limit open-ended vectors for tari script (#6501)
Browse files Browse the repository at this point in the history
Description
---
Limited open-ended vectors to guard against malicious network messages
in:
- tari-script

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
---

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

<!-- Does this include a breaking change? If so, include this line as a
footer -->
<!-- BREAKING CHANGE: Description what the user should do, e.g. delete a
database, resync the chain -->

---------

Co-authored-by: SW van Heerden <swvheerden@gmail.com>
  • Loading branch information
hansieodendaal and SWvheerden committed Aug 26, 2024
1 parent 444b5a3 commit f91cffa
Show file tree
Hide file tree
Showing 78 changed files with 410 additions and 297 deletions.
19 changes: 19 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions applications/minotari_app_grpc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ tari_comms = { path = "../../comms/core" }
tari_core = { path = "../../base_layer/core" }
tari_crypto = { version = "0.20.3" }
tari_script = { path = "../../infrastructure/tari_script" }
tari_max_size = { path = "../../infrastructure/max_size" }
tari_utilities = { version = "0.7" }

argon2 = { version = "0.4.1", features = ["std", "password-hash"] }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@

use std::convert::{TryFrom, TryInto};

use tari_common_types::{
types::{PublicKey, Signature},
MaxSizeString,
};
use tari_common_types::types::{PublicKey, Signature};
use tari_core::transactions::transaction_components::{
BuildInfo,
CodeTemplateRegistration,
Expand All @@ -35,6 +32,7 @@ use tari_core::transactions::transaction_components::{
ValidatorNodeRegistration,
ValidatorNodeSignature,
};
use tari_max_size::MaxSizeString;
use tari_utilities::ByteArray;

use crate::tari_rpc as grpc;
Expand Down
1 change: 1 addition & 0 deletions applications/minotari_console_wallet/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ tari_contacts = { path = "../../base_layer/contacts" }
tari_crypto = { version = "0.20.3" }
tari_key_manager = { path = "../../base_layer/key_manager" }
tari_libtor = { path = "../../infrastructure/libtor", optional = true }
tari_max_size = { path = "../../infrastructure/max_size" }
tari_p2p = { path = "../../base_layer/p2p", features = ["auto-update"] }
tari_script = { path = "../../infrastructure/tari_script" }
tari_shutdown = { path = "../../infrastructure/shutdown" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1205,7 +1205,7 @@ pub async fn command_runner(
&TransactionOutputVersion::get_current_version(),
&script!(PushPubKey(Box::new(
session_info.recipient_address.public_spend_key().clone()
))),
)))?,
&leader_info.output_features,
&leader_info.sender_offset_pubkey,
&leader_info.metadata_signature_ephemeral_commitment,
Expand Down
3 changes: 3 additions & 0 deletions applications/minotari_console_wallet/src/automation/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ use tari_common_types::types::FixedHashSizeError;
use tari_core::transactions::{tari_amount::MicroMinotariError, transaction_components::TransactionError};
use tari_crypto::signatures::SchnorrSignatureError;
use tari_key_manager::key_manager_service::KeyManagerServiceError;
use tari_script::ScriptError;
use tari_utilities::{hex::HexError, ByteArrayError};
use thiserror::Error;
use tokio::task::JoinError;
Expand Down Expand Up @@ -92,6 +93,8 @@ pub enum CommandError {
GrpcTlsError(#[from] GrpcTlsError),
#[error("Invalid signature: `{0}`")]
FailedSignature(#[from] SchnorrSignatureError),
#[error("Tari script error: {0}")]
ScriptError(#[from] ScriptError),
}

impl From<HexError> for CommandError {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -976,7 +976,7 @@ impl wallet_server::Wallet for WalletGrpcServer {
.await
.map_err(|e| Status::internal(e.to_string()))?;

output = output.with_script(script![Nop]);
output = output.with_script(script![Nop].map_err(|e| Status::invalid_argument(e.to_string()))?);

let (tx_id, transaction) = output_manager
.create_send_to_self_with_output(vec![output], fee_per_gram.into(), UtxoSelectionCriteria::default())
Expand Down
3 changes: 1 addition & 2 deletions applications/minotari_console_wallet/src/ui/state/tasks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ use rand::{random, rngs::OsRng};
use tari_common_types::{
tari_address::TariAddress,
types::{PublicKey, Signature},
MaxSizeBytes,
MaxSizeString,
};
use tari_core::{
consensus::DomainSeparatedConsensusHasher,
Expand All @@ -47,6 +45,7 @@ use tari_core::{
use tari_crypto::{keys::PublicKey as PublicKeyTrait, ristretto::RistrettoPublicKey};
use tari_hashing::TransactionHashDomain;
use tari_key_manager::key_manager::KeyManager;
use tari_max_size::{MaxSizeBytes, MaxSizeString};
use tari_utilities::{hex::Hex, ByteArray};
use tokio::sync::{broadcast, watch};

Expand Down
12 changes: 8 additions & 4 deletions applications/minotari_ledger_wallet/comms/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,23 +59,27 @@ mod test {
fn test_push_pub_key_serialized_byte_representation() {
let mut scripts = Vec::new();

scripts.push((script!(Nop), NOP_IDENTIFIER, "".to_string()));
scripts.push((script!(PushOne), PUSH_ONE_IDENTIFIER, "".to_string()));
scripts.push((script!(Nop).unwrap(), NOP_IDENTIFIER, "".to_string()));
scripts.push((script!(PushOne).unwrap(), PUSH_ONE_IDENTIFIER, "".to_string()));

for pub_key in [
RistrettoPublicKey::default(),
RistrettoPublicKey::from_secret_key(&RistrettoSecretKey::random(&mut OsRng)),
] {
scripts.push((
script!(PushPubKey(Box::new(pub_key.clone()))),
script!(PushPubKey(Box::new(pub_key.clone()))).unwrap(),
PUSH_PUBKEY_IDENTIFIER,
pub_key.to_hex(),
));
}

let key = RistrettoSecretKey::random(&mut OsRng);
let msg = slice_to_boxed_message(key.as_bytes());
scripts.push((script!(CheckSigVerify(msg)), CHECK_SIG_VERIFY_IDENTIFIER, key.to_hex()));
scripts.push((
script!(CheckSigVerify(msg)).unwrap(),
CHECK_SIG_VERIFY_IDENTIFIER,
key.to_hex(),
));

for (script, hex_identifier, hex_payload) in scripts {
let mut serialized = Vec::new();
Expand Down
1 change: 1 addition & 0 deletions applications/minotari_merge_mining_proxy/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ tari_common_types = { path = "../../base_layer/common_types" }
tari_comms = { path = "../../comms/core" }
tari_core = { path = "../../base_layer/core", default-features = false, features = ["transactions"] }
tari_key_manager = { path = "../../base_layer/key_manager", features = ["key_manager_service"] }
tari_max_size = { path = "../../infrastructure/max_size" }
tari_utilities = { version = "0.7" }

anyhow = "1.0.53"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ 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, MaxSizeBytes};
use tari_common_types::{tari_address::TariAddress, types::FixedHash};
use tari_core::{
consensus::ConsensusManager,
proof_of_work::{monero_rx, monero_rx::FixedByteArray, Difficulty},
Expand All @@ -37,6 +37,7 @@ use tari_core::{
},
AuxChainHashes,
};
use tari_max_size::MaxSizeBytes;
use tari_utilities::{hex::Hex, ByteArray};

use crate::{
Expand Down
2 changes: 1 addition & 1 deletion applications/minotari_merge_mining_proxy/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ 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},
transactions::{key_manager::CoreKeyManagerError, CoinbaseBuildError},
};
use tari_key_manager::key_manager_service::KeyManagerServiceError;
use tari_max_size::{MaxSizeBytesError, MaxSizeVecError};
use thiserror::Error;
use tonic::{codegen::http::uri::InvalidUri, transport};

Expand Down
1 change: 1 addition & 0 deletions applications/minotari_miner/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ tari_core = { path = "../../base_layer/core", default-features = false }
tari_common = { path = "../../common" }
tari_common_types = { path = "../../base_layer/common_types" }
tari_comms = { path = "../../comms/core" }
tari_max_size = { path = "../../infrastructure/max_size" }
minotari_app_utilities = { path = "../minotari_app_utilities", features = ["miner_input"] }
minotari_app_grpc = { path = "../minotari_app_grpc" }
tari_crypto = { version = "0.20.3" }
Expand Down
2 changes: 1 addition & 1 deletion applications/minotari_miner/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +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 tari_max_size::MaxSizeBytesError;
use thiserror::Error;
use tonic::codegen::http::uri::InvalidUri;

Expand Down
2 changes: 1 addition & 1 deletion applications/minotari_miner/src/stratum/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +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.

use tari_common_types::MaxSizeBytesError;
use tari_max_size::MaxSizeBytesError;
use thiserror::Error;

#[allow(clippy::enum_variant_names)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use borsh::BorshDeserialize;
use futures::stream::StreamExt;
use log::*;
use minotari_app_grpc::tari_rpc::BlockHeader;
use tari_common_types::MaxSizeBytes;
use tari_max_size::MaxSizeBytes;
use tari_utilities::{hex::Hex, ByteArray};

use crate::{
Expand Down
9 changes: 0 additions & 9 deletions base_layer/common_types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,9 @@ pub mod encryption;
pub mod epoch;
pub mod grpc_authentication;
pub mod key_branches;
mod max_size;
pub mod serializers;
pub mod tari_address;
pub mod transaction;
mod tx_id;
pub mod types;
pub mod wallet_types;
pub use max_size::{
MaxSizeBytes,
MaxSizeBytesError,
MaxSizeString,
MaxSizeStringLengthError,
MaxSizeVec,
MaxSizeVecError,
};
1 change: 1 addition & 0 deletions base_layer/contacts/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ tari_common_types = { path = "../../base_layer/common_types", version = "1.3.0-p
tari_comms = { path = "../../comms/core", version = "1.3.0-pre.0" }
tari_comms_dht = { path = "../../comms/dht", version = "1.3.0-pre.0" }
tari_crypto = { version = "0.20.3" }
tari_max_size = { path = "../../infrastructure/max_size" }
tari_p2p = { path = "../p2p", features = ["auto-update"], version = "1.3.0-pre.0" }
tari_service_framework = { path = "../service_framework", version = "1.3.0-pre.0" }
tari_shutdown = { path = "../../infrastructure/shutdown", version = "1.3.0-pre.0" }
Expand Down
1 change: 1 addition & 0 deletions base_layer/contacts/src/chat_client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ tari_common_types = { path = "../../../common_types" }
tari_comms = { path = "../../../../comms/core" }
tari_comms_dht = { path = "../../../../comms/dht" }
tari_contacts = { path = "../../../contacts" }
tari_max_size = { path = "../../../../infrastructure/max_size" }
tari_p2p = { path = "../../../p2p" }
tari_service_framework= { path = "../../../service_framework" }
tari_shutdown = { path = "../../../../infrastructure/shutdown" }
Expand Down
2 changes: 1 addition & 1 deletion base_layer/contacts/src/chat_client/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ use std::io;
use diesel::ConnectionError;
use minotari_app_utilities::identity_management::IdentityError;
use tari_common_sqlite::error::StorageError as SqliteStorageError;
use tari_common_types::MaxSizeBytesError;
use tari_comms::peer_manager::PeerManagerError;
use tari_contacts::contacts_service::error::ContactsServiceError;
use tari_max_size::MaxSizeBytesError;
use tari_p2p::initialization::CommsInitializationError;
use tari_storage::lmdb_store::LMDBError;

Expand Down
2 changes: 1 addition & 1 deletion base_layer/contacts/src/contacts_service/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@

use diesel::result::Error as DieselError;
use tari_common_sqlite::error::SqliteStorageError;
use tari_common_types::MaxSizeBytesError;
use tari_comms::connectivity::ConnectivityError;
use tari_comms_dht::outbound::DhtOutboundError;
use tari_max_size::MaxSizeBytesError;
use tari_p2p::services::liveness::error::LivenessError;
use tari_service_framework::reply_channel::TransportChannelError;
use thiserror::Error;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

use std::{convert::TryFrom, fmt::Display};

use tari_common_types::MaxSizeBytes;
use tari_max_size::MaxSizeBytes;
use tari_utilities::ByteArray;

use crate::contacts_service::{error::ContactsServiceError, proto, types::MessageId};
Expand Down
3 changes: 2 additions & 1 deletion base_layer/contacts/src/contacts_service/types/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ use std::{convert::TryFrom, fmt::Display};
use num_derive::FromPrimitive;
use num_traits::FromPrimitive;
use serde::{Deserialize, Serialize};
use tari_common_types::{tari_address::TariAddress, MaxSizeBytes};
use tari_common_types::tari_address::TariAddress;
use tari_comms_dht::domain_message::OutboundDomainMessage;
use tari_max_size::MaxSizeBytes;
use tari_p2p::tari_message::TariMessageType;
use tari_utilities::ByteArray;

Expand Down
1 change: 1 addition & 0 deletions base_layer/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ tari_comms = { path = "../../comms/core", version = "1.3.0-pre.0" }
tari_comms_dht = { path = "../../comms/dht", version = "1.3.0-pre.0" }
tari_comms_rpc_macros = { path = "../../comms/rpc_macros", version = "1.3.0-pre.0" }
tari_crypto = { version = "0.20.3", features = ["borsh"] }
tari_max_size = { path = "../../infrastructure/max_size" }
tari_metrics = { path = "../../infrastructure/metrics", optional = true, version = "1.3.0-pre.0" }
tari_mmr = { path = "../../base_layer/mmr", optional = true, version = "1.3.0-pre.0" }
tari_p2p = { path = "../../base_layer/p2p", version = "1.3.0-pre.0" }
Expand Down
2 changes: 1 addition & 1 deletion base_layer/core/src/blocks/pre_mine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ pub async fn create_pre_mine_genesis_block_info(
Else
PushPubKey(Box::new(backup_key.clone()))
EndIf
);
).map_err(|e| e.to_string())?;
let output = WalletOutputBuilder::new(item.value, commitment_mask.key_id)
.with_features(OutputFeatures::new(
OutputFeaturesVersion::get_current_version(),
Expand Down
4 changes: 2 additions & 2 deletions base_layer/core/src/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@

use blake2::Blake2b;
use digest::consts::U64;
#[cfg(feature = "base_node")]
use tari_common_types::MaxSizeVec;
use tari_hashing::ConfidentialOutputHashDomain;
#[cfg(feature = "base_node")]
use tari_max_size::MaxSizeVec;

use crate::consensus::DomainSeparatedConsensusHasher;

Expand Down
Loading

0 comments on commit f91cffa

Please sign in to comment.