lastResortTimelockOwnerClaimNFT() can retrieve the token although drawing is in progress

[code423n4]

Lines of code

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

Vulnerability details

Impact

owner can retrieve the token although drawing is in progress

Proof of Concept

lastResortTimelockOwnerClaimNFT() Be used in:
"
If no users ultimately claim the NFT, the admin specifies a timelock period after which they can retrieve the raffled NFT.
"

But it is no check that if draw is already in progress, i.e. the current round of request.drawTimelock >= block.timestamp
it should not be able to be lastResortTimelockOwnerClaimNFT().
Because a new round of draw has already started and the results may be available.

The above problems can occur in the following two cases：
1:
Because:
_settings.drawBufferTime >= HOUR_IN_SECONDS and <= MONTH_IN_SECONDS
_settings.recoverTimelock > = WEEK_IN_SECONDS and <= MONTH_IN_SECONDS * 12

if drawBufferTime = two weeks and recoverTimelock = one weeks

So: this condition is possible: drawBufferTime > recoverTimelock

2:
The first few rounds have been unfulfilled and keep repeating to start a new round, resulting in request.drawTimelock > settings.recoverTimelock will appear

Tools Used

Recommended Mitigation Steps

add check drawTimelock

    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 (request.drawTimelock >= block.timestamp) {
+         revert STILL_IN_DRAWING();
+       }
        ...

[c4-judge]

gzeon-c4 marked the issue as duplicate of #146

[c4-judge]

gzeon-c4 marked the issue as satisfactory

[c4-judge]

gzeon-c4 changed the severity to 3 (High Risk)