code423n4 commented 1 year ago

Lines of code

Vulnerability details

If the contract accepts invalid signatures for minting, this will lead to a potential drainage of rewards

We acknowledge that a non-zero validation is a known-issue. However, while the following has in fact to do with it, the consequences might not be known and are not mentioned in the known-issues.

Consider the following scenario:

1) An admin changes the claimSignerAddress to 0x0 in order to prevent any further mintings or for any other reasons.

2) While the claimSignerAddress now is 0x0, all signatures will pass due to the following line:

if (recoverSigner(hash_, signature_) != claimSignerAddress) revert AddressNotSigned();

The problem here lies in the recover function, which returns 0x0 if the signature is not signed.

3) Any address can mint any desired NFT with any signature.

IIRC this was even an issue why a famous bridge-hack happened, but im unsure which one it was right now.

VSCode

Consider implementing the following additional check within the mintReceipt function:

if (recoverSigner(hash_, signature_) == address(0) revert AddressNotSigned();

Moreover, the claimSignerAddress should obviously have a non-zero check in the setter but this is a known-issue.

c4-judge commented 1 year ago

kirk-baird marked the issue as unsatisfactory: Invalid

kirk-baird commented 1 year ago

This is not true, ECDSA.recover() will check to ensure the zero address is not returned by ecrecover()