-
Notifications
You must be signed in to change notification settings - Fork 982
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
add _disableInitializers() detection #1344
Conversation
Is there a reason you check all internal calls instead of just the contract's constructor? EDIT: Oh nvm, this is only checking the constructor and not all functions already... Would you mind adding test cases here and generating the output using |
I was looking at the function object and |
for f in functions: | ||
for m in f.modifiers: | ||
if m.name == "initializer": | ||
return True | ||
# filtering out SolidityFunction from the internal calls as we don't need to match against those names | ||
internal_func_calls = [c for c in f.all_internal_calls() if not isinstance(c, SolidityFunction)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This loops over the list twice. I think it'd be better to remove the list comprehension and iterate once over f.all_internal_calls()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good suggestion!
Can you run pylint and black? You can obtain the correct versions by running |
Co-authored-by: alpharush <0xalpharush@protonmail.com>
Installing |
Tests + coverage came out good 👍️ |
This is great, thanks @plotchy |
Enhance
unprotected_upgradeable
detector to remove_disableInitializers()
false positive.Related to #1236