Skip to content

Commit

Permalink
Guarantee only valid bools are being decoded (#6436)
Browse files Browse the repository at this point in the history
## Description

`AbiDecode` for `bool` was not checking for valid values allowing
unsound bools that were not `true` nor `false`. Given that `3VL` is not
within our scope here (:smile:), this PR will now `revert` in this case
which is certainly a problem decoding values.

The same thing already happens for `enum` variant, which also
`revert(0)` when the variant is unknown.

These two are the only cases where the "encoded bytes" can live outside
of their respective valid set.

## Checklist

- [ ] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [ ] 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.
- [ ] 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.
  • Loading branch information
xunilrj authored Aug 22, 2024
1 parent ae9a8a9 commit 5e0e3a4
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 18 deletions.
12 changes: 11 additions & 1 deletion sway-lib-core/src/codec.sw
Original file line number Diff line number Diff line change
Expand Up @@ -2585,7 +2585,11 @@ impl AbiDecode for u8 {

impl AbiDecode for bool {
fn abi_decode(ref mut buffer: BufferReader) -> bool {
buffer.read::<bool>()
match buffer.read::<u8>() {
0 => false,
1 => true,
_ => __revert(0),
}
}
}

Expand Down Expand Up @@ -5357,3 +5361,9 @@ fn ok_abi_encoding() {
assert_eq(array[0], 255u8, 0);
assert_eq(array[1], 254u8, 0);
}

#[test(should_revert)]
fn nok_abi_encoding_invalid_bool() {
let actual = encode(2u8);
let _ = abi_decode::<bool>(actual);
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,82 +62,82 @@
{
"concreteTypeId": "b760f44fa5965c2474a3b471467a22c43185152129295af588b022ae50b50903",
"name": "BOOL",
"offset": 7400
"offset": 7432
},
{
"concreteTypeId": "c89951a24c6ca28c13fd1cfdc646b2b656d69e61a92b91023be7eb58eb914b6b",
"name": "U8",
"offset": 7592
"offset": 7624
},
{
"concreteTypeId": "c89951a24c6ca28c13fd1cfdc646b2b656d69e61a92b91023be7eb58eb914b6b",
"name": "ANOTHER_U8",
"offset": 7328
"offset": 7360
},
{
"concreteTypeId": "29881aad8730c5ab11d275376323d8e4ff4179aae8ccb6c13fe4902137e162ef",
"name": "U16",
"offset": 7536
"offset": 7568
},
{
"concreteTypeId": "d7649d428b9ff33d188ecbf38a7e4d8fd167fa01b2e10fe9a8f9308e52f1d7cc",
"name": "U32",
"offset": 7576
"offset": 7608
},
{
"concreteTypeId": "d7649d428b9ff33d188ecbf38a7e4d8fd167fa01b2e10fe9a8f9308e52f1d7cc",
"name": "U64",
"offset": 7584
"offset": 7616
},
{
"concreteTypeId": "1b5759d94094368cfd443019e7ca5ec4074300e544e5ea993a979f5da627261e",
"name": "U256",
"offset": 7544
"offset": 7576
},
{
"concreteTypeId": "7c5ee1cecf5f8eacd1284feb5f0bf2bdea533a51e2f0c9aabe9236d335989f3b",
"name": "B256",
"offset": 7368
"offset": 7400
},
{
"concreteTypeId": "81fc10c4681a3271cf2d66b2ec6fbc8ed007a442652930844fcf11818c295bff",
"name": "CONFIGURABLE_STRUCT",
"offset": 7488
"offset": 7520
},
{
"concreteTypeId": "a2922861f03be8a650595dd76455b95383a61b46dd418f02607fa2e00dc39d5c",
"name": "CONFIGURABLE_ENUM_A",
"offset": 7408
"offset": 7440
},
{
"concreteTypeId": "a2922861f03be8a650595dd76455b95383a61b46dd418f02607fa2e00dc39d5c",
"name": "CONFIGURABLE_ENUM_B",
"offset": 7448
"offset": 7480
},
{
"concreteTypeId": "4926d35d1a5157936b0a29bc126b8aace6d911209a5c130e9b716b0c73643ea6",
"name": "ARRAY_BOOL",
"offset": 7336
"offset": 7368
},
{
"concreteTypeId": "776fb5a3824169d6736138565fdc20aad684d9111266a5ff6d5c675280b7e199",
"name": "ARRAY_U64",
"offset": 7344
"offset": 7376
},
{
"concreteTypeId": "c998ca9a5f221fe7b5c66ae70c8a9562b86d964408b00d17f883c906bc1fe4be",
"name": "TUPLE_BOOL_U64",
"offset": 7520
"offset": 7552
},
{
"concreteTypeId": "94f0fa95c830be5e4f711963e83259fe7e8bc723278ab6ec34449e791a99b53a",
"name": "STR_4",
"offset": 7512
"offset": 7544
},
{
"concreteTypeId": "c89951a24c6ca28c13fd1cfdc646b2b656d69e61a92b91023be7eb58eb914b6b",
"name": "NOT_USED",
"offset": 7504
"offset": 7536
}
],
"encodingVersion": "1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::hash::*;
#[cfg(experimental_new_encoding = false)]
const CONTRACT_ID = 0x3bc28acd66d327b8c1b9624c1fabfc07e9ffa1b5d71c2832c3bfaaf8f4b805e9;
#[cfg(experimental_new_encoding = true)]
const CONTRACT_ID = 0xa6026cfed3ba7a4ab3aa17908da17248f0489c042acc7ffaefb8b6c6536b069a; // AUTO-CONTRACT-ID ../../test_contracts/storage_access_contract --release
const CONTRACT_ID = 0xe4d312c5acc2982abb501d0baec18fe10ec1e8f7d68ae9dc1e8c92482a0c1d85; // AUTO-CONTRACT-ID ../../test_contracts/storage_access_contract --release

fn main() -> bool {
let caller = abi(StorageAccess, CONTRACT_ID);
Expand Down

0 comments on commit 5e0e3a4

Please sign in to comment.