Submitted by immeas, also found by mert_eren
When distributing rewards for a market, each emissionConfig is looped over and sent rewards for. disburseBorrowerRewardsInternal as an example, the same holds true for disburseSupplierRewardsInternal:
https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L1147-L1247
If one transfer reverts the whole transaction fails and no rewards will be paid out for this user. Hence if there is a malicious token that would revert on transfer it would cause no rewards to paid out. As long as there is some rewards accrued for the malicious emissionConfig.
The users with already unclaimed rewards for this emissionConfig would have their rewards permanently locked.
The admin (TemporalGovernor) of MultiRewardsDistributor could update the reward speed for the token to 0 but that would just prevent further damage from being done.
Upgradeable tokens arent unusual and hence the token might seem harmless to begin with but be upgraded to a malicious implementation that reverts on transfer.
Test in MultiRewardDistributor.t.sol, MultiRewardSupplySideDistributorUnitTest, most of the test is copied from testSupplierHappyPath with the addition of MaliciousToken:
Consider adding a way for admin to remove an emissionConfig.
Alternatively, the reward transfer could be wrapped in a try/catch and returning _amount in catch. Be mindful to only allow a certain amount of gas to the transfer then as otherwise the same attack works with consuming all gas available.
ElliotFriedman (Moonwell) disputed and commented:
alcueca (Judge) commented:
lyoungblood (Warden) commented:
alcueca (Judge) commented:
