code-423n4 / 2024-02-uniswap-foundation-findings

2 stars 3 forks source link

High gas prices can halt staking and fee claims, disrupting rewards and governance. #246

Closed c4-bot-4 closed 7 months ago

c4-bot-4 commented 7 months ago

Lines of code

https://github.com/code-423n4/2024-02-uniswap-foundation/blob/5298812a129f942555466ebaa6ea9a2af4be0ccc/src/UniStaker.sol#L256-L261 https://github.com/code-423n4/2024-02-uniswap-foundation/blob/5298812a129f942555466ebaa6ea9a2af4be0ccc/src/UniStaker.sol#L499-L503 https://github.com/code-423n4/2024-02-uniswap-foundation/blob/5298812a129f942555466ebaa6ea9a2af4be0ccc/src/V3FactoryOwner.sol#L181-L198

Vulnerability details

Description

Several UniStaker operations rely on users submitting transactions to the contract. This includes stakers calling stake and withdraw, and external actors calling claimFees to trigger reward distribution. If gas prices spiked, these activities could halt.

These operations are at risk stake(...) external {...}, withdraw(...) external {...}, and claimFees(...) external {...}

  function stake(uint256 _amount, address _delegatee)
    external
    returns (DepositIdentifier _depositId)
  {
    _depositId = _stake(msg.sender, _amount, _delegatee, msg.sender);
  }

  function withdraw(DepositIdentifier _depositId, uint256 _amount) external {
    Deposit storage deposit = deposits[_depositId];
    _revertIfNotDepositOwner(deposit, msg.sender);
    _withdraw(deposit, _depositId, _amount);
  }

  function claimFees(
    IUniswapV3PoolOwnerActions _pool,
    address _recipient,
    uint128 _amount0Requested,
    uint128 _amount1Requested
  ) external returns (uint128, uint128) {
    PAYOUT_TOKEN.safeTransferFrom(msg.sender, address(REWARD_RECEIVER), payoutAmount);
    REWARD_RECEIVER.notifyRewardAmount(payoutAmount);
    (uint128 _amount0, uint128 _amount1) =
      _pool.collectProtocol(_recipient, _amount0Requested, _amount1Requested);

    // Protect the caller from receiving less than requested. See `collectProtocol` for context.
    if (_amount0 < _amount0Requested || _amount1 < _amount1Requested) {
      revert V3FactoryOwner__InsufficientFeesCollected();
    }
    emit FeesClaimed(address(_pool), msg.sender, _recipient, _amount0, _amount1);
    return (_amount0, _amount1);
  }

If gas fees for these exceed expected thresholds, usage may stop entirely.

Impact:

With stalkees and fee claimers disincentivized by high gas, it would have follow-on effects:

Probability:

Given historical spikes in Ethereum gas costs, periods of 1000+ gwei are likely over timeframes of years. Lack of protection makes the issues above quite likely long-term.

Proof of Concept

If gas prices rise too high, users cannot afford to call stake, withdraw, or claimFees. This halts the core reward distribution process.

Without stakers delegating and fee claimers enabling rewards, the system breaks:

Exploit Conditions:

The problem surfaces when gas exceeds expected thresholds, like:

Given historical spikes, this is likely over the +10 year lifecycle.

Root Cause:

No checks exist on gas price in relation to usage costs. Users are never priced out of core operations. This will eventually occur.

By adding gas cost caps, the issue can be addressed before rollout.

Tools Used

VS

Recommended Mitigation Steps

Provide users with upfront gas cost estimations before they submit a transaction. If potential costs exceed a sensible limit, issue a prominent warning with options to:

Assessed type

Governance

MarioPoneder commented 7 months ago

External systemic risk, better discussed in Analysis report.

c4-judge commented 7 months ago

MarioPoneder marked the issue as unsatisfactory: Invalid