Consider making all storage writes before calling another contract

Sway
1

We’ve organized a project consisting of five different smart contracts, with some serving as dependencies for others. Our project structure appears as follows:

/ Abis folder
/ contract 1
/ contract 2
/ contract 3
...

These dependencies are declared in the Forc.toml file subsequently:

[dependencies]
i64 = { path = "../libs/i64" }
clearing_house_abi = { path = "../abi/clearing_house_abi" }
account_balance_abi = { path = "../abi/account_balance_abi" }
vault_abi = { path = "../abi/vault_abi" }

However, we’ve encountered warnings. Can you offer guidance on avoiding or disabling these warnings?

Storage write after external contract interaction in function or method "settle_bad_debt". Consider making all storage writes before calling another contract
1 Like
2

Hello @sway, I hope you’re doing well! :slight_smile:

I wanted to bring to your attention some critical warnings you’re encountering. They indicate a violation of the CEI pattern (Checks, Effects, Interactions) within your settle_bad_debt function. This is a significant concern as it could lead to a reentrancy bug, especially if the function’s current behavior of calling another contract and then accessing storage is unintentional. The specific warning you’re receiving is:

Storage write after external contract interaction in function or method "settle_bad_debt". Consider making all storage writes before calling another contract.

It’s crucial for you to thoroughly review the CEI section in the Sway documentation, which will provide greater insight into this issue. Here’s the link for easy access: CEI Pattern Violation - Static Analysis.

Additionally, I recommend these two resources for a deeper understanding and context:

  1. Curve LP Oracle Manipulation Post-Mortem by ChainSecurity
  2. Understanding Reentrancy by Smart Contract Security Forum

Please let me know if you need any further clarification or assistance!

1 Like
3

Wow, I didn’t recognize this is that important error.
Thanks a lot, will study

4

Hi, thanks for your answer, but what about cross-contract calls to read-only data (where there is no write or require)? Do you think they can be called before changing the state safely?

5

Hi @fuel, it’s challenging to provide a definitive answer, and I strongly recommend consulting your auditors for such queries, as the specifics may vary based on how your code is structured. The primary concern isn’t about the read-only aspect, but rather the risk of a vector through which someone could potentially trigger a recursive call to your function using reentrancy.

Consider a scenario where you make a read-only call to an external contract. If that external contract is upgradeable, there’s a risk that it could be maliciously altered without your knowledge, thereby completely changing the intended behavior. This potential modification poses a significant security risk.

6

Ok, can you please give examples of when it is safe to simulate similar calls to other contracts before updating the state or changing balances?

7

Could you please provide information abt fn reentrancy_guard(); from
use reentrancy::reentrancy_guard;. Do you think that it’s enough to avoid this kind of attack?

8

according to the docs wrt that function (reentrancy_guard):

While this can protect against both single-function reentrancy and cross-function reentrancy attacks, it WILL NOT PREVENT a cross-contract reentrancy attack.

Source: https://github.com/FuelLabs/sway-libs/blob/master/libs/reentrancy/README.md

1 Like
9

Building on @elena’s explanation, a non-reentrant modifier in one contract is incapable of preventing reentrant calls initiated by another contract. Its scope is limited to ensuring that functions within the same contract are not re-entered. Again I strongly advise consulting with your auditors regarding these warnings later to ensure comprehensive security measures.

In addition, I’d like to point out that another team is developing a static analyzer for Sway, which can be a crucial tool for your project. This analyzer, available at https://github.com/camden-smallwood/sway-analyzer, is designed to scan your code for potential security flaws. It functions similarly to Slither for Solidity, created by trailofbits, and could be instrumental in identifying vulnerabilities early in the development process.

10

Thanks for the suggestion maybe you can help me with the analyzer, I didn’t get how to run it.
I just found installation in it’s readme

11

To analyze your Sway files, execute the command sway-analyzer --directory src, ensuring that src is the directory containing your files.

For a list of available options and flags for running the analyzer, use sway-analyzer -h.

sway-analyzer 0.1.0

USAGE:
    sway-analyzer [OPTIONS]

FLAGS:
    -h, --help       Prints help information
    -V, --version    Prints version information

OPTIONS:
        --detectors <detectors>...           The specific detectors to utilize. (Optional; Leave unused for all)
        --directory <directory>              The path to the Forc project directory. (Optional)
        --display-format <display-format>    The display format of the report. Can be "Text" or "Json". (Default = Text)
        --files <files>...                   The paths to the Sway source files. (Optional)
        --sorting <sorting>                  The order to sort report entries by. Can be "Line" or "Severity". (Default
                                             = Line)
12

oh, I understood, It was not obvious, thanks!

1 Like
13

Let me know how else I can help :slight_smile: :palm_tree: