Missing Input Validation and Error Definition

[code423n4]

Lines of code

https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L827-L844 https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L865-L883

Vulnerability details

Impact

Missing Input Validation and Error Definition of _globalSupplyIndex & globalBorrowIndex in L827-L844 & L865-L883 respectively of MultiRewardDistributor.sol could create complications as "sub()" function of L844 & L883 would stop execution in cases of underflow if _globalSupplyIndex & _globalBorrowIndex is zero. An input and error check is needed to clarify this when such happens.

Proof of Concept

https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L827-L844 https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/MultiRewardDistributor/MultiRewardDistributor.sol#L865-L883

827. function calculateSupplyRewardsForUser(
        ...
        uint224 _globalSupplyIndex,
        ...
    ) internal view returns (uint256) {
       ...
844.            mantissa: sub_(_globalSupplyIndex, userSupplyIndex)

This code shows that no check was done for _globalSupplyIndex and tracing back this parameter from L912 of function calculateNewIndex(...) to L827 of function calculateSupplyRewardsForUser(...) and to L844 where it is being being used, it can be deduced that a zero value is possible and no check or error type validation was done for _globalSupplyIndex at any point in time, this applies to _globalBorrowIndex too

Tools Used

Solidity, Hardhat

Recommended Mitigation Steps

A Zero check is needed for _globalSupplyIndex & _globalBorrowIndex before any operation is carried out.

Assessed type

Error

[0xSorryNotSorry]

The scenario is handled via func overloading as below;

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

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

Invalid assumption.

[c4-pre-sort]

0xSorryNotSorry marked the issue as low quality report

[alcueca]

It is hard to understand what exactly the issue is here. At most it would be a grade-b QA finding leading to some documentation or natspec fix. On the grounds of low quality, I think it is fair to just invalidate this finding.

[c4-judge]

alcueca marked the issue as unsatisfactory: Insufficient quality