Forgeries contest - sces60107's results

Lines of code

https://github.com/code-423n4/2022-12-forgeries/blob/main/src/VRFNFTRandomDraw.sol#L306

Vulnerability details

Impact

Owners can call lastResortTimelockOwnerClaimNFT to reclaim NFT back if settings.recoverTimelock <= block.timestamp. But settings.recoverTimelock is set in initialize, Owner can delay the call to startDraw until settings.recoverTimelock.

If the owner wants to refuse to give NFT to some specific winners, the owner can call lastResortTimelockOwnerClaimNFT if some specific winners win the draw. Because the owner delayed the call to startDraw, the owner can call (or front-run winnerClaimNFT) lastResortTimelockOwnerClaimNFT in no time.

Proof of Concept

The lastResortTimelockOwnerClaimNFT function checks settings.recoverTimelock:

    function lastResortTimelockOwnerClaimNFT() external onlyOwner {
        // If recoverTimelock is not setup, or if not yet occurred
        if (settings.recoverTimelock > block.timestamp) {
            // Stop the withdraw
            revert RECOVERY_IS_NOT_YET_POSSIBLE();
        }
        ...

If an owner delays the startDraw until settings.recoverTimelock, the owner can call lastResortTimelockOwnerClaimNFT in no time. The owner can reject some specific winners by frontrunning winnerClaimNFT() which is called by the winner.

Tools Used

Manual Review

Recommended Mitigation Steps

Add recoverBufferTime variable, and reset settings.recoverTimelock in startDraw:

    function startDraw() external onlyOwner returns (uint256) {
+       settings.recoverTimelock = block.timestamp + settings.recoverBufferTime;
        ...