Skip to content

Commit

Permalink
Handle Out of Bounds Read In Input Message Data (#6439)
Browse files Browse the repository at this point in the history
## Description

Previously, when the offset in `input_message_data()` was not zero, an
out of bound read would occur as the offset was not subtracted from the
length to read. This has been added. A check to ensure that the offset
does not exceed the length has also been added.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.

---------

Co-authored-by: K1-R1 <[email protected]>
Co-authored-by: Joshua Batty <[email protected]>
  • Loading branch information
3 people authored Aug 23, 2024
1 parent 34f8515 commit dbff13d
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 13 deletions.
13 changes: 9 additions & 4 deletions sway-lib-std/src/inputs.sw
Original file line number Diff line number Diff line change
Expand Up @@ -586,11 +586,16 @@ pub fn input_message_data(index: u64, offset: u64) -> Option<Bytes> {
Some(Input::Message) => {
let data = __gtf::<raw_ptr>(index, GTF_INPUT_MESSAGE_DATA);
let data_with_offset = data.add_uint_offset(offset);
let length = input_message_data_length(index).unwrap();
let new_ptr = alloc_bytes(length);
let total_length = input_message_data_length(index).unwrap();
if offset > total_length {
return None
}
let offset_length = total_length - offset;

data_with_offset.copy_bytes_to(new_ptr, length);
Some(Bytes::from(raw_slice::from_parts::<u8>(new_ptr, length)))
let new_ptr = alloc_bytes(offset_length);

data_with_offset.copy_bytes_to(new_ptr, offset_length);
Some(Bytes::from(raw_slice::from_parts::<u8>(new_ptr, offset_length)))
},
_ => None,
}
Expand Down
21 changes: 14 additions & 7 deletions test/src/sdk-harness/test_artifacts/tx_contract/src/main.sw
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ abi TxContractTest {
fn get_input_message_data_length(index: u64) -> Option<u64>;
fn get_input_predicate_length(index: u64) -> Option<u64>;
fn get_input_predicate_data_length(index: u64) -> Option<u64>;
fn get_input_message_data(index: u64, offset: u64, expected: [u8; 3]) -> bool;
fn get_input_message_data(index: u64, offset: u64, expected_data_bytes: Bytes) -> bool;
fn get_input_predicate(index: u64, bytecode: Vec<u8>) -> bool;

fn get_output_type(ptr: u64) -> Option<Output>;
Expand Down Expand Up @@ -123,18 +123,25 @@ impl TxContractTest for Contract {
fn get_input_predicate_data_length(index: u64) -> Option<u64> {
input_predicate_data_length(index)
}
fn get_input_message_data(index: u64, offset: u64, expected: [u8; 3]) -> bool {
fn get_input_message_data(index: u64, offset: u64, expected_data_bytes: Bytes) -> bool {
let data = match input_message_data(index, offset) {
Some(b) => b,
None => return false,
};

let mut expected_data_bytes = Bytes::new();
if expected_data_bytes.len() != data.len() {
return false
}

let mut iter = 0;
while iter < expected_data_bytes.len() {
if data.get(iter).unwrap() != expected_data_bytes.get(iter).unwrap() {
return false
}
iter += 1;
}

expected_data_bytes.push(expected[0]);
expected_data_bytes.push(expected[1]);
expected_data_bytes.push(expected[2]);
(data.get(0).unwrap() == expected_data_bytes.get(0).unwrap()) && (data.get(1).unwrap() == expected_data_bytes.get(1).unwrap()) && (data.get(2).unwrap() == expected_data_bytes.get(2).unwrap())
return true
}

fn get_input_predicate(index: u64, bytecode: Vec<u8>) -> bool {
Expand Down
76 changes: 74 additions & 2 deletions test/src/sdk-harness/test_projects/tx_fields/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -890,7 +890,7 @@ mod inputs {

let mut builder = contract_instance
.methods()
.get_input_message_data(3, 0, MESSAGE_DATA)
.get_input_message_data(3, 0, Bytes(MESSAGE_DATA.into()))
.transaction_builder()
.await
.unwrap();
Expand Down Expand Up @@ -920,14 +920,86 @@ mod inputs {
// Assert none returned when transaction type is not a message
let none_result = contract_instance
.methods()
.get_input_message_data(3, 0, MESSAGE_DATA)
.get_input_message_data(3, 0, Bytes(MESSAGE_DATA.into()))
.call()
.await
.unwrap();

assert_eq!(none_result.value, false);
}

#[tokio::test]
async fn can_get_input_message_data_with_offset() {
let (contract_instance, _, wallet, _) = get_contracts(true).await;
let message = &wallet.get_messages().await.unwrap()[0];
let provider = wallet.provider().unwrap();

let mut builder = contract_instance
.methods()
.get_input_message_data(3, 1, Bytes(MESSAGE_DATA[1..].into()))
.transaction_builder()
.await
.unwrap();

wallet.adjust_for_fee(&mut builder, 1000).await.unwrap();

builder.inputs_mut().push(SdkInput::ResourceSigned {
resource: CoinType::Message(message.clone()),
});

builder.add_signer(wallet.clone()).unwrap();

let tx = builder.build(provider).await.unwrap();

let provider = wallet.provider().unwrap();
let tx_id = provider.send_transaction(tx).await.unwrap();

let receipts = provider
.tx_status(&tx_id)
.await
.unwrap()
.take_receipts_checked(None)
.unwrap();

assert_eq!(receipts[1].data(), Some(&[1][..]));
}

#[tokio::test]
async fn input_message_data_none_when_offset_exceeds_length() {
let (contract_instance, _, wallet, _) = get_contracts(true).await;
let message = &wallet.get_messages().await.unwrap()[0];
let provider = wallet.provider().unwrap();

let mut builder = contract_instance
.methods()
.get_input_message_data(3, (MESSAGE_DATA.len() + 1) as u64, Bytes(MESSAGE_DATA.into()))
.transaction_builder()
.await
.unwrap();

wallet.adjust_for_fee(&mut builder, 1000).await.unwrap();

builder.inputs_mut().push(SdkInput::ResourceSigned {
resource: CoinType::Message(message.clone()),
});

builder.add_signer(wallet.clone()).unwrap();

let tx = builder.build(provider).await.unwrap();

let provider = wallet.provider().unwrap();
let tx_id = provider.send_transaction(tx).await.unwrap();

let receipts = provider
.tx_status(&tx_id)
.await
.unwrap()
.take_receipts_checked(None)
.unwrap();

assert_eq!(receipts[1].data(), Some(&[0][..]));
}

#[tokio::test]
async fn can_get_input_message_predicate() {
let (contract_instance, _, wallet, _) = get_contracts(true).await;
Expand Down

0 comments on commit dbff13d

Please sign in to comment.