Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-entrancy detector + Control Flow Graph #752

Open
wants to merge 48 commits into
base: dev
Choose a base branch
from

Conversation

TilakMaddy
Copy link
Collaborator

@TilakMaddy TilakMaddy commented Oct 4, 2024

Fix #313

  • Control Flow Graph implementation
  • State change after external call detector
  • Emit after external call detector

Long version:

At the moment Control Flow Graph module exposes -

  • Cfg::from_function_body(f: &FunctionDefinition)
  • Cfg::from_modifier_body(f: &ModifierDefinition)

Returns a tuple (cfg, start_node, end_node)

The first argument is a cfg object that gives you access to the adjacency list representation of the control flow graph.
The second argument is used to point you to the place in the cfg where the function body's cfg start.

Those 2 things are used to build and traverse the control flow graph. In case you want to see the corresponding AST node, you can call cfg_node.reflect(context) on the CFG node. It will return ASTNode. Thanks to this, you can make use of existing ASTNode libraries and helpers.

Also attached to this PR are 2 reentrancy detectors plus an incorrect use of modifier detector that server as examples to see learn how cfg can be leveraged in various detectors

NOTE:
It constructs CFG from f's body only. It's not called Cfg::from_function by choice as that would involve decomposing the entire function (which involves resolving internal functions, resolving modifiers, etc). We don't have that ability yet

(Same logic goes for modifiers)

@TilakMaddy TilakMaddy changed the title Feature/control flow graphs plus re entrancy detector Re-entrancy detector introduced with Control Flow Graph Oct 6, 2024
@TilakMaddy TilakMaddy changed the title Re-entrancy detector introduced with Control Flow Graph Re-entrancy detector + Control Flow Graph Oct 6, 2024
@TilakMaddy TilakMaddy marked this pull request as ready for review October 6, 2024 16:23
@TilakMaddy
Copy link
Collaborator Author

@alexroan Please review the detector test file, and see if you can break it ! Let me know if you can think of some edge cases not covered

@TilakMaddy

This comment was marked as resolved.

@TilakMaddy

This comment was marked as resolved.

@TilakMaddy TilakMaddy marked this pull request as draft October 7, 2024 09:54
@TilakMaddy TilakMaddy marked this pull request as ready for review October 7, 2024 10:10
@TilakMaddy
Copy link
Collaborator Author

TilakMaddy commented Oct 18, 2024

Just realized I need to put a filter on the external calls to exclude the ones that are library calls because they are delegate calls and they can't reenter

UPDATE
Nope that wasn't true .. delegate calls can re-enter by this.XX() So I'm not making any changes :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

High Detector: Checks-effects-interactions
1 participant