From 5e0e3a4f8aba1171dcaeb236857c6be4f2ef70a4 Mon Sep 17 00:00:00 2001 From: Daniel Frederico Lins Leite Date: Thu, 22 Aug 2024 12:22:31 +0100 Subject: [PATCH] Guarantee only valid bools are being decoded (#6436) ## 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. --- sway-lib-core/src/codec.sw | 12 ++++++- .../json_abi_oracle_new_encoding.json | 32 +++++++++---------- .../storage_access_caller/src/main.sw | 2 +- 3 files changed, 28 insertions(+), 18 deletions(-) diff --git a/sway-lib-core/src/codec.sw b/sway-lib-core/src/codec.sw index 2b2489b16e8..2e9ed4aae20 100644 --- a/sway-lib-core/src/codec.sw +++ b/sway-lib-core/src/codec.sw @@ -2585,7 +2585,11 @@ impl AbiDecode for u8 { impl AbiDecode for bool { fn abi_decode(ref mut buffer: BufferReader) -> bool { - buffer.read::() + match buffer.read::() { + 0 => false, + 1 => true, + _ => __revert(0), + } } } @@ -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::(actual); +} diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/language/configurable_consts/json_abi_oracle_new_encoding.json b/test/src/e2e_vm_tests/test_programs/should_pass/language/configurable_consts/json_abi_oracle_new_encoding.json index fb14cd4d3c7..c7ff882e97c 100644 --- a/test/src/e2e_vm_tests/test_programs/should_pass/language/configurable_consts/json_abi_oracle_new_encoding.json +++ b/test/src/e2e_vm_tests/test_programs/should_pass/language/configurable_consts/json_abi_oracle_new_encoding.json @@ -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", diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/storage_access_caller/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/storage_access_caller/src/main.sw index f720cbddea9..aa4a3e1f59b 100644 --- a/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/storage_access_caller/src/main.sw +++ b/test/src/e2e_vm_tests/test_programs/should_pass/require_contract_deployment/storage_access_caller/src/main.sw @@ -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);