It allows the admin to steal deposited funds from users
It removes mTokens used as collateral to back borrows
Proof of Concept
Assume there is a mToken at address 0x123 that has issued some tokens.
The admin calls _rescueFunds(0x123, uint.max). This will transfer ALL the tokens at 0x123 to the admin.
Now the admin has all the tokens that were previously held by users of the mToken contract. The admin can do whatever they want with these tokens, disrupting the normal operation of the mToken and protocol.
The _rescueFunds function does not check that the _tokenAddress is not a mToken. It blindly transfers the tokens.
Tools Used
Manual
Recommended Mitigation Steps
_rescueFunds should have an explicit whitelist of allowed tokens, or blacklist of protocol tokens that cannot be swept. And governance controls like timelocks should be required for critical token rescues.
Lines of code
https://github.com/code-423n4/2023-07-moonwell/blob/fced18035107a345c31c9a9497d0da09105df4df/src/core/Comptroller.sol#L959-L969
Vulnerability details
Impact
It allows the admin to steal deposited funds from users It removes mTokens used as collateral to back borrows
Proof of Concept
Assume there is a mToken at address 0x123 that has issued some tokens. The admin calls _rescueFunds(0x123, uint.max). This will transfer ALL the tokens at 0x123 to the admin. Now the admin has all the tokens that were previously held by users of the mToken contract. The admin can do whatever they want with these tokens, disrupting the normal operation of the mToken and protocol. The _rescueFunds function does not check that the _tokenAddress is not a mToken. It blindly transfers the tokens.
Tools Used
Manual
Recommended Mitigation Steps
_rescueFunds should have an explicit whitelist of allowed tokens, or blacklist of protocol tokens that cannot be swept. And governance controls like timelocks should be required for critical token rescues.
Assessed type
Other