code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

restakeToken is used to restaking an NFT, which can be invoked by anyone when the current period expires according to code comments. But a problem with the code implementation caused only the owner can invoke restakeToken even after the period expired, which the team admitted was an implementation error.

Proof of Concept

    function restakeToken(uint256 tokenId) external {
        IncentiveKey storage incentiveId = stakedIncentiveKey[tokenId];
        if (incentiveId.startTime != 0) _unstakeToken(incentiveId, tokenId, true);

        (IUniswapV3Pool pool, int24 tickLower, int24 tickUpper, uint128 liquidity) =
            NFTPositionInfo.getPositionInfo(factory, nonfungiblePositionManager, tokenId);

        _stakeToken(tokenId, pool, tickLower, tickUpper, liquidity);
    }

    function _unstakeToken(IncentiveKey memory key, uint256 tokenId, bool isNotRestake) private {
        Deposit storage deposit = deposits[tokenId];

        (uint96 endTime, uint256 stakedDuration) =
            IncentiveTime.getEndAndDuration(key.startTime, deposit.stakedTimestamp, block.timestamp);

        address owner = deposit.owner;

        // 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();

When restakeToken invoke _unstakeToken, the parameter isNotRestake should be set to false, but in the code is true, which would prevent other users from calling restakeToken.

Tools Used

Manual review

Recommended Mitigation Steps

Set isNotRestake to false

Assessed type

Context

c4-judge commented 1 year ago

trust1995 marked the issue as duplicate of #745