feat: limit open ended vectors in chat (#6467)

Description
---
- Limited open-ended vectors in chat to guard against malicious network
messages.
- Fixed instability issues with chat FFI cucumber tests.

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

How Has This Been Tested?
---
- Existing unit tests pass.
- Added new unit tests to test all limits

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

applications/minotari_app_grpc/src/conversions/sidechain_feature.rs

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

applications/minotari_console_wallet/src/ui/state/tasks.rs

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

base_layer/chat_ffi/src/application_config.rs

@@ -73,6 +73,7 @@ pub unsafe extern "C" fn create_chat_config(
     let mut bad_network = |e| {
         error = LibChatError::from(InterfaceError::InvalidArgument(e)).code;
         ptr::swap(error_out, &mut error as *mut c_int);
+        ptr::null_mut::<ApplicationConfig>()
     };
 
     let network = if network_str.is_null() {

base_layer/chat_ffi/src/callback_handler.rs

@@ -74,23 +74,40 @@ impl CallbackHandler {
                 rec_message = chat_messages.recv() => {
                     match rec_message {
                         Ok(message_dispatch) => {
-                            trace!(target: LOG_TARGET, "FFI Callback monitor received a new MessageDispatch");
                             match message_dispatch.deref() {
                                 MessageDispatch::Message(m) => {
-                                    trace!(target: LOG_TARGET, "FFI Callback monitor received a new Message");
+                                    trace!(
+                                        target: LOG_TARGET,
+                                        "FFI Callback monitor received a new Message ({})",
+                                        m
+                                    );
                                     self.trigger_message_received(m.clone());
                                 }
                                 MessageDispatch::DeliveryConfirmation(c) => {
-                                    trace!(target: LOG_TARGET, "FFI Callback monitor received a new Delivery Confirmation");
+                                    trace!(
+                                        target: LOG_TARGET,
+                                        "FFI Callback monitor received a new Delivery Confirmation ({})",
+                                        c
+                                    );
                                     self.trigger_delivery_confirmation_received(c.clone());
                                 },
                                 MessageDispatch::ReadConfirmation(c) => {
-                                    trace!(target: LOG_TARGET, "FFI Callback monitor received a new Read Confirmation");
+                                    trace!(
+                                        target: LOG_TARGET,
+                                        "FFI Callback monitor received a new Read Confirmation ({})",
+                                        c
+                                    );
                                     self.trigger_read_confirmation_received(c.clone());
                                 }
                             };
                         },
-                        Err(_) => { debug!(target: LOG_TARGET, "FFI Callback monitor had an error receiving new messages")}
+                        Err(e) => {
+                            debug!(
+                                target: LOG_TARGET,
+                                "FFI Callback monitor had an error receiving new messages ({})",
+                                e
+                            )
+                        }
                     }
                 },
 
@@ -100,18 +117,27 @@ impl CallbackHandler {
                             match liveness_event.deref() {
                                 ContactsLivenessEvent::StatusUpdated(data) => {
                                     trace!(target: LOG_TARGET,
-                                        "FFI Callback monitor received Contact Status Updated event"
+                                        "FFI Callback monitor received Contact Status Updated event ({})", data
                                     );
                                     self.trigger_contact_status_change(data.deref().clone());
                                 }
                                 ContactsLivenessEvent::NetworkSilence => {},
                             }
                         },
-                        Err(_) => { debug!(target: LOG_TARGET, "FFI Callback monitor had an error with contacts liveness")}
+                        Err(e) => {
+                            debug!(
+                                target: LOG_TARGET,
+                                "FFI Callback monitor had an error with contacts liveness ({})",
+                                e
+                            )
+                        }
                     }
                 },
                 _ = self.shutdown.wait() => {
-                    info!(target: LOG_TARGET, "ChatFFI Callback Handler shutting down because the shutdown signal was received");
+                    info!(
+                        target: LOG_TARGET,
+                        "ChatFFI Callback Handler shutting down because the shutdown signal was received"
+                    );
                     break;
                 },
             }
@@ -145,7 +171,7 @@ impl CallbackHandler {
     fn trigger_delivery_confirmation_received(&mut self, confirmation: Confirmation) {
         debug!(
             target: LOG_TARGET,
-            "Calling DeliveryConfirmationReceived callback function for message {:?}",
+            "Calling DeliveryConfirmationReceived callback function for message {}",
             confirmation.message_id,
         );
 
@@ -157,7 +183,7 @@ impl CallbackHandler {
     fn trigger_read_confirmation_received(&mut self, confirmation: Confirmation) {
         debug!(
             target: LOG_TARGET,
-            "Calling ReadConfirmationReceived callback function for message {:?}",
+            "Calling ReadConfirmationReceived callback function for message {}",
             confirmation.message_id,
         );
 

base_layer/chat_ffi/src/confirmation.rs

@@ -57,11 +57,13 @@ pub unsafe extern "C" fn send_read_confirmation_for_message(
     if client.is_null() {
         error = LibChatError::from(InterfaceError::NullError("client".to_string())).code;
         ptr::swap(error_out, &mut error as *mut c_int);
+        return;
     }
 
     if message.is_null() {
         error = LibChatError::from(InterfaceError::NullError("message".to_string())).code;
         ptr::swap(error_out, &mut error as *mut c_int);
+        return;
     }
 
     let result = (*client)
@@ -97,6 +99,7 @@ pub unsafe extern "C" fn read_confirmation_message_id(
     if confirmation.is_null() {
         error = LibChatError::from(InterfaceError::NullError("client".to_string())).code;
         ptr::swap(error_out, &mut error as *mut c_int);
+        return ptr::null_mut();
     }
 
     let c = &(*confirmation);
@@ -107,7 +110,7 @@ pub unsafe extern "C" fn read_confirmation_message_id(
         Err(e) => {
             error = LibChatError::from(InterfaceError::ConversionError(e.to_string())).code;
             ptr::swap(error_out, &mut error as *mut c_int);
-            0
+            return ptr::null_mut();
         },
     };
 
@@ -162,15 +165,15 @@ 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},
         confirmation::{destroy_confirmation, read_confirmation_message_id, read_confirmation_timestamp},
     };
 
     #[test]
-    fn test_reading_from_confrimation() {
+    fn test_reading_from_confirmation() {
         let message_id = MessageBuilder::new().build().message_id;
         let timestamp = EpochTime::now().as_u64();
         let confirmation = Confirmation {
@@ -190,7 +193,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 {

base_layer/chat_ffi/src/contacts.rs

@@ -51,11 +51,13 @@ pub unsafe extern "C" fn add_chat_contact(client: *mut ChatClient, address: *mut
     if client.is_null() {
         error = LibChatError::from(InterfaceError::NullError("client".to_string())).code;
         ptr::swap(error_out, &mut error as *mut c_int);
+        return;
     }
 
     if address.is_null() {
         error = LibChatError::from(InterfaceError::NullError("receiver".to_string())).code;
         ptr::swap(error_out, &mut error as *mut c_int);
+        return;
     }
 
     let result = (*client).runtime.block_on((*client).client.add_contact(&(*address)));
@@ -94,11 +96,13 @@ pub unsafe extern "C" fn check_online_status(
     if client.is_null() {
         error = LibChatError::from(InterfaceError::NullError("client".to_string())).code;
         ptr::swap(error_out, &mut error as *mut c_int);
+        return 0;
     }
 
     if receiver.is_null() {
         error = LibChatError::from(InterfaceError::NullError("receiver".to_string())).code;
         ptr::swap(error_out, &mut error as *mut c_int);
+        return 0;
     }
 
     let rec = (*receiver).clone();

base_layer/chat_ffi/src/conversationalists.rs

@@ -56,6 +56,7 @@ pub unsafe extern "C" fn get_conversationalists(
     if client.is_null() {
         error = LibChatError::from(InterfaceError::NullError("client".to_string())).code;
         ptr::swap(error_out, &mut error as *mut c_int);
+        return ptr::null_mut();
     }
 
     let result = (*client).runtime.block_on((*client).client.get_conversationalists());

base_layer/chat_ffi/src/lib.rs

@@ -126,6 +126,7 @@ pub unsafe extern "C" fn create_chat_client(
     let mut bad_identity = |e| {
         error = LibChatError::from(InterfaceError::InvalidArgument(e)).code;
         ptr::swap(error_out, &mut error as *mut c_int);
+        ptr::null_mut::<ChatClient>()
     };
 
     let identity = match setup_node_identity(

base_layer/chat_ffi/src/message.rs

@@ -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::{
@@ -60,10 +60,12 @@ pub unsafe extern "C" fn create_chat_message(
     if receiver.is_null() {
         error = LibChatError::from(InterfaceError::NullError("receiver".to_string())).code;
         ptr::swap(error_out, &mut error as *mut c_int);
+        return ptr::null_mut();
     }
     if sender.is_null() {
         error = LibChatError::from(InterfaceError::NullError("sender".to_string())).code;
         ptr::swap(error_out, &mut error as *mut c_int);
+        return ptr::null_mut();
     }
 
     let message_str = match CStr::from_ptr(message).to_str() {
@@ -75,11 +77,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))
 }
@@ -127,16 +136,26 @@ pub unsafe extern "C" fn get_chat_message(
     if client.is_null() {
         error = LibChatError::from(InterfaceError::NullError("client".to_string())).code;
         ptr::swap(error_out, &mut error as *mut c_int);
+        return ptr::null_mut();
     }
 
     if message_id.is_null() {
         error = LibChatError::from(InterfaceError::NullError("message_id".to_string())).code;
         ptr::swap(error_out, &mut error as *mut c_int);
+        return ptr::null_mut();
     }
 
     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)),
@@ -168,11 +187,13 @@ pub unsafe extern "C" fn send_chat_message(client: *mut ChatClient, message: *mu
     if client.is_null() {
         error = LibChatError::from(InterfaceError::NullError("client".to_string())).code;
         ptr::swap(error_out, &mut error as *mut c_int);
+        return;
     }
 
     if message.is_null() {
         error = LibChatError::from(InterfaceError::NullError("message".to_string())).code;
         ptr::swap(error_out, &mut error as *mut c_int);
+        return;
     }
 
     let result = (*client)
@@ -563,7 +584,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);
@@ -575,7 +596,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));