-
Notifications
You must be signed in to change notification settings - Fork 10
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
fix: check if hook installed before emergency uninstall #175
Conversation
Changes to gas cost
🧾 Summary (5% most significant diffs)
Full diff report 👇
|
fix: check if hook installed before emergency uninstall
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #175 +/- ##
==========================================
+ Coverage 94.58% 94.86% +0.28%
==========================================
Files 14 14
Lines 720 721 +1
Branches 138 164 +26
==========================================
+ Hits 681 684 +3
+ Misses 39 37 -2
Continue to review full report in Codecov by Sentry.
|
🤖 Slither Analysis Report 🔎Slither report
# Slither report
_This comment was automatically generated by the GitHub Actions workflow._
THIS CHECKLIST IS NOT COMPLETE. Use
unused-return🟡 Impact: Medium
base/ERC7739Validator.sol#L176-L206 constable-statesImpact: Optimization
|
Problem:
The emergencyUninstallHook() function is meant to allow the account owner to remove a hook from the account (minus the pre and post checks on the hook).
It is an alternate to the uninstallModule() function which offers hook checks on calls to uninstall any type of modules.
A timelock has been placed to only allow the account to emergency-uninstall the hook after 1 day of placing an uninstall request. But emergencyUninstallHook() fails to check that the hook was actually installed.
This allows the timelock to be bypassed through the following steps :
Call emergencyUninstallHook() to place an uninstall request even before installing the hook => this records a timestamp corresponding to the hook address
After nearly a day has passed, install the hook and use it
Immediately call emergencyUninstallHook() to utilize the request that was placed before
As only a day has passed, the call will go through
The timelock has been effectively bypassed by the account
Solution:
UninstallModule() on Nexus.sol checks that the module they are trying to uninstall through the call is actually installed. Add the same check to emergencyUninstallHook()
Also replaced the event EmergencyHookUninstallRequest in reset case with EmergencyHookUninstallRequestReset (Cantina 59)