Merge branch 'master' into nightly-ci

Author: SwayStar123

CHANGELOG.md

@@ -11,13 +11,13 @@ Description of the upcoming release here.
 
 ### Added
 
-- Something new here 1
-- Something new here 2
+- [#309](https://github.com/FuelLabs/sway-libs/pull/309) Adds fallback function test cases to the Reentrancy Guard Library.
 
 ### Changed
 
 - [#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
 

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.

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
 ///

tests/Forc.lock

@@ -183,6 +183,11 @@ dependencies = [
     "std",
 ]
 
+[[package]]
+name = "reentrancy_fallback_abi"
+source = "path+from-root-F53252C7DB7025EE"
+dependencies = ["std"]
+
 [[package]]
 name = "reentrancy_target_abi"
 source = "member"
@@ -193,6 +198,7 @@ name = "reentrancy_target_contract"
 source = "member"
 dependencies = [
     "reentrancy_attacker_abi",
+    "reentrancy_fallback_abi",
     "reentrancy_target_abi",
     "std",
     "sway_libs",

tests/src/reentrancy/mod.rs

@@ -161,4 +161,20 @@ mod revert {
             .await
             .unwrap();
     }
+
+    #[tokio::test]
+    #[should_panic(expected = "NonReentrant")]
+    async fn can_block_fallback_reentrancy() {
+        let wallet = launch_provider_and_get_wallet().await.unwrap();
+        let (attacker_instance, _) = get_attacker_instance(wallet.clone()).await;
+        let (instance, target_id) = get_target_instance(wallet).await;
+
+        attacker_instance
+            .methods()
+            .launch_thwarted_attack_4(target_id)
+            .with_contracts(&[&instance])
+            .call()
+            .await
+            .unwrap();
+    }
 }

tests/src/reentrancy/reentrancy_attack_fallback_abi/Forc.toml

@@ -0,0 +1,7 @@
+[project]
+authors = ["Fuel Labs <contact@fuel.sh>"]
+entry = "main.sw"
+license = "Apache-2.0"
+name = "reentrancy_fallback_abi"
+
+[dependencies]

tests/src/reentrancy/reentrancy_attack_fallback_abi/src/main.sw

@@ -0,0 +1,5 @@
+library;
+
+abi FallbackAttack {
+    fn nonexistant_function(contract_id: ContractId);
+}

tests/src/reentrancy/reentrancy_attacker_abi/src/main.sw

@@ -6,6 +6,7 @@ abi Attacker {
     fn launch_thwarted_attack_2(target: ContractId);
     #[storage(write)]
     fn launch_thwarted_attack_3(target: ContractId, helper: ContractId);
+    fn launch_thwarted_attack_4(target: ContractId);
     fn innocent_call(target: ContractId);
     fn evil_callback_1() -> bool;
     fn evil_callback_2();

tests/src/reentrancy/reentrancy_attacker_contract/src/main.sw

@@ -1,6 +1,6 @@
 contract;
 
-use std::auth::*;
+use std::{auth::*, call_frames::*,};
 
 use reentrancy_target_abi::Target;
 use reentrancy_attacker_abi::Attacker;
@@ -41,6 +41,10 @@ impl Attacker for Contract {
             .cross_contract_reentrancy_denied();
     }
 
+    fn launch_thwarted_attack_4(target: ContractId) {
+        abi(Target, target.bits()).fallback_contract_call();
+    }
+
     fn innocent_call(target: ContractId) {
         abi(Target, target.bits()).guarded_function_is_callable();
     }
@@ -71,3 +75,11 @@ impl Attacker for Contract {
 
     fn innocent_callback() {}
 }
+
+#[fallback]
+fn fallback() {
+    let call_args = called_args::<ContractId>();
+
+    let target_abi = abi(Target, call_args.bits());
+    target_abi.fallback_contract_call();
+}

tests/src/reentrancy/reentrancy_target_abi/src/main.sw

@@ -7,4 +7,5 @@ abi Target {
     fn intra_contract_call();
     fn guarded_function_is_callable();
     fn cross_contract_reentrancy_denied();
+    fn fallback_contract_call();
 }

tests/src/reentrancy/reentrancy_target_contract/Forc.toml

@@ -6,5 +6,6 @@ name = "reentrancy_target_contract"
 
 [dependencies]
 reentrancy_attacker_abi = { path = "../reentrancy_attacker_abi" }
+reentrancy_fallback_abi = { path = "../reentrancy_attack_fallback_abi" }
 reentrancy_target_abi = { path = "../reentrancy_target_abi" }
 sway_libs = { path = "../../../../libs" }

tests/src/reentrancy/reentrancy_target_contract/src/main.sw

@@ -5,6 +5,7 @@ use sway_libs::reentrancy::*;
 
 use reentrancy_attacker_abi::Attacker;
 use reentrancy_target_abi::Target;
+use reentrancy_fallback_abi::FallbackAttack;
 
 // Return the sender as a ContractId or panic:
 pub fn get_msg_sender_id_or_panic() -> ContractId {
@@ -63,4 +64,14 @@ impl Target for Contract {
             .bits())
             .evil_callback_4();
     }
+
+    fn fallback_contract_call() {
+        // panic if reentrancy detected
+        reentrancy_guard();
+
+        // this call transfers control to the attacker contract, allowing it to execute arbitrary code.
+        abi(FallbackAttack, get_msg_sender_id_or_panic()
+            .bits())
+            .nonexistant_function(ContractId::this());
+    }
 }