[ H ] Infinite loop in calculateNewIndex prevents tokens from being minted and rewards from being distributed

[code423n4]

Lines of code

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

Vulnerability details

Impact

Recursive calls from calculateNewIndex in MultiRewardDistributor will result in an infinite loop and out of gas errors, preventing tokens from being minted and rewards being sent to some users as disburseSupplierRewardsInternal will not be called.

Proof of Concept

Summarizing the interactions that occur when a user calls mint:

MToken -> Comptroller -> MultiRewardDistributor -> MToken

When calculating a new index in function calculateNewIndex, there is a call to an internal sub_ function, passing in blockTimestamp and _currentTimestamp

uint256 deltaTimestamps = sub_(blockTimestamp, uint256(_currentTimestamp));

However, the way the sub_ function is implemented, it will cause an infinite loop due to the recursive call to itself, which will throw an out of gas error and any gas used will be lost.

    function sub_(uint a, uint b) pure internal returns (uint) {
        return sub_(a, b, "subtraction underflow");
    }

Tools Used

Manual Review

Recommended Mitigation Steps

I would recommend adding a check to the sub_ function that prevents the recursive call when the condition is not met.

This is in line with the safeMath library implementation. (https://github.com/ConsenSysMesh/openzeppelin-solidity/blob/master/contracts/math/SafeMath.sol)

function sub_(uint a, uint b) pure internal returns (uint) {
    require(b <= a, "subtraction underflow");
    return a - b;
}

The same issue is existential in the mul, div and add_ functions within the function flow of

getOutstandingRewardsForUser, where recursive calls are made.

Where it is recommended to update the implementations in ExponentialNoError to the following:

function add_(uint a, uint b) pure internal returns (uint) {
    require(b >= a, "addition overflow");
    return a + b;
}

function mul_(uint a, uint b) pure internal returns (uint) {
    require(b <=a, "multiplication overflow");
    return a * b;
}

function div_(uint a, uint b) pure internal returns (uint) {
    require(b != 0, "division by zero");
    return a / b;
}

Assessed type

Under/Overflow

[0xSorryNotSorry]

The function overloading is only called once for the uint params and ends with;

    function sub_(uint a, uint b, string memory errorMessage) pure internal returns (uint) {
        require(b <= a, errorMessage);
        return a - b;
    }

However, the way the sub_ function is implemented, it will cause an infinite loop due to the recursive call to itself, which will throw an out of gas error and any gas used will be lost.

Thus, the above statement requires more proof.

[c4-pre-sort]

0xSorryNotSorry marked the issue as low quality report

[c4-judge]

alcueca marked the issue as unsatisfactory: Insufficient proof