From 9fdcb945d98ba8939249d57db16ae4ba4fc73bf5 Mon Sep 17 00:00:00 2001 From: Hannes Karppila Date: Thu, 27 Jul 2023 08:51:10 +0300 Subject: [PATCH 1/4] Fix a crash when doing from_str with multibyte characters --- fuel-asm/src/lib.rs | 1 + fuel-crypto/src/lib.rs | 3 ++- fuel-tx/src/lib.rs | 1 + fuel-tx/src/transaction/types/utxo_id.rs | 25 ++++++++++++++++-------- fuel-tx/src/tx_pointer.rs | 11 ++++++++--- fuel-vm/src/lib.rs | 1 + 6 files changed, 30 insertions(+), 12 deletions(-) diff --git a/fuel-asm/src/lib.rs b/fuel-asm/src/lib.rs index e547f7b62d..9f8d161541 100644 --- a/fuel-asm/src/lib.rs +++ b/fuel-asm/src/lib.rs @@ -3,6 +3,7 @@ #![cfg_attr(docsrs, feature(doc_auto_cfg))] #![cfg_attr(not(feature = "std"), no_std)] #![cfg_attr(feature = "std", doc = include_str!("../README.md"))] +#![deny(clippy::string_slice)] #![deny(missing_docs)] #![deny(unsafe_code)] #![deny(unused_crate_dependencies)] diff --git a/fuel-crypto/src/lib.rs b/fuel-crypto/src/lib.rs index 0ca97b450f..08db7bd732 100644 --- a/fuel-crypto/src/lib.rs +++ b/fuel-crypto/src/lib.rs @@ -2,10 +2,11 @@ #![cfg_attr(docsrs, feature(doc_auto_cfg))] #![cfg_attr(not(feature = "std"), no_std)] -#![warn(missing_docs)] // Wrong clippy convention; check // https://rust-lang.github.io/api-guidelines/naming.html #![allow(clippy::wrong_self_convention)] +#![deny(clippy::string_slice)] +#![warn(missing_docs)] #![deny(unsafe_code)] #![deny(unused_crate_dependencies)] diff --git a/fuel-tx/src/lib.rs b/fuel-tx/src/lib.rs index 60a1950a69..941960e99b 100644 --- a/fuel-tx/src/lib.rs +++ b/fuel-tx/src/lib.rs @@ -4,6 +4,7 @@ // Wrong clippy convention; check // https://rust-lang.github.io/api-guidelines/naming.html #![allow(clippy::wrong_self_convention)] +#![deny(clippy::string_slice)] #![deny(unused_crate_dependencies)] #![deny(unsafe_code)] diff --git a/fuel-tx/src/transaction/types/utxo_id.rs b/fuel-tx/src/transaction/types/utxo_id.rs index f66127de21..453718f6eb 100644 --- a/fuel-tx/src/transaction/types/utxo_id.rs +++ b/fuel-tx/src/transaction/types/utxo_id.rs @@ -101,20 +101,29 @@ impl fmt::UpperHex for UtxoId { impl str::FromStr for UtxoId { type Err = &'static str; + /// UtxoId is encoded as hex string with optional 0x prefix, where + /// the last two characters are the output index and the part + /// optionally preceeding it is the transaction id. fn from_str(s: &str) -> Result { const ERR: &str = "Invalid encoded byte"; let s = s.trim_start_matches("0x"); - let utxo_id = if s.is_empty() { + + Ok(if s.is_empty() { UtxoId::new(Bytes32::default(), 0) - } else if s.len() > 2 { + } else if s.len() <= 2 { + UtxoId::new(TxId::default(), u8::from_str_radix(s, 16).map_err(|_| ERR)?) + } else { + let i = s.len() - 2; + if !s.is_char_boundary(i) { + return Err(ERR) + } + let (tx_id, output_index) = s.split_at(i); + UtxoId::new( - Bytes32::from_str(&s[..s.len() - 2])?, - u8::from_str_radix(&s[s.len() - 2..], 16).map_err(|_| ERR)?, + Bytes32::from_str(tx_id)?, + u8::from_str_radix(output_index, 16).map_err(|_| ERR)?, ) - } else { - UtxoId::new(TxId::default(), u8::from_str_radix(s, 16).map_err(|_| ERR)?) - }; - Ok(utxo_id) + }) } } diff --git a/fuel-tx/src/tx_pointer.rs b/fuel-tx/src/tx_pointer.rs index 1273f593ff..03b1510135 100644 --- a/fuel-tx/src/tx_pointer.rs +++ b/fuel-tx/src/tx_pointer.rs @@ -92,15 +92,20 @@ impl fmt::UpperHex for TxPointer { impl str::FromStr for TxPointer { type Err = &'static str; + /// TxPointer is encoded as 12 hex characters: + /// - 8 characters for block height + /// - 4 characters for tx index fn from_str(s: &str) -> Result { const ERR: &str = "Invalid encoded byte"; - if s.len() != 12 { + if s.len() != 12 || !s.is_char_boundary(8) { return Err(ERR) } - let block_height = u32::from_str_radix(&s[..8], 16).map_err(|_| ERR)?; - let tx_index = u16::from_str_radix(&s[8..12], 16).map_err(|_| ERR)?; + let (block_height, tx_index) = s.split_at(8); + + let block_height = u32::from_str_radix(block_height, 16).map_err(|_| ERR)?; + let tx_index = u16::from_str_radix(tx_index, 16).map_err(|_| ERR)?; Ok(Self::new(block_height.into(), tx_index)) } diff --git a/fuel-vm/src/lib.rs b/fuel-vm/src/lib.rs index e5d2fa71a8..3e0ee1f301 100644 --- a/fuel-vm/src/lib.rs +++ b/fuel-vm/src/lib.rs @@ -3,6 +3,7 @@ #![warn(missing_docs)] #![deny(unsafe_code)] #![deny(unused_crate_dependencies)] +#![deny(clippy::string_slice)] pub mod arith; pub mod backtrace; From e7e8be071623cb2e4b9c199ef80e963eb328026d Mon Sep 17 00:00:00 2001 From: Hannes Karppila Date: Thu, 27 Jul 2023 08:55:08 +0300 Subject: [PATCH 2/4] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7fd2c742e..1c2efecaae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Changed - [#525](https://github.com/FuelLabs/fuel-vm/pull/525): The `$hp` register is no longer restored to it's previous value when returning from a call, making it possible to return heap-allocated types from `CALL`. +- [#531](https://github.com/FuelLabs/fuel-vm/pull/531): UtxoId::from_str and TxPointer::from_str no longer crash on invalid input with multibyte characters. Also adds clippy lints to prevent future issues. #### Breaking From 1744ddb4569bb488f6cb2e3126384ac3e7bb0a1b Mon Sep 17 00:00:00 2001 From: Hannes Karppila Date: Thu, 27 Jul 2023 09:07:32 +0300 Subject: [PATCH 3/4] Add test cases --- fuel-tx/src/transaction/types/utxo_id.rs | 6 ++++++ fuel-tx/src/tx_pointer.rs | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/fuel-tx/src/transaction/types/utxo_id.rs b/fuel-tx/src/transaction/types/utxo_id.rs index 453718f6eb..7c80ff6489 100644 --- a/fuel-tx/src/transaction/types/utxo_id.rs +++ b/fuel-tx/src/transaction/types/utxo_id.rs @@ -214,4 +214,10 @@ mod tests { assert_eq!(utxo_id.tx_id[0], 12); Ok(()) } + + #[test] + fn from_str_utxo_id_multibyte_bug() { + UtxoId::from_str("0x00😎").expect_err("Should fail on incorrect input"); + UtxoId::from_str("0x000😎").expect_err("Should fail on incorrect input"); + } } diff --git a/fuel-tx/src/tx_pointer.rs b/fuel-tx/src/tx_pointer.rs index 03b1510135..6bc0f3b169 100644 --- a/fuel-tx/src/tx_pointer.rs +++ b/fuel-tx/src/tx_pointer.rs @@ -195,3 +195,9 @@ fn fmt_encode_decode() { } } } + +#[test] +fn decode_bug() { + use core::str::FromStr; + TxPointer::from_str("00000😎000").expect_err("Should fail on incorrect input"); +} From 2aff7a7503eefc9d3d1a4e483c8dae962c55cd84 Mon Sep 17 00:00:00 2001 From: Hannes Karppila Date: Thu, 27 Jul 2023 09:38:56 +0300 Subject: [PATCH 4/4] Add comments linking to the issue to the test cases --- fuel-tx/src/transaction/types/utxo_id.rs | 1 + fuel-tx/src/tx_pointer.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/fuel-tx/src/transaction/types/utxo_id.rs b/fuel-tx/src/transaction/types/utxo_id.rs index 7c80ff6489..274964d04e 100644 --- a/fuel-tx/src/transaction/types/utxo_id.rs +++ b/fuel-tx/src/transaction/types/utxo_id.rs @@ -215,6 +215,7 @@ mod tests { Ok(()) } + /// See https://github.com/FuelLabs/fuel-vm/issues/521 #[test] fn from_str_utxo_id_multibyte_bug() { UtxoId::from_str("0x00😎").expect_err("Should fail on incorrect input"); diff --git a/fuel-tx/src/tx_pointer.rs b/fuel-tx/src/tx_pointer.rs index 6bc0f3b169..1edfd174d8 100644 --- a/fuel-tx/src/tx_pointer.rs +++ b/fuel-tx/src/tx_pointer.rs @@ -196,6 +196,7 @@ fn fmt_encode_decode() { } } +/// See https://github.com/FuelLabs/fuel-vm/issues/521 #[test] fn decode_bug() { use core::str::FromStr;