code-423n4 / 2023-08-verwa-findings

8 stars 7 forks source link

the `claim` function may underFlow when it calculate the `claimEnd ` #441

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/498a3004d577c8c5d0c71bff99ea3a7907b5ec23/src/LendingLedger.sol#L164

Vulnerability details

Impact

in the claim function there is possibility of the underflow which lead the transaction to revert, the function may underflow in this line uint256 claimEnd = Math.min(currEpoch - WEEK, _claimUpToTimestamp) because the currEpoch will return the current epoch time rounded down to week which mean it return thursday in seconds(current epoch always returns thursday because epoch is set as weeks) and if the WEEK was bigger than the currEpoch (if it was friday) then this line will underflow and return negative value. even if the currEpoch was sunday or monday or any day in second it will revert if it wasn't friday in term of seconds.

Proof of Concept

the claim function:

function claim(
        address _market,
        uint256 _claimFromTimestamp,
        uint256 _claimUpToTimestamp
    )
        external
        is_valid_epoch(_claimFromTimestamp)
        is_valid_epoch(_claimUpToTimestamp)
    {
        address lender = msg.sender;
        uint256 userLastClaimed = userClaimedEpoch[_market][lender];
        require(userLastClaimed > 0, "No deposits for this user");
        _checkpoint_lender(_market, lender, _claimUpToTimestamp);
        _checkpoint_market(_market, _claimUpToTimestamp);
        uint256 currEpoch = (block.timestamp / WEEK) * WEEK;
        uint256 claimStart = Math.max(userLastClaimed, _claimFromTimestamp);
        //@audit this may underflow
        uint256 claimEnd = Math.min(currEpoch - WEEK, _claimUpToTimestamp);
        uint256 cantoToSend;
        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"); // Can only claim for epochs where rewards are set, even if it is set to 0
                uint256 marketWeight = gaugeController
                    .gauge_relative_weight_write(_market, i); // Normalized to 1e18
                cantoToSend +=
                    (marketWeight * userBalance * ri.amount) /
                    (1e18 * marketBalance); // (marketWeight / 1e18) * (userBalance / marketBalance) * ri.amount;
            }
            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

recommend to change the line mentioned to this to avoid underflow and returning of negative value: uint256 claimEnd = Math.min(WEEK - currEpoch, _claimUpToTimestamp);

Assessed type

Under/Overflow

c4-pre-sort commented 1 year ago

141345 marked the issue as low quality report

141345 commented 1 year ago

invalid

uint256 currEpoch = (block.timestamp / WEEK) * WEEK;
c4-judge commented 1 year ago

alcueca marked the issue as unsatisfactory: Invalid