veRWA - 0xmuxyz's results

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/main/src/LendingLedger.sol#L173 https://github.com/code-423n4/2023-08-verwa/blob/main/src/GaugeController.sol#L181 https://github.com/code-423n4/2023-08-verwa/blob/main/src/GaugeController.sol#L69

Vulnerability details

Impact

Within the GaugeController#_get_sum(), 500, which is the maximum number of the sum (loop) used in the for-loop, would be too many. This may lead to that the transaction would be reverted in the for-loop of the GaugeController#_get_sum() due to reaching the gas limit.

As a result, when a user call the LendingLedger#claim(), the transaction of the function would also be reverted. Because the GaugeController#_get_sum(), which is called for calculating the marketWeight in the LendingLedger#claim(), would be reverted.

Proof of Concept

Within the the LendingLedger#claim(), the GaugeController#gauge_relative_weight_write() would be called to calculate the marketWeight like this: https://github.com/code-423n4/2023-08-verwa/blob/main/src/LendingLedger.sol#L173

    /// @notice Claim the CANTO for a given market. Can only be performed for prior (i.e. finished) epochs, not the current one
    /// @param _market Address of the market
    /// @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
    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);
        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 /// @audit
                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");
        }
    }

Within the GaugeController#gauge_relative_weight_write(), the GaugeController#_get_sum() would be called like this: https://github.com/code-423n4/2023-08-verwa/blob/main/src/GaugeController.sol#L181

    /// @notice Get gauge weight normalized to 1e18 and also fill all the unfilled
    ///     values for type and gauge records
    /// @dev Any address can call, however nothing is recorded if the values are filled already
    /// @param _gauge Gauge address
    /// @param _time Relative weight at the specified timestamp in the past or present
    /// @return Value of relative weight normalized to 1e18
    function gauge_relative_weight_write(address _gauge, uint256 _time) external returns (uint256) {
        _get_weight(_gauge);
        _get_sum(); /// @audit
        return _gauge_relative_weight(_gauge, _time);
    }

Within the GaugeController#_get_sum(), the sum of the gauge weights would be calculated by using for-loop, which the maximum number of the sum (loop) is 500 like this: https://github.com/code-423n4/2023-08-verwa/blob/main/src/GaugeController.sol#L69

    /// @notice Fill historic gauge weights week-over-week for missed checkins and return the sum for the future week
    /// @return Sum of weights
    function _get_sum() internal returns (uint256) {
        uint256 t = time_sum;
        Point memory pt = points_sum[t];
        for (uint256 i; i < 500; ++i) { /// @audit
            if (t > block.timestamp) break;
            t += WEEK;
            uint256 d_bias = pt.slope * WEEK;
            if (pt.bias > d_bias) {
                pt.bias -= d_bias;
                uint256 d_slope = changes_sum[t];
                pt.slope -= d_slope;
            } else {
                pt.bias = 0;
                pt.slope = 0;
            }
            points_sum[t] = pt;
            if (t > block.timestamp) time_sum = t;
        }
        return pt.bias;
    }

However, within the GaugeController#_get_sum(), 500, which is the maximum number of the sum (loop) used in the for-loop, would be too many. This may lead to that the transaction would be reverted in the for-loop of the GaugeController#_get_sum() due to reaching the gas limit.

As a result, when a user call the LendingLedger#claim(), the transaction of the function would also be reverted. Because the GaugeController#_get_sum(), which is called for calculating the marketWeight in the LendingLedger#claim(), would be reverted.

Tools Used

• Foundry

Recommended Mitigation Steps

Within the GaugeController#_get_sum() (GaugeController.sol#L69), consider reducing the maximum number of the sum (loop) in the for-loop from 500 to smaller number.

Assessed type

DoS