code423n4 commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-07-moonwell/blob/fced18035107a345c31c9ay9497d0da09105df4df/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L471-L487

Vulnerability details

Impact

abuse of sweeping arbitrary tokens.

Proof of Concept

The _rescueFunds function allows sweeping any tokens, when it should only allow sweeping of the underlying asset

Tools Used

Manual

Recommended Mitigation Steps

Remove the _tokenAddress parameter - no need to specify which token, only sweep underlying
Add a check of getCashPrior() >= amount to ensure there are sufficient underlying funds
Use doTransferOut(admin, amount) to transfer underlyings instead of arbitrary tokens
Use a onlyAdmin modifier for access control rather than checking msg.sender

Assessed type

Other

0xSorryNotSorry commented 11 months ago

OOS --> [M‑04] The owner is a single point of failure and a centralization risk

c4-pre-sort commented 11 months ago

0xSorryNotSorry marked the issue as low quality report

c4-sponsor commented 11 months ago

lyoungblood marked the issue as sponsor disputed

alcueca commented 10 months ago

The assumption here is that _rescueFunds is not supposed to be able to sweep everything. It would be very useful to rescue funds in a crisis situation to avoid being hacked.

c4-judge commented 10 months ago

alcueca marked the issue as unsatisfactory: Invalid

code-423n4 / 2023-07-moonwell-findings

The _rescueFunds function allows sweeping any tokens, when it should only allow sweeping of the underlying asset #401