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

function.is_protected: Improve heuristic #401

Closed
montyly opened this issue Feb 24, 2020 · 0 comments
Closed

function.is_protected: Improve heuristic #401

montyly opened this issue Feb 24, 2020 · 0 comments
Labels
enhancement New feature or request

Comments

@montyly
Copy link
Member

montyly commented Feb 24, 2020

function.is_protected is heavily used by the inbuilt-analyses and many detectors:

def is_protected(self):
"""
Determine if the function is protected using a check on msg.sender
Only detects if msg.sender is directly used in a condition
For example, it wont work for:
address a = msg.sender
require(a == owner)
Returns
(bool)
"""
if self.is_constructor:
return True
conditional_vars = self.all_conditional_solidity_variables_read(include_loop=False)
args_vars = self.all_solidity_variables_used_as_args()
return SolidityVariableComposed('msg.sender') in conditional_vars + args_vars

This function was introduced ~1,5 year ago, and gives too many false positives and false negatives (ex: require(msg.sender == tx.origin), #399). We need to improve it. some ideas:

  • Use all_slithir_operations, and check for direct comparison with msg.sender
  • Find a way to capture correctly msg.sender that are used in a call/modifier. The data dependency can't be used here, as it relies on is_protected. An alternative would be to compute first the data dependency without the protected information, and recompute it a second time. This would give better results but will require some refactoring.
  • We should make it a property and memoize it
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant