code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-moonwell/blob/fced18035107a345c31c9a9497d0da09105df4df/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L422

Vulnerability details

Impact

Invariant that emission token doesn't already exist in a config can be broken with token that has multiple entry points.

Proof of Concept

In code, there is the check that a particular emission token is not listed again for a market: https://github.com/code-423n4/2023-07-moonwell/blob/fced18035107a345c31c9a9497d0da09105df4df/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L422.

Nevertheless, a token that has multiple entry points (addresses) would be able to circumvent this check. This is easy to be missed as the knowledge around tokens with multiple entry points is not widespread and has led to exploits in the past.

Here is some additional background: https://medium.com/chainsecurity/trueusd-compound-vulnerability-bc5b696d29e2/

Tools Used

Manual review

Recommended Mitigation Steps

Apply through manual validation whether a token has more than one entry point before allowing it to be added as an emission token. Educate the decision-makers on this topic. Add a (potentially governance-administered) whitelist of allowed emission tokens that either prevents adding these tokens at all or only whitelist one of their multiple entry points. Alternatively, add a blacklist of token addresses to not be used as emission tokens. A whitelist is highly preferable.

Assessed type

Invalid Validation

0xSorryNotSorry commented 1 year ago

Function _addEmissionConfig allows adding an additional reward stream to an MToken. Rewards can be provided for borrowing, lending, or both, as long as the reward speeds are less than the emission cap.

The submission doesn't demonstrate the vulnerability referring to the context above. Insufficient proof.

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as low quality report

alcueca commented 1 year ago

Indeed, missing proof of impact. Still, acceptable as grade-b QA.

c4-judge commented 1 year ago

alcueca changed the severity to QA (Quality Assurance)