veRWA - HChang26's results

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/main/src/LendingLedger.sol#L153-L182

Vulnerability details

Impact

Claim() will revert when marketBalance = 0. Caller will waste gas doing complex computations and the function call will revert anyways.

Proof of Concept

Let's take a look at claim(). The recommended input is as follow: / @param _claimFromTimestamp From which epoch (provided as timestmap) should the claim start. Usually, this parameter should be set to 0, in which case the epoch of the last claim will be used. / However, it can be useful to skip certain epochs, e.g. when the balance was very low or 0 (after everything was withdrawn) and the gas usage should be reduced. / Note that all rewards are forfeited forever if epochs are explicitly skipped by providing this parameter / @param _claimUpToTimestamp Until which epoch (provided as timestamp) should the claim be applied. If it is higher than the timestamp of the previous epoch, this will be used / Set to type(uint256).max to claim all possible epochs.

marketBalance may be 0 for specific epoch in between _claimFromTimestamp and _claimUpToTimestamp period. Solidity will revert when a dividing by 0. The only way to get around this issue, is to make multiple claim() calls avoiding all epochs where marketBalance = 0 across different markets. However, this does seem unrealistic especially for a regular user. It will also likely cost more gas.

if (claimEnd >= claimStart) {
            // This ensures that we only set userClaimedEpoch when a claim actually happened
            for (uint256 i = claimStart; i <= claimEnd; i += WEEK) {
                uint256 userBalance = lendingMarketBalances[_market][lender][i];
                uint256 marketBalance = lendingMarketTotalBalance[_market][i];
                RewardInformation memory ri = rewardInformation[i];
                require(ri.set, "Reward not set yet");
                uint256 marketWeight = gaugeController.gauge_relative_weight_write(_market, i);
                cantoToSend += (marketWeight * userBalance * ri.amount) / (1e18 * marketBalance);
            }
            userClaimedEpoch[_market][lender] = claimEnd + WEEK;
        }
        if (cantoToSend > 0) {
            (bool success, ) = msg.sender.call{value: cantoToSend}("");
            require(success, "Failed to send CANTO");
        }

Tools Used

Manual Review

Recommended Mitigation Steps

if (claimEnd >= claimStart) {
            // This ensures that we only set userClaimedEpoch when a claim actually happened
            for (uint256 i = claimStart; i <= claimEnd; i += WEEK) {
                uint256 userBalance = lendingMarketBalances[_market][lender][i];
                uint256 marketBalance = lendingMarketTotalBalance[_market][i];
                RewardInformation memory ri = rewardInformation[i];
                require(ri.set, "Reward not set yet"); 
                uint256 marketWeight = gaugeController.gauge_relative_weight_write(_market, i);
+               if(marketBalance != 0){
                cantoToSend += (marketWeight * userBalance * ri.amount) / (1e18 * marketBalance);
+               } 
            }
            userClaimedEpoch[_market][lender] = claimEnd + WEEK;
        }
        if (cantoToSend > 0) {
            (bool success, ) = msg.sender.call{value: cantoToSend}("");
            require(success, "Failed to send CANTO");
        }

Assessed type

Math