Lines of code
<https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/uni-v3-staker/UniswapV3Staker.sol#L438-L451&gt;
<https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/uni-v3-staker/UniswapV3Staker.sol#L429-L451&gt;

Vulnerability details

Impact

In the UniswapV3Staker.sol contract, a user can drain the incentives by repeatedly staking and unstaking.

Proof of Concept

During staking, the _stakeToken(…) function checks that incentives is not zero (this would later become insufficient) but does not in itself create incentive

        if (incentives[incentiveId].totalRewardUnclaimed == 0) revert NonExistentIncentiveError();

When the user tries to restake by calling the _unstakeToken(…) through the restakeToken(…) function,

    function restakeToken(uint256 tokenId) external {
        ...
        if (incentiveId.startTime != 0) _unstakeToken(incentiveId, tokenId, true);
        ...
        _stakeToken(tokenId, pool, tickLower, tickUpper, liquidity);
    }

the incentives[incentiveId].totalRewardUnclaimed is decreased by the reward in an unchecked block. When _stakeToken(…) is called to complete restakeToken(…) function, it checks again to ensure that incentives[incentiveId].totalRewardUnclaimed is not zero as shown above.

        unchecked {
            ...
            // reward is never greater than total reward unclaimed
            incentive.totalRewardUnclaimed -= reward;
            ...
            tokenIdRewards[tokenId] += reward;
        }

The malicious user repeats the cycle until the incentives[incentiveId].totalRewardUnclaimed becomes small but is not zero since it is continuously reduced by the reward and this is done within an unchecked block. At this point, the developer’s assumption that reward is never greater than total reward unclaimed becomes dangerous because he does not add a check to ensure his assumption holds true since it is not guarantee that the reward will decreasing for every epoch.

At some point during the user’s staking and unstaking iteration (via the restakeToken(…) function), the incentives[incentiveId].totalRewardUnclaimed will overflow since the accounting is done in an unchecked block leading to an inflated totalRewardUnclaimed.

For the next restake(…) done by the malicious user, the reward will be calculated using the inflated incentive.totalRewardUnclaimed value leading to the user accruing a larger than normal reward.

    function _unstakeToken(IncentiveKey memory key, uint256 tokenId, bool isNotRestake) private {
        ...
        uint256 reward = RewardMath.computeBoostedRewardAmount(
            incentive.totalRewardUnclaimed,
            incentive.totalSecondsClaimedX128,
            key.startTime,
            endTime,
            secondsInsideX128,
            block.timestamp
        );


        unchecked {
            ...
            incentive.totalRewardUnclaimed -= reward;
            ...
            rewards[owner] += reward;
        }
    }

At this point the user can unstake the position with a higher total reward that expected or continue the exploit unnoticed.

This will happen slowly but surely and a malicious user does not need to

Tools Used

Manual review.

Recommended Mitigation Steps

Add a check to enforce the developers assumption that the reward is never greater than total reward unclaimed as shown below to prevent the overflow from happening in the first place.

Modify the _unstakeToken(…) function to ensure the user cannot keep restaking when the incentive.totalRewardUnclaimed is small enough to cause an overflow

   require(reward < incentive.totalRewardUnclaimed, "...");

as shown below

    function _unstakeToken(IncentiveKey memory key, uint256 tokenId, bool isNotRestake) private {
        ...
        uint256 reward = RewardMath.computeBoostedRewardAmount(
            incentive.totalRewardUnclaimed,
            incentive.totalSecondsClaimedX128,
            key.startTime,
            endTime,
            secondsInsideX128,
            block.timestamp
        );

        require(reward < incentive.totalRewardUnclaimed, "..."); // add this line


        unchecked {
            ...
            incentive.totalRewardUnclaimed -= reward;
            ...
            rewards[owner] += reward;
        }
    }

It is also not out of place to call createIncentive(…) during the staking phase if the reward is greater than the incentive.totalRewardUnclaimed

Assessed type

Other

---

The text was updated successfully, but these errors were encountered:

All reactions