From 0d6fb63a353f5fbd8f2e70356e87422ea4981c38 Mon Sep 17 00:00:00 2001 From: Arturo Beccar-Varela <107512933+arturoBeccar@users.noreply.github.com> Date: Thu, 14 Dec 2023 16:04:28 -0300 Subject: [PATCH 1/4] Update README.md --- test-cases/README.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test-cases/README.md b/test-cases/README.md index b2956151..3d933d53 100644 --- a/test-cases/README.md +++ b/test-cases/README.md @@ -118,3 +118,10 @@ and has a Critical severity. If users are allowed to call `update_current_contract_wasm()`, they can intentionally modify the contract behaviour, leading to the loss of all associated data/tokens and functionalities given by this contract or by others that depend on it. To prevent this, the function should be restricted to administrators or authorized users only. This vulnerability falls under the [Authorization](#vulnerability-categories) category and has a Critical severity. + +### Avoid core::mem::forget + +The `core::mem::forget` function is used to forget about a value without running its destructor. This could lead to memory leaks and logic errors. + +We classified this issue, a deviation from best practices which could have +security implications, under the [Best practices](#vulnerability-categories) category and assigned it an Enhancement severity. From 2a065bbc98be51f3448066598576a556a8949962 Mon Sep 17 00:00:00 2001 From: Arturo Beccar-Varela <107512933+arturoBeccar@users.noreply.github.com> Date: Thu, 14 Dec 2023 16:17:17 -0300 Subject: [PATCH 2/4] Create README.md --- detectors/avoid-core-mem-forget/README.md | 31 +++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 detectors/avoid-core-mem-forget/README.md diff --git a/detectors/avoid-core-mem-forget/README.md b/detectors/avoid-core-mem-forget/README.md new file mode 100644 index 00000000..089b9c8c --- /dev/null +++ b/detectors/avoid-core-mem-forget/README.md @@ -0,0 +1,31 @@ +# Avoid core::mem::forget usage + +### What it does + +Checks for `core::mem::forget` usage. + +### Why is this bad? + +This is a bad practice because it can lead to memory leaks, resource leaks and logic errors. + +### Example + +```rust +pub fn forget_something(n: WithoutCopy) -> u64 { + core::mem::forget(n); + 0 +} +``` + +Use instead: + +```rust +pub fn forget_something(n: WithoutCopy) -> u64 { + let _ = n; + 0 +} +``` + +### Implementation + +The detector's implementation can be found at [this link](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/avoid-core-mem-forget). From 8160a82c3e876ee184d88f1d9b44f05b4bc2f196 Mon Sep 17 00:00:00 2001 From: Arturo Beccar-Varela <107512933+arturoBeccar@users.noreply.github.com> Date: Thu, 14 Dec 2023 16:28:07 -0300 Subject: [PATCH 3/4] Update README.md --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 09e8b4df..1d0afcef 100644 --- a/README.md +++ b/README.md @@ -44,6 +44,8 @@ cargo scout-audit | [overflow-check](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/overflow-check) | An arithmetic operation overflows or underflows the available memory allocated to the variable. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/overflow-check/overflow-check-1)| Critical | | [insufficiently-random-values](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/insufficiently-random-values) | Avoid using block attributes for random number generation to prevent manipulation. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/insufficiently-random-values/insufficiently-random-values-1)| Critical | | [unprotected-update-current-contract-wasm](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unprotected-update-current-contract-wasm) | If users are allowed to call `update_current_contract_wasm()`, they can intentionally modify the contract behaviour. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unprotected-update-current-contract-wasm/unprotected-update-current-contract-wasm-1)| Critical | +| [unprotected-update-current-contract-wasm](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unprotected-update-current-contract-wasm) | If users are allowed to call `update_current_contract_wasm()`, they can intentionally modify the contract behaviour. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unprotected-update-current-contract-wasm/unprotected-update-current-contract-wasm-1)| Critical | +| [avoid-core-mem-forget](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/avoid-core-mem-forget) | The use of `core::mem::forget()` could lead to memory leaks and logic errors. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/avoid-core-mem-forget/avoid-core-mem-forget-1) | Enhacement | ## Tests From d68d087397719cfd2fcc5a50a9811a9873848a30 Mon Sep 17 00:00:00 2001 From: Arturo Beccar-Varela <107512933+arturoBeccar@users.noreply.github.com> Date: Thu, 14 Dec 2023 16:30:33 -0300 Subject: [PATCH 4/4] Remove repeated unprotected-update-current-contract-wasm from README.md --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index 1d0afcef..64b3157f 100644 --- a/README.md +++ b/README.md @@ -44,7 +44,6 @@ cargo scout-audit | [overflow-check](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/overflow-check) | An arithmetic operation overflows or underflows the available memory allocated to the variable. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/overflow-check/overflow-check-1)| Critical | | [insufficiently-random-values](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/insufficiently-random-values) | Avoid using block attributes for random number generation to prevent manipulation. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/insufficiently-random-values/insufficiently-random-values-1)| Critical | | [unprotected-update-current-contract-wasm](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unprotected-update-current-contract-wasm) | If users are allowed to call `update_current_contract_wasm()`, they can intentionally modify the contract behaviour. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unprotected-update-current-contract-wasm/unprotected-update-current-contract-wasm-1)| Critical | -| [unprotected-update-current-contract-wasm](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/unprotected-update-current-contract-wasm) | If users are allowed to call `update_current_contract_wasm()`, they can intentionally modify the contract behaviour. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/unprotected-update-current-contract-wasm/unprotected-update-current-contract-wasm-1)| Critical | | [avoid-core-mem-forget](https://github.com/CoinFabrik/scout-soroban/tree/main/detectors/avoid-core-mem-forget) | The use of `core::mem::forget()` could lead to memory leaks and logic errors. | [1](https://github.com/CoinFabrik/scout-soroban/tree/main/test-cases/avoid-core-mem-forget/avoid-core-mem-forget-1) | Enhacement |