Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a crash in UtxoId::from_str and TxPointer::from_str with multibyte characters #531

Merged
merged 4 commits into from
Jul 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions fuel-asm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
3 changes: 2 additions & 1 deletion fuel-crypto/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]

Expand Down
1 change: 1 addition & 0 deletions fuel-tx/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we also add this to the fuel-types lib.rs?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do that as a follow-up, as I'm alreaady doing parsing changes as a part of #533

#![deny(unused_crate_dependencies)]
#![deny(unsafe_code)]

Expand Down
32 changes: 24 additions & 8 deletions fuel-tx/src/transaction/types/utxo_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Self, Self::Err> {
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)
})
}
}

Expand Down Expand Up @@ -205,4 +214,11 @@ mod tests {
assert_eq!(utxo_id.tx_id[0], 12);
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");
UtxoId::from_str("0x000😎").expect_err("Should fail on incorrect input");
}
}
18 changes: 15 additions & 3 deletions fuel-tx/src/tx_pointer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Self, Self::Err> {
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))
}
Expand Down Expand Up @@ -190,3 +195,10 @@ fn fmt_encode_decode() {
}
}
}

/// See https://github.com/FuelLabs/fuel-vm/issues/521
#[test]
fn decode_bug() {
use core::str::FromStr;
TxPointer::from_str("00000😎000").expect_err("Should fail on incorrect input");
}
1 change: 1 addition & 0 deletions fuel-vm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#![warn(missing_docs)]
#![deny(unsafe_code)]
#![deny(unused_crate_dependencies)]
#![deny(clippy::string_slice)]

pub mod arith;
pub mod backtrace;
Expand Down