Skip to content

Commit

Permalink
Add CEI pattern static analysis (#3168)
Browse files Browse the repository at this point in the history
CEI stands for "Checks, Effects, Interactions". We check no storage
reads/writes (effects) occur after calling external contracts
(interaction). See this [blog
post](https://fravoll.github.io/solidity-patterns/checks_effects_interactions.html)
and [this
blogpost](https://chainsecurity.com/curve-lp-oracle-manipulation-post-mortem)
for more detail.

TODOs:
- [x] analyze libraries, scripts, and predicates (only contracts are
analyzed at the moment)
- [x] process assembly blocks
- [x] what is the effects of the `__revert` intrinsic
- [x] Treat nested code blocks
- [x] more tests
  * [x] Tests using `storage.var = ...`
  * [x] Tests using `asm` blocks directly
  * [x] Tests using complex control flow
* [x] Tests having multiple contract calls or multiple storage writes in
various scenarios
* [ ] Tests with nested code blocks (there is a very simple test, this
needs more testing in a separate PR)
- [x] check for storage reads after interaction 
   * [x] Storage read intrinsics
   * [x] `StorageAccess` expressions
   * [x] `asm` blocks using `srw` or `srwq`
* [x] `asm` blocks using
[`bal`](https://github.com/FuelLabs/fuel-specs/blob/master/specs/vm/instruction_set.md#bal-balance-of-contract-id)
because `bal` actually reads from storage
- [x] ~~CLI option or attribute to control the warnings~~ (an internal
discussion showed that we cannot do this at the moment, I guess this
should be handled as a separate issue)
- [ ] open an issue about unification of the currently existing storage
annotation checks and effects inference in this PR

close #10

Co-authored-by: Mohammad Fawaz <mohammadfawaz89@gmail.com>
  • Loading branch information
anton-trunov and mohammadfawaz authored Nov 10, 2022
1 parent 7a8ab39 commit 57b6d08
Show file tree
Hide file tree
Showing 40 changed files with 1,100 additions and 0 deletions.
5 changes: 5 additions & 0 deletions sway-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,11 @@ pub fn parsed_to_ast(
errors.push(e);
}

// CEI pattern analysis
let cei_analysis_warnings =
semantic_analysis::cei_pattern_analysis::analyze_program(&typed_program);
warnings.extend(cei_analysis_warnings);

// Check that all storage initializers can be evaluated at compile time.
let typed_wiss_res = typed_program.get_typed_program_with_initialized_storage_slots(
&mut ctx,
Expand Down
1 change: 1 addition & 0 deletions sway-core/src/semantic_analysis.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Type checking for Sway.
pub mod ast_node;
pub(crate) mod cei_pattern_analysis;
mod module;
pub mod namespace;
mod node_dependencies;
Expand Down
Loading

0 comments on commit 57b6d08

Please sign in to comment.