code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/main/src/uni-v3-staker/UniswapV3Staker.sol#L342 https://github.com/code-423n4/2023-05-maia/blob/main/src/uni-v3-staker/UniswapV3Staker.sol#L373-L374

Vulnerability details

Impact

The UniswapV3Staker.restakeToken() function is used to restake the liquidity NFT every week by the users since the Incentives are weekly. The restakeToken function checks whether the tokenId is already staked, if so it will unstake first and then will retrieve the latest position info of the tokenId and will restake again.

The unstake of the tokenId happens as follows:

    if (incentiveId.startTime != 0) _unstakeToken(incentiveId, tokenId, true);

Once the _unstakeToken() function is called, if the msg.sender == owner then it will allow the unstaking of the tokenId given subsequent conditions are also satisfied. But as per the Natspec comments of the function, if the block time has passed the incentive end time (block.timestamp >= endtime) anyone (not necessarily the owner) can call the restake function.

    // anyone can call restakeToken if the block time is after the end time of the incentive

But this is not true. Since inside the UniswapV3Staker.restakeToken() function, when calling the _unstakeToken function the isNotRestake is set to true. Which means irrespective of the block.timestamp is less or more than the endtime the following condition will result in true since it is logical or operation.

if (isNotRestake || block.timestamp < endTime)

As a result even after the incentive end time has passed only the owner of the tokenId will be able to call the UniswapV3Staker.restakeToken() function, contrary to what the protocol needs to achieve as mentioned in its comments.

As a result if the owner forgets to restake his NFT after one week he will miss out on staking rewards since nobody else can restake on his behalf.

Proof of Concept

        if (incentiveId.startTime != 0) _unstakeToken(incentiveId, tokenId, true);

https://github.com/code-423n4/2023-05-maia/blob/main/src/uni-v3-staker/UniswapV3Staker.sol#L342

        // anyone can call restakeToken if the block time is after the end time of the incentive
        if ((isNotRestake || block.timestamp < endTime) && owner != msg.sender) revert NotCalledByOwner();

https://github.com/code-423n4/2023-05-maia/blob/main/src/uni-v3-staker/UniswapV3Staker.sol#L373-L374

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

In order to achieve the intended behaviour of the protocol as far as restaking is concerned, it recommended to make the following change in the restake() function as shown below:

    if (incentiveId.startTime != 0) _unstakeToken(incentiveId, tokenId, false);

Here the isNotRestake is set to false. Now the restaking can be controlled by the block.timestamp and incentive endTime when msg.sender != owner. Hence now anyone can call the restake function once the incentive endTime has elapsed.

Assessed type

Other

c4-judge commented 1 year ago

trust1995 marked the issue as duplicate of #745