Skip to content

Commit

Permalink
Document CEI pattern analysis in Sway book (FuelLabs#3393)
Browse files Browse the repository at this point in the history
close FuelLabs#3298

TODO:
- [x] change the code example to a more realistic one
- [x] include code from a separate file using `{{#include ...}}`
  • Loading branch information
anton-trunov authored Nov 23, 2022
1 parent 7fb338f commit 9dbfc03
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 0 deletions.
53 changes: 53 additions & 0 deletions docs/src/blockchain-development/calling_contracts.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,59 @@ impl ContractB for Contract {
}
```

### CEI pattern violation static analysis

Another way of avoiding re-entrancy-related attacks is to follow the so-called
_CEI_ pattern. CEI stands for "Checks, Effects, Interactions", meaning that the
contract code should first perform safety checks, also known as
"pre-conditions", then perform effects, i.e. modify or read the contract storage
and execute external contract calls (interaction) only at the very end of the
function/method.

Please see this [blog post](https://fravoll.github.io/solidity-patterns/checks_effects_interactions.html)
for more detail on some vulnerabilities in case of storage modification after
interaction and this [blog post](https://chainsecurity.com/curve-lp-oracle-manipulation-post-mortem) for
more information on storage reads after interaction.

The Sway compiler implements a check that the CEI pattern is not violated in the
user contract and issues warnings if that's the case.

For example, in the following contract the CEI pattern is violated, because an
external contract call is executed before a storage write.

```sway
{{#include ../../../examples/cei_analysis/src/main.sw}}
```

Here, `other_contract` is defined as follows:

```sway
{{#include ../../../examples/cei_analysis/src/other_contract.sw}}
```

The CEI pattern analyzer issues a warning as follows, pointing to the
interaction before a storage modification:

```sh
warning
--> /path/to/contract/main.sw:28:9
|
26 |
27 | let caller = abi(OtherContract, external_contract_id.into());
28 | caller.external_call { coins: bal }();
| _________-
29 | |
30 | | // Storage update _after_ external call
31 | | storage.balances.insert(sender, 0);
| |__________________________________________- Storage modification after external contract interaction in function or method "withdraw". Consider making all storage writes before calling another contract
32 | }
33 | }
|
____
```
In case there is a storage read after an interaction, the CEI analyzer will issue a similar warning.
## Differences from the EVM
While the Fuel contract calling paradigm is similar to the EVM's (using an ABI, forwarding gas and data), it differs in _two_ key ways:
Expand Down
2 changes: 2 additions & 0 deletions examples/cei_analysis/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
out
target
13 changes: 13 additions & 0 deletions examples/cei_analysis/Forc.lock
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[[package]]
name = 'cei_analysis'
source = 'member'
dependencies = ['std']

[[package]]
name = 'core'
source = 'path+from-root-52AF21993C880521'

[[package]]
name = 'std'
source = 'path+from-root-52AF21993C880521'
dependencies = ['core']
8 changes: 8 additions & 0 deletions examples/cei_analysis/Forc.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[project]
authors = ["Fuel Labs <[email protected]>"]
entry = "main.sw"
license = "Apache-2.0"
name = "cei_analysis"

[dependencies]
std = { path = "../../sway-lib-std" }
33 changes: 33 additions & 0 deletions examples/cei_analysis/src/main.sw
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
contract;

dep other_contract;

use other_contract::*;

use std::auth::msg_sender;

abi MyContract {
#[storage(read, write)]
fn withdraw(external_contract_id: ContractId);
}

storage {
balances: StorageMap<Identity, u64> = StorageMap {},
}

impl MyContract for Contract {
#[storage(read, write)]
fn withdraw(external_contract_id: ContractId) {
let sender = msg_sender().unwrap();
let bal = storage.balances.get(sender);

assert(bal > 0);

// External call
let caller = abi(OtherContract, external_contract_id.into());
caller.external_call { coins: bal }();

// Storage update _after_ external call
storage.balances.insert(sender, 0);
}
}
5 changes: 5 additions & 0 deletions examples/cei_analysis/src/other_contract.sw
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
library other_contract;

abi OtherContract {
fn external_call();
}

0 comments on commit 9dbfc03

Please sign in to comment.