Minting would be inaccessible in public sales

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/MinterContract.sol#L220

Vulnerability details

Impact

The comments in MinterContract suggest that for public sales allowListStartTime and allowListEndTime should be equal to publicStartTime and publicEndTime respectively. However, this would DoS all minting functionality.

Proof of Concept

When MinterContract.mint() function is called, it first checks if timestamp is between allowlist time and enters the corresponding code block.

        if (block.timestamp >= collectionPhases[col].allowlistStartTime && block.timestamp <= collectionPhases[col].allowlistEndTime) {
            phase = 1;
            bytes32 node;
            if (_delegator != 0x0000000000000000000000000000000000000000) {
                bool isAllowedToMint;
                isAllowedToMint = dmc.retrieveGlobalStatusOfDelegation(_delegator, 0x8888888888888888888888888888888888888888, msg.sender, 1) || dmc.retrieveGlobalStatusOfDelegation(_delegator, 0x8888888888888888888888888888888888888888, msg.sender, 2);
                if (isAllowedToMint == false) {
                isAllowedToMint = dmc.retrieveGlobalStatusOfDelegation(_delegator, collectionPhases[col].delAddress, msg.sender, 1) || dmc.retrieveGlobalStatusOfDelegation(_delegator, collectionPhases[col].delAddress, msg.sender, 2);    
                }
                require(isAllowedToMint == true, "No delegation");
                node = keccak256(abi.encodePacked(_delegator, _maxAllowance, tokData));
                require(_maxAllowance >= gencore.retrieveTokensMintedALPerAddress(col, _delegator) + _numberOfTokens, "AL limit");
                mintingAddress = _delegator;
            } else {
                node = keccak256(abi.encodePacked(msg.sender, _maxAllowance, tokData));
                require(_maxAllowance >= gencore.retrieveTokensMintedALPerAddress(col, msg.sender) + _numberOfTokens, "AL limit");
                mintingAddress = msg.sender;
            }
            require(MerkleProof.verifyCalldata(merkleProof, collectionPhases[col].merkleRoot, node), 'invalid proof');

However, if there is only public sale for NFT, the mint would revert on verifying the merkle proof here since the collectionPhases[col].merkleRoot would be equal to zero but the hash of proof and node wouldn't (since node is not user-provided, but the hash of tokData and msg.sender):

require(MerkleProof.verifyCalldata(merkleProof, collectionPhases[col].merkleRoot, node), 'invalid proof');

Tools Used

Manual review

Recommended Mitigation Steps

Check if allowListStartTime == publicStartTime. If that is the case, skip the allow list block and enter the block for public mint.

Assessed type

DoS

[c4-pre-sort]

141345 marked the issue as primary issue

[c4-sponsor]

a2rocket (sponsor) disputed

[a2rocket]

the comment refers to only if we have public phase and not allowlist phase. Here is the comment: // for public sale set the allowlist the same time as publicsale

We do not mention that the publicEndTime should be the same as allowlistEndTime which is totally wrong.

[c4-pre-sort]

141345 marked the issue as sufficient quality report

[alex-ppg]

The Warden specifies that the comments hint at the allowlist period being the same as the public period which would result in an inoperable sale.

As the Sponsor specifies, this is not implied by the comment, and further documentation in the code clarifies that the only the start times should be equal. As such, I consider this submission invalid.

[c4-judge]

alex-ppg marked the issue as unsatisfactory:
Invalid