Skip to content

Commit

Permalink
Limit open ended vectors
Browse files Browse the repository at this point in the history
Limited some more open ended vectors to guard against malicious nework messages.
  • Loading branch information
hansieodendaal committed Aug 14, 2024
1 parent 131225c commit 8a8fec2
Show file tree
Hide file tree
Showing 34 changed files with 405 additions and 191 deletions.
24 changes: 12 additions & 12 deletions applications/minotari_app_grpc/src/conversions/sidechain_feature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,18 @@

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

use tari_common_types::types::{PublicKey, Signature};
use tari_core::{
consensus::MaxSizeString,
transactions::transaction_components::{
BuildInfo,
CodeTemplateRegistration,
ConfidentialOutputData,
SideChainFeature,
TemplateType,
ValidatorNodeRegistration,
ValidatorNodeSignature,
},
use tari_common_types::{
types::{PublicKey, Signature},
MaxSizeString,
};
use tari_core::transactions::transaction_components::{
BuildInfo,
CodeTemplateRegistration,
ConfidentialOutputData,
SideChainFeature,
TemplateType,
ValidatorNodeRegistration,
ValidatorNodeSignature,
};
use tari_utilities::ByteArray;

Expand Down
4 changes: 3 additions & 1 deletion applications/minotari_console_wallet/src/ui/state/tasks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,11 @@ use rand::{random, rngs::OsRng};
use tari_common_types::{
tari_address::TariAddress,
types::{PublicKey, Signature},
MaxSizeBytes,
MaxSizeString,
};
use tari_core::{
consensus::{DomainSeparatedConsensusHasher, MaxSizeBytes, MaxSizeString},
consensus::DomainSeparatedConsensusHasher,
transactions::{
tari_amount::MicroMinotari,
transaction_components::{encrypted_data::PaymentId, BuildInfo, OutputFeatures, TemplateType},
Expand Down
4 changes: 2 additions & 2 deletions base_layer/chat_ffi/src/confirmation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ pub unsafe extern "C" fn destroy_confirmation(ptr: *mut Confirmation) {
#[cfg(test)]
mod test {
use tari_contacts::contacts_service::types::{Confirmation, MessageBuilder};
use tari_utilities::epoch_time::EpochTime;
use tari_utilities::{epoch_time::EpochTime, ByteArray};

use crate::{
byte_vector::{chat_byte_vector_get_at, chat_byte_vector_get_length},
Expand Down Expand Up @@ -190,7 +190,7 @@ mod test {
read_id.push(chat_byte_vector_get_at(id_byte_vec, i, error_out));
}

assert_eq!(message_id, read_id)
assert_eq!(message_id.to_vec(), read_id)
}

unsafe {
Expand Down
27 changes: 21 additions & 6 deletions base_layer/chat_ffi/src/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use std::{convert::TryFrom, ffi::CStr, ptr};
use libc::{c_char, c_int, c_uchar, c_uint, c_ulonglong};
use tari_chat_client::ChatClient as ChatClientTrait;
use tari_common_types::tari_address::TariAddress;
use tari_contacts::contacts_service::types::{Message, MessageBuilder, MessageMetadata};
use tari_contacts::contacts_service::types::{Message, MessageBuilder, MessageId, MessageMetadata};
use tari_utilities::ByteArray;

use crate::{
Expand Down Expand Up @@ -75,11 +75,18 @@ pub unsafe extern "C" fn create_chat_message(
},
};

let message_out = MessageBuilder::new()
let message_out = match MessageBuilder::new()
.receiver_address((*receiver).clone())
.sender_address((*sender).clone())
.message(message_str)
.build();
{
Ok(val) => val.build(),
Err(e) => {
error = LibChatError::from(InterfaceError::InvalidArgument(e.to_string())).code;
ptr::swap(error_out, &mut error as *mut c_int);
return ptr::null_mut();
},
};

Box::into_raw(Box::new(message_out))
}
Expand Down Expand Up @@ -135,8 +142,16 @@ pub unsafe extern "C" fn get_chat_message(
}

let id = process_vector(message_id, error_out);
let message_id = match MessageId::try_from(id) {
Ok(val) => val,
Err(e) => {
error = LibChatError::from(InterfaceError::ConversionError(format!("message_id ({})", e))).code;
ptr::swap(error_out, &mut error as *mut c_int);
return ptr::null_mut();
},
};

let result = (*client).runtime.block_on((*client).client.get_message(&id));
let result = (*client).runtime.block_on((*client).client.get_message(&message_id));

match result {
Ok(message) => Box::into_raw(Box::new(message)),
Expand Down Expand Up @@ -563,7 +578,7 @@ mod test {
message_id.push(chat_byte_vector_get_at(message_byte_vector, i, error_out));
}

assert_eq!(message.message_id, message_id);
assert_eq!(message.message_id.to_vec(), message_id);

destroy_chat_message(message_ptr);
chat_byte_vector_destroy(message_byte_vector);
Expand All @@ -575,7 +590,7 @@ mod test {
fn test_reading_message_body() {
let body = "Hey there!";
let body_bytes = body.as_bytes();
let message = MessageBuilder::new().message(body.into()).build();
let message = MessageBuilder::new().message(body.into()).unwrap().build();

let message_ptr = Box::into_raw(Box::new(message));
let error_out = Box::into_raw(Box::new(0));
Expand Down
24 changes: 19 additions & 5 deletions base_layer/chat_ffi/src/message_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
use std::{convert::TryFrom, ptr};

use libc::{c_int, c_uint};
use tari_contacts::contacts_service::types::{Message, MessageMetadata};
use tari_contacts::contacts_service::types::{Message, MessageMetadata, MetadataData, MetadataKey};
use tari_utilities::ByteArray;

use crate::{
Expand Down Expand Up @@ -62,10 +62,23 @@ pub unsafe extern "C" fn add_chat_message_metadata(
}
}

let metadata = MessageMetadata {
key: process_vector(key, error_out),
data: process_vector(data, error_out),
let key = match MetadataKey::try_from(process_vector(key, error_out)) {
Ok(val) => val,
Err(e) => {
error = LibChatError::from(InterfaceError::InvalidArgument(e.to_string())).code;
ptr::swap(error_out, &mut error as *mut c_int);
return;
},
};
let data = match MetadataData::try_from(process_vector(data, error_out)) {
Ok(val) => val,
Err(e) => {
error = LibChatError::from(InterfaceError::InvalidArgument(e.to_string())).code;
ptr::swap(error_out, &mut error as *mut c_int);
return;
},
};
let metadata = MessageMetadata { key, data };

(*message).push(metadata);
}
Expand Down Expand Up @@ -197,7 +210,7 @@ mod test {

let message = unsafe { Box::from_raw(message_ptr) };
assert_eq!(message.metadata.len(), 1);
assert_eq!(message.metadata[0].data, data_bytes);
assert_eq!(message.metadata[0].data.as_bytes(), data_bytes);

unsafe {
chat_byte_vector_destroy(data);
Expand All @@ -211,6 +224,7 @@ mod test {
let message_ptr = Box::into_raw(Box::new(
MessageBuilder::new()
.message("hello".to_string())
.unwrap()
.receiver_address(address.clone())
.sender_address(address)
.build(),
Expand Down
12 changes: 6 additions & 6 deletions base_layer/chat_ffi/src/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,13 +192,13 @@ mod test {

#[test]
fn test_retrieving_messages_from_vector() {
let m = MessageBuilder::new().message("hello 2".to_string()).build();
let m = MessageBuilder::new().message("hello 2".to_string()).unwrap().build();
let messages = MessageVector(vec![
MessageBuilder::new().message("hello 0".to_string()).build(),
MessageBuilder::new().message("hello 1".to_string()).build(),
MessageBuilder::new().message("hello 0".to_string()).unwrap().build(),
MessageBuilder::new().message("hello 1".to_string()).unwrap().build(),
m.clone(),
MessageBuilder::new().message("hello 3".to_string()).build(),
MessageBuilder::new().message("hello 4".to_string()).build(),
MessageBuilder::new().message("hello 3".to_string()).unwrap().build(),
MessageBuilder::new().message("hello 4".to_string()).unwrap().build(),
]);

let messages_len = messages.0.len();
Expand All @@ -218,7 +218,7 @@ mod test {
message_id.push(chat_byte_vector_get_at(message_byte_vector, i, error_out));
}

assert_eq!(m.message_id, message_id);
assert_eq!(m.message_id.to_vec(), message_id);

destroy_message_vector(message_vector_ptr);
destroy_chat_message(message_ptr);
Expand Down
2 changes: 2 additions & 0 deletions base_layer/common_types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,11 @@ 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};
Original file line number Diff line number Diff line change
@@ -1,24 +1,27 @@
// Copyright 2022, The Tari Project
// 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:
// 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.
// 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.
// 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.
// 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.
// 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
//
// Portions of this file were originally copyrighted (c) 2018 The Grin Developers, issued under the Apache License,
// Version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0.

use std::{
cmp,
Expand Down Expand Up @@ -141,7 +144,7 @@ impl<const MAX: usize> ByteArray for MaxSizeBytes<MAX> {
})
}

/// Return the NodeId as a byte array
/// Return the data as a byte array
fn as_bytes(&self) -> &[u8] {
self.inner.as_ref()
}
Expand Down
26 changes: 26 additions & 0 deletions base_layer/common_types/src/max_size/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// 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.

mod string;
pub use string::{MaxSizeString, MaxSizeStringLengthError};
mod bytes;
pub use bytes::{MaxSizeBytes, MaxSizeBytesError};
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,8 @@ pub struct MaxSizeStringLengthError {

#[cfg(test)]
mod tests {
use super::*;

mod from_str_checked {
use super::*;

use crate::MaxSizeString;
#[test]
fn it_returns_none_if_size_exceeded() {
let s = MaxSizeString::<10>::from_str_checked("12345678901234567890");
Expand Down Expand Up @@ -152,8 +149,7 @@ mod tests {
}

mod from_utf8_bytes_checked {
use super::*;

use crate::MaxSizeString;
#[test]
fn it_returns_none_if_size_exceeded() {
let s = MaxSizeString::<10>::from_utf8_bytes_checked([0u8; 11]);
Expand Down
27 changes: 14 additions & 13 deletions base_layer/contacts/src/chat_client/src/client.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 std::{
convert::TryFrom,
fmt::{Debug, Formatter},
sync::Arc,
time::Duration,
Expand All @@ -34,7 +35,7 @@ use tari_comms::{peer_manager::PeerFeatures, CommsNode, NodeIdentity};
use tari_contacts::contacts_service::{
handle::ContactsServiceHandle,
service::ContactOnlineStatus,
types::{Message, MessageBuilder, MessageMetadata},
types::{Message, MessageBuilder, MessageId, MessageMetadata, MetadataData, MetadataKey},
};
use tari_shutdown::Shutdown;

Expand All @@ -45,11 +46,11 @@ const LOG_TARGET: &str = "contacts::chat_client";
#[async_trait]
pub trait ChatClient {
async fn add_contact(&self, address: &TariAddress) -> Result<(), Error>;
fn add_metadata(&self, message: Message, metadata_type: String, data: String) -> Message;
fn add_metadata(&self, message: Message, metadata_type: String, data: String) -> Result<Message, Error>;
async fn check_online_status(&self, address: &TariAddress) -> Result<ContactOnlineStatus, Error>;
fn create_message(&self, receiver: &TariAddress, message: String) -> Message;
fn create_message(&self, receiver: &TariAddress, message: String) -> Result<Message, Error>;
async fn get_messages(&self, sender: &TariAddress, limit: u64, page: u64) -> Result<Vec<Message>, Error>;
async fn get_message(&self, id: &[u8]) -> Result<Message, Error>;
async fn get_message(&self, id: &MessageId) -> Result<Message, Error>;
async fn send_message(&self, message: Message) -> Result<(), Error>;
async fn send_read_receipt(&self, message: Message) -> Result<(), Error>;
async fn get_conversationalists(&self) -> Result<Vec<TariAddress>, Error>;
Expand Down Expand Up @@ -171,22 +172,22 @@ impl ChatClient for Client {
Ok(())
}

fn add_metadata(&self, mut message: Message, key: String, data: String) -> Message {
fn add_metadata(&self, mut message: Message, key: String, data: String) -> Result<Message, Error> {
let metadata = MessageMetadata {
key: key.into_bytes(),
data: data.into_bytes(),
key: MetadataKey::try_from(key)?,
data: MetadataData::try_from(data)?,
};

message.push(metadata);
message
Ok(message)
}

fn create_message(&self, receiver: &TariAddress, message: String) -> Message {
MessageBuilder::new()
fn create_message(&self, receiver: &TariAddress, message: String) -> Result<Message, Error> {
Ok(MessageBuilder::new()
.receiver_address(receiver.clone())
.sender_address(self.address().clone())
.message(message)
.build()
.message(message)?
.build())
}

async fn check_online_status(&self, address: &TariAddress) -> Result<ContactOnlineStatus, Error> {
Expand All @@ -208,7 +209,7 @@ impl ChatClient for Client {
Ok(messages)
}

async fn get_message(&self, message_id: &[u8]) -> Result<Message, Error> {
async fn get_message(&self, message_id: &MessageId) -> Result<Message, Error> {
match self.contacts.clone() {
Some(mut contacts_service) => contacts_service.get_message(message_id).await.map_err(|e| e.into()),
None => Err(Error::InitializationError(
Expand Down
Loading

0 comments on commit 8a8fec2

Please sign in to comment.