In the original issue, due to a combination of the usage of transfer and the usage of a two-step withdrawal process, people who used multisig would be able to complete the first step of the two-step withdrawal process, but they would not be able to complete the second step of the two-step withdrawal process, leading to funds lost.
This mitigation succesfully mitigates the original issue - usage of call() will forward all available gas, which would prevent a lockup like in the original issue.
Suggestions
If this has not been done already, add documentation telling users to be extra careful when using multisigs. Even though this mitigation mitigates the original issue, Safe multisigs are complex and not one-size fits all. Safe multisigs can have different Guards/Modules that change the inner-workings of a Safe multisig, so it would be good to give the multisig users a heads-up.
Lines of code
Vulnerability details
Original Issue Summary
In the original issue, due to a combination of the usage of
transfer
and the usage of a two-step withdrawal process, people who used multisig would be able to complete the first step of the two-step withdrawal process, but they would not be able to complete the second step of the two-step withdrawal process, leading to funds lost.Mitigation
This mitigation proposes the usage of
call()
instead oftransfer()
and checks if thecall()
returns atrue
value.Comments
This mitigation succesfully mitigates the original issue - usage of
call()
will forward all available gas, which would prevent a lockup like in the original issue.Suggestions
If this has not been done already, add documentation telling users to be extra careful when using multisigs. Even though this mitigation mitigates the original issue, Safe multisigs are complex and not one-size fits all. Safe multisigs can have different Guards/Modules that change the inner-workings of a Safe multisig, so it would be good to give the multisig users a heads-up.
Conclusion
LGTM