From f901f5097690b060ae2d6df469f893112d579150 Mon Sep 17 00:00:00 2001 From: Cameron Carstens Date: Wed, 27 Nov 2024 01:56:14 +0700 Subject: [PATCH] Remove comments on cross-contract reentrancy venerability (#308) ## Type of change - Documentation ## Changes The following changes have been made: - Removed cautionary note on cross-contract vulnerability in inline docs - Removed comment on cross-contract vulnerability in documentation - Added note stating cross-contract reentrancy not possible but to still exercise caution ## Notes - Cross-contract reentrancy occurs when a contract like a vault issues and manages assets for token contract. However, as Fuel uses native assets, no contract call must be made to update balances. Therefore it is not possible to perform a cross-contract reentrancy attack. - A cautionary note on relying on other contracts for state has been added as this can introduce dependency attacks. ## Related Issues Closes #307 ## 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). - [x] 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. - [x] I have updated the changelog to reflect the changes on this PR. --- CHANGELOG.md | 1 + docs/book/src/reentrancy/index.md | 11 +++++------ libs/src/reentrancy.sw | 2 -- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23e1db58..bf537a21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ Description of the upcoming release here. - [#305](https://github.com/FuelLabs/sway-libs/pull/305) Updates to forc `v0.66.2`, fuel-core `v0.40.0`, and fuels-rs `v0.66.9`. - [#306](https://github.com/FuelLabs/sway-libs/pull/306) Updates the SRC-7 naming to Onchain Native Asset Metadata Standard. +- [#308](https://github.com/FuelLabs/sway-libs/pull/308) Removes comments on Cross-Contract Reentrancy vulnerability. ### Fixed diff --git a/docs/book/src/reentrancy/index.md b/docs/book/src/reentrancy/index.md index 9d47d3a3..a7ad9d6f 100644 --- a/docs/book/src/reentrancy/index.md +++ b/docs/book/src/reentrancy/index.md @@ -2,17 +2,12 @@ The Reentrancy Guard Library provides an API to check for and disallow reentrancy on a contract. A reentrancy attack happens when a function is externally invoked during its execution, allowing it to be run multiple times in a single transaction. -The reentrancy check is used to check if a contract ID has been called more than -once in the current call stack. +The reentrancy check is used to check if a contract ID has been called more than once in the current call stack. A reentrancy, or "recursive call" attack can cause some functions to behave in unexpected ways. This can be prevented by asserting a contract has not yet been called in the current transaction. An example can be found [here](https://swcregistry.io/docs/SWC-107). For implementation details on the Reentrancy Guard Library please see the [Sway Libs Docs](https://fuellabs.github.io/sway-libs/master/sway_libs/reentrancy/index.html). -## Known Issues - -While this can protect against both single-function reentrancy and cross-function reentrancy attacks, it WILL NOT PREVENT a cross-contract reentrancy attack. - ## Importing the Reentrancy Guard Library In order to use the Reentrancy Guard library, Sway Libs must be added to the `Forc.toml` file and then imported into your Sway project. To add Sway Libs as a dependency to the `Forc.toml` file in your project please see the [Getting Started](../getting_started/index.md). @@ -45,3 +40,7 @@ To check if the current caller is a reentrant, you may call the `is_reentrant()` ```sway {{#include ../../../../examples/reentrancy/src/main.sw:is_reentrant}} ``` + +## Cross Contract Reentrancy + +Cross-Contract Reentrancy is not possible on Fuel due to the use of Native Assets. As such, no contract calls are performed when assets are transferred. However standard security practices when relying on other contracts for state should still be applied, especially when making external calls. diff --git a/libs/src/reentrancy.sw b/libs/src/reentrancy.sw index 39757f41..2737e5ac 100644 --- a/libs/src/reentrancy.sw +++ b/libs/src/reentrancy.sw @@ -15,8 +15,6 @@ use std::registers::frame_ptr; /// /// Not needed if the Checks-Effects-Interactions (CEI) pattern is followed (as prompted by the /// compiler). -/// > Caution: While this can protect against both single-function reentrancy and cross-function -/// reentrancy attacks, it WILL NOT PREVENT a cross-contract reentrancy attack. /// /// # Examples ///