code-423n4 / 2022-12-forgeries-findings

0 stars 0 forks source link

Anyone should be able to call `redraw()` #103

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-forgeries/blob/fc271cf20c05ce857d967728edfb368c58881d85/src/VRFNFTRandomDraw.sol#L200-L226

Vulnerability details

Impact

Anyone should be able to call redraw() in case admin key gets compromised and winner doesn't claim the NFT, otherwise valuable NFT potentially worth millions could be stuck forever.

Proof of Concept

onlyOwner modifier unnecessarily prevents anyone else involved in the raffle to call the function after drawTimelock has passed.

https://github.com/code-423n4/2022-12-forgeries/blob/fc271cf20c05ce857d967728edfb368c58881d85/src/VRFNFTRandomDraw.sol#L200-L226

    /// @notice Call this to re-draw the raffle
    /// @return chainlink request ID
    /// @dev Only callable by the owner
    function redraw() external onlyOwner returns (uint256) {
        if (request.drawTimelock >= block.timestamp) {
            revert TOO_SOON_TO_REDRAW();
        }
        // Reset request
        delete request;

        // Re-roll
        _requestRoll();

        // Owner of token to raffle needs to be this contract
        if (
            IERC721EnumerableUpgradeable(settings.token).ownerOf(
                settings.tokenId
            ) != address(this)
        ) {
            revert DOES_NOT_OWN_NFT();
        }

        // Return current chainlink request ID
        return request.currentChainlinkRequestId;
    }

Recommended Mitigation Steps

Allow other users to call redraw().

c4-judge commented 1 year ago

gzeon-c4 marked the issue as duplicate of #195

c4-judge commented 1 year ago

Duplicate of https://github.com/code-423n4/2022-12-forgeries-findings/issues/106

c4-judge commented 1 year ago

gzeon-c4 marked the issue as grade-b