[ M ] Denial of Service in mintAllowed

[code423n4]

Lines of code

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

Vulnerability details

Impact

If an external user directly calls the mintAllowed function on the Comptroller contract, they can potentially manipulate the supply cap check by providing a mintAmount that, when added to the current total supplies, exceeds the supply cap.

Proof of Concept

The relevant code block in the mintAllowed function is:

uint nextTotalSupplies = add_(totalSupplies, mintAmount);
require(nextTotalSupplies < supplyCap, "market supply cap reached");

The main attack vector, as previously discussed, is the ability of an external contract to call the mintAllowed function directly, bypassing the intended flow of going through the mintFresh function in the ``MToken contract```.

This allows an attacker to artificially trigger the "market supply cap reached" error by supplying a large mintAmount that exceeds the supply cap. By doing so, the attacker can prevent legitimate users from successfully minting tokens and effectively cause a denial-of-service (DoS) scenario.

In essence, the vulnerability lies in the fact that mintAllowed is an externally accessible function that can be called independently, potentially leading to an unexpected and disruptive interaction with the token minting process.

Tools Used

Manual Review

Recommended Mitigation Steps

Add a modifier to the mintAllowed function to ensure that it can only be called by the MToken contract.

Assessed type

Access Control

[0xSorryNotSorry]

This allows an attacker to artificially trigger the "market supply cap reached" error by supplying a large mintAmount that exceeds the supply cap.

The above statement requires detailed scenario and the call trace to picture the attack.

More proof required.

[c4-pre-sort]

0xSorryNotSorry marked the issue as low quality report

[alcueca]

mintAmount is unused in mintAllowed, so invalid.

Still, it is wrong for a "check" function to be transactional, and to potentially allow anyone to change state. @ElliotFriedman, I would suggest that you revisit this function, and possibly change the natspec to note that the function updadtes the flywheel and therefore changes state.

[c4-judge]

alcueca marked the issue as unsatisfactory: Invalid