ERC404 icon indicating copy to clipboard operation
ERC404 copied to clipboard

Fix whitelist minting NFT bug.

Open Raylin51 opened this issue 1 year ago • 5 comments

In certain circumstances, exploiting two different states of transferFrom can lead to a bug that allows minting ERC-721 tokens out of thin air through a whitelist. Specifically, when a regular EOA (Externally Owned Account) transfers an ERC-721 token to a Whitelist EOA, and then the Whitelist transfers one ERC-20 token back to the regular EOA, due to the _transfer method skipping the burn for whitelisted addresses, two ERC-721 tokens are created. To prevent this method from minting tokens, transferFrom strictly prohibits transferring the tokenId to a whitelisted address. Moreover, to prevent circumventing this restriction by toggling the whitelist status, setting a whitelist address to true will forcibly clear any ERC-721 tokens on the whitelist address.

Raylin51 avatar Feb 08 '24 15:02 Raylin51

I think that won't happen cuz there is NFT ownership check here. It will be reverted in those cases. https://github.com/0xacme/ERC404/blame/main/src/ERC404.sol#L362-L364

if (_ownerOf[id] != address(0)) {
    revert AlreadyExists();
}

codeninja819 avatar Feb 09 '24 08:02 codeninja819

I think that won't happen cuz there is NFT ownership check here. It will be reverted in those cases. https://github.com/0xacme/ERC404/blame/main/src/ERC404.sol#L362-L364

if (_ownerOf[id] != address(0)) {
    revert AlreadyExists();
}

This revert indicates that minting duplicate IDs is prohibited. The minting process only cares whether the ID to be minted is already taken, without considering the possibility of over-issuance due to errors in the preceding logic. I'm not discussing the logic errors of _mint method, but rather a bug caused by the inconsistent behavior of whitelist accounts under different transfer methods. For example, in the case of _transfer, if a token is transferred to a whitelist account, it will not mint an NFT for the whitelist. However, if an ERC-721 is transferred to a whitelist account, it will also transfer the token to the whitelist, resulting in a discrepancy. https://github.com/0xacme/ERC404/blame/14c1362ca6cd3b0d0a65febc0f9cb1418b4b56e6/src/ERC404.sol#L235

unchecked {
  balanceOf[to] += _getUnit();
}

Raylin51 avatar Feb 09 '24 18:02 Raylin51

I don't think it's a logic error. It's the way it works. NFT balances of whitelisted addresses can be incorrect, but Uniswap protocol will work because their ERC20 balance is correct. And there is no need for Uniswap contracts to hold NFTs.

codeninja819 avatar Feb 09 '24 20:02 codeninja819

This issue implies that the owner could surreptitiously mint additional ERC-721 tokens. Suppose the owner sets their own EOA wallet as a whitelist and exploits this to infinitely mint ERC-721 tokens, potentially flooding the NFT market. If you don't consider this a logical error, then perhaps calling it a backdoor would be more appropriate.

Raylin51 avatar Feb 09 '24 20:02 Raylin51

Yes, that can be possible. Now I understand what you mean.

codeninja819 avatar Feb 09 '24 20:02 codeninja819