Forgeries contest - wagmi's results

Lines of code

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

Vulnerability details

Impact

Constant MONTH_IN_SECONDS has incorrect value. Instead of 1 month, it has the value of 7 months.

// @dev about 30 days in a month
uint256 immutable MONTH_IN_SECONDS = (3600 * 24 * 7) * 30; 
// @audit wrong value, could allow bufferTime and recoverTimelock become too long

This constant is used to check some settings in function initialize()

if (_settings.drawBufferTime < HOUR_IN_SECONDS) {
    revert REDRAW_TIMELOCK_NEEDS_TO_BE_MORE_THAN_AN_HOUR();
}
if (_settings.drawBufferTime > MONTH_IN_SECONDS) {
    revert REDRAW_TIMELOCK_NEEDS_TO_BE_LESS_THAN_A_MONTH();
}

if (_settings.recoverTimelock < block.timestamp + WEEK_IN_SECONDS) {
    revert RECOVER_TIMELOCK_NEEDS_TO_BE_AT_LEAST_A_WEEK();
}
if (
    _settings.recoverTimelock >
    block.timestamp + (MONTH_IN_SECONDS * 12)
) {
    revert RECOVER_TIMELOCK_NEEDS_TO_BE_LESS_THAN_A_YEAR();
}

As we can see, the last check make sure recoverTimelock cannot be longer than 1 year, but because MONTH_IN_SECONDS, value of recoverTimelock could be mistakenly set to 7 years.

Proof of Concept

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

/// @dev 60 seconds in a min, 60 mins in an hour
uint256 immutable HOUR_IN_SECONDS = 60 * 60;
/// @dev 24 hours in a day 7 days in a week
uint256 immutable WEEK_IN_SECONDS = (3600 * 24 * 7);
// @dev about 30 days in a month
uint256 immutable MONTH_IN_SECONDS = (3600 * 24 * 7) * 30;

Value of MONTH_IN_SECONDS should be 3600 * 24 * 30

Tools Used

Manual Review

Recommended Mitigation Steps

Correcting the value of MONTH_IN_SECONDS to (3600 * 24 * 30)