You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
An oversight was found in removeOwnerAtIndex where all owners could be removed including the last owner. This presents a significant risk that can potentially lead to loss of funds if all owners loose access.
The number 1 is used to check if there's only one owner left. While understandable, using magic numbers directly in code can be considered poor practice. Define a constant at the beginning of your contract to give context to this value.
Lines of code
Vulnerability details
https://github.com/code-423n4/2024-03-coinbase/blob/e0573369b865d47fed778de00a7b6df65ab1744e/src/SmartWallet/MultiOwnable.sol#L102
Issue Report
QA-01: All Smart Wallet funds will be lost if users remove all owners
Details
Issue#181
An oversight was found in
removeOwnerAtIndex
where all owners could be removed including the last owner. This presents a significant risk that can potentially lead to loss of funds if all owners loose access.Mitigation
PR#43
removeOwnerAtIndex
now includes a check to ensure that the operation does not proceed if attempting to remove the last owner.Loc:
Loc:
Suggestion
The number 1 is used to check if there's only one owner left. While understandable, using magic numbers directly in code can be considered poor practice. Define a constant at the beginning of your contract to give context to this value.
Conclusion
This fix succesfully mitigates the issue#181
The text was updated successfully, but these errors were encountered: