From b1dea76d42bb78ccfd145a88127154922ba9ea45 Mon Sep 17 00:00:00 2001 From: SW van Heerden Date: Fri, 19 Jul 2024 08:47:44 +0200 Subject: [PATCH] Fix panic in from_base58 fn --- .../minotari_merge_mining_proxy/src/proxy.rs | 2 +- .../src/tari_address/dual_address.rs | 4 +-- .../common_types/src/tari_address/mod.rs | 26 +++++++++++++++++-- .../src/tari_address/single_address.rs | 4 +-- base_layer/wallet_ffi/src/error.rs | 6 ++++- common/src/build/application.rs | 3 ++- 6 files changed, 36 insertions(+), 9 deletions(-) diff --git a/applications/minotari_merge_mining_proxy/src/proxy.rs b/applications/minotari_merge_mining_proxy/src/proxy.rs index c80ae56267..33fe079486 100644 --- a/applications/minotari_merge_mining_proxy/src/proxy.rs +++ b/applications/minotari_merge_mining_proxy/src/proxy.rs @@ -648,7 +648,7 @@ impl InnerService { .iter() .position(|x| x == &last_used_url) .unwrap_or(0); - let (left, right) = self.config.monerod_url.split_at(pos); + let (left, right) = self.config.monerod_url.split_at_checked(pos).ok_or(MmProxyError::ConversionError("Invalid utf 8 url".to_string()))?; let left = left.to_vec(); let right = right.to_vec(); let iter = right.iter().chain(left.iter()).chain(right.iter()).chain(left.iter()); diff --git a/base_layer/common_types/src/tari_address/dual_address.rs b/base_layer/common_types/src/tari_address/dual_address.rs index db591aebbf..4347f79484 100644 --- a/base_layer/common_types/src/tari_address/dual_address.rs +++ b/base_layer/common_types/src/tari_address/dual_address.rs @@ -162,8 +162,8 @@ impl DualAddress { if hex_str.len() != 90 && hex_str.len() != 91 { return Err(TariAddressError::InvalidSize); } - let (first, rest) = hex_str.split_at(2); - let (network, features) = first.split_at(1); + let (first, rest) = hex_str.split_at_checked(2).ok_or(TariAddressError::InvalidCharacter)?; + let (network, features) = first.split_at_checked(1).ok_or(TariAddressError::InvalidCharacter)?; let mut result = bs58::decode(network) .into_vec() .map_err(|_| TariAddressError::CannotRecoverNetwork)?; diff --git a/base_layer/common_types/src/tari_address/mod.rs b/base_layer/common_types/src/tari_address/mod.rs index 2f2a19471b..f57f38ac50 100644 --- a/base_layer/common_types/src/tari_address/mod.rs +++ b/base_layer/common_types/src/tari_address/mod.rs @@ -104,6 +104,8 @@ pub enum TariAddressError { InvalidChecksum, #[error("Invalid emoji character")] InvalidEmoji, + #[error("Invalid text character")] + InvalidCharacter, #[error("Cannot recover public key")] CannotRecoverPublicKey, #[error("Cannot recover network")] @@ -246,8 +248,8 @@ impl TariAddress { if hex_str.len() < 47 { return Err(TariAddressError::InvalidSize); } - let (first, rest) = hex_str.split_at(2); - let (network, features) = first.split_at(1); + let (first, rest) = hex_str.split_at_checked(2).ok_or(TariAddressError::InvalidCharacter)?; + let (network, features) = first.split_at_checked(1).ok_or(TariAddressError::InvalidCharacter)?; let mut result = bs58::decode(network) .into_vec() .map_err(|_| TariAddressError::CannotRecoverNetwork)?; @@ -820,4 +822,24 @@ mod test { Err(TariAddressError::CannotRecoverPublicKey) ); } + + #[test] + /// Test invalid emoji + fn invalid_utf8_char() { + // This emoji string contains an invalid utf8 character + let emoji_string = "🦊 | 🦊 | 🦊 | 🦊 | 🦊 | 🦊| 🦊 | 🦊| 🦊 | 🦊| 🦊 | 🦊| 🦊 | 🦊| 🦊 | 🦊| 🦊 | 🦊| 🦊 | 🦊| 🦊 | 🦊| 🦊 | 🦊| 🦊 | 🦊| 🦊 | 🦊| 🦊 | 🦊| 🦊 | 🦊| 🦊 | 🦊| 🦊 | 🦊| 🦊 | 🦊| 🦊 | 🦊| 🦊 | 🦊| 🦊 | 🦊"; + assert_eq!( + TariAddress::from_base58(emoji_string), + Err(TariAddressError::InvalidCharacter) + ); + assert_eq!( + TariAddress::from_emoji_string(emoji_string), + Err(TariAddressError::InvalidSize) + ); + assert_eq!( + TariAddress::from_str(emoji_string), + Err(TariAddressError::InvalidAddressString) + ); + } + } diff --git a/base_layer/common_types/src/tari_address/single_address.rs b/base_layer/common_types/src/tari_address/single_address.rs index 5553fc0dcb..323156905b 100644 --- a/base_layer/common_types/src/tari_address/single_address.rs +++ b/base_layer/common_types/src/tari_address/single_address.rs @@ -145,8 +145,8 @@ impl SingleAddress { if hex_str.len() != 46 && hex_str.len() != 47 && hex_str.len() != 48 { return Err(TariAddressError::InvalidSize); } - let (first, rest) = hex_str.split_at(2); - let (network, features) = first.split_at(1); + let (first, rest) = hex_str.split_at_checked(2).ok_or(TariAddressError::InvalidCharacter)?; + let (network, features) = first.split_at_checked(1).ok_or(TariAddressError::InvalidCharacter)?; let mut result = bs58::decode(network) .into_vec() .map_err(|_| TariAddressError::CannotRecoverNetwork)?; diff --git a/base_layer/wallet_ffi/src/error.rs b/base_layer/wallet_ffi/src/error.rs index 4855faa85d..016cb4f3a2 100644 --- a/base_layer/wallet_ffi/src/error.rs +++ b/base_layer/wallet_ffi/src/error.rs @@ -430,7 +430,11 @@ impl From for LibWalletError { message: format!("{:?}", e), }, TariAddressError::InvalidAddressString => Self { - code: 706, + code: 707, + message: format!("{:?}", e), + }, + TariAddressError::InvalidCharacter => Self { + code: 708, message: format!("{:?}", e), }, } diff --git a/common/src/build/application.rs b/common/src/build/application.rs index 39bddc484e..d10972626f 100644 --- a/common/src/build/application.rs +++ b/common/src/build/application.rs @@ -128,7 +128,8 @@ fn get_commit() -> Result { let repo = git2::Repository::open(git_root)?; let head = repo.revparse_single("HEAD")?; let id = format!("{:?}", head.id()); - id.split_at(7).0.to_string(); + id.split_at_checked(7).ok_or(anyhow::anyhow!( + "invalid utf8 in commit id"))?.0.to_string(); Ok(id) }