initialize() checks if admin owns NFT but startDraw() doesn't.

[code423n4]

Lines of code

https://github.com/code-423n4/2022-12-forgeries/blob/fc271cf20c05ce857d967728edfb368c58881d85/src/VRFNFTRandomDraw.sol#L125-L136
https://github.com/code-423n4/2022-12-forgeries/blob/fc271cf20c05ce857d967728edfb368c58881d85/src/VRFNFTRandomDraw.sol#L172-L198

Vulnerability details

Impact

admin can sell, transfer or have their tokenId NFT stolen via phishing attack to another address after initialising the VRFNFTRandomDraw contract.
This would make it impossible to startDraw because admin is no longer in possession of tokenId.

An admin with the intention of never really going through with the raffle could maliciously initialize a raffle just for PR purposes and then back out.

This makes the contract far from trustless.

Proof of Concept

initialize() checks if admin owns NFT but startDraw() doesn't.

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

        // Get owner of raffled tokenId and ensure the current owner is the admin
        try
            IERC721EnumerableUpgradeable(_settings.token).ownerOf(
                _settings.tokenId
            )
        returns (address nftOwner) {
            // Check if address is the admin address
            if (nftOwner != admin) {
                revert DOES_NOT_OWN_NFT();
            }
        } catch {
            revert TOKEN_BEING_OFFERED_NEEDS_TO_EXIST();

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

    function startDraw() external onlyOwner returns (uint256) {
        // Only can be called on first drawing
        if (request.currentChainlinkRequestId != 0) {
            revert REQUEST_IN_FLIGHT();
        }

        // Emit setup draw user event
        emit SetupDraw(msg.sender, settings);

        // Request initial roll
        _requestRoll();

        // Attempt to transfer token into this address
        try
            IERC721EnumerableUpgradeable(settings.token).transferFrom(
                msg.sender,
                address(this),
                settings.tokenId
            )
        {} catch {
            revert TOKEN_NEEDS_TO_BE_APPROVED_TO_CONTRACT();
        }

        // Return the current chainlink request id
        return request.currentChainlinkRequestId;
    }

For the reasons above and others mentioned in separate findings, I believe the disjointed execution of initialize() and startDraw()
allows for admin to manipulate incentives for users to join a raffle and not follow through with his promise.

Recommended Mitigation Steps

Make startDraw() internal and call it within initialize().

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory:
Invalid

[trust1995]

@gzeon-c4
Seems to be dup of #104 . Same exact attack path as example 1 of 104.

[c4-judge]

gzeon-c4 marked the issue as duplicate of #104

[c4-judge]

gzeon-c4 marked the issue as satisfactory