Lines of code

Vulnerability details

Impact

CidNFT.mint(bytes[]) allow user to mint and add subprotocol NFTs directly after minting. The _addList args to the add call include the _cidNFTID param, which can change if there are other mint before the user’s transaction.

Proof of Concept

An attacker can DOS a user’s mint and add by front-running their mint by another mint, bumping the cid nft id.

<https://github.com/code-423n4/2023-01-canto-identity/blob/dff8e74c54471f5f3b84c217848234d474477d82/src/CidNFT.sol#L143-L157&gt;

    /// @notice Mint a new CID NFT
    /// @dev An address can mint multiple CID NFTs, but it can only set one as associated with it in the AddressRegistry
    /// @param _addList An optional list of encoded parameters for add to add subprotocol NFTs directly after minting.
    /// The parameters should not include the function selector itself, the function select for add is always prepended.
    function mint(bytes[] calldata _addList) external {
        _mint(msg.sender, ++numMinted); // We do not use _safeMint here on purpose. If a contract calls this method, he expects to get an NFT back
        bytes4 addSelector = this.add.selector;
        for (uint256 i = 0; i < _addList.length; ++i) {
            (
                bool success, /*bytes memory result*/

            ) = address(this).delegatecall(abi.encodePacked(addSelector, _addList[i]));
            if (!success) revert AddCallAfterMintingFailed(i);
        }
    }

Recommended Mitigation Steps

Instead of a self delegate call, use a switch statement with abi.decode to support different type of entry, and call the internal function according with the newly generated cid nft id.

The text was updated successfully, but these errors were encountered:

All reactions