0x0 - Admin Cannot Batch Mint (1155)

[sherlock-admin]

0x0

medium

Admin Cannot Batch Mint (1155)

Summary

mintByOwnerBatch allows 1155 to be minted from selected accounts, protected by a modifier. The docstring states that accounts with roles MINT_ROLE or ADMIN_ROLE may call this function. In the implementation this is not working for accounts with ADMIN_ROLE.

Vulnerability Detail

ERC1155NFTProduct.mintByOwnerBatch

The modifier on this function allows accounts holding specific roles to perform the mint. Accounts holding ADMIN_ROLE may not call this without additionally being granted the MINT_ROLE.

Impact

• Accounts holding ADMIN_ROLE will have to pay additional gas to modify the RBAC role to include the admin account

Code Snippet

function mintByOwnerBatch(
    address[] memory to,
    uint256[] memory ids,
    uint256[] memory amounts,
    string[] memory uris
) public onlyRole(MINT_ROLE) {
    for (uint256 i = 0; i < ids.length; i++) {
        require(!_exists(ids[i]), "One of tokens is already minted");
        require(
            to[i] == address(to[i]),
            "NFT: one of addresses is invalid"
        );
        require(amounts[i] > 0, "NFT: all amounts must be > 0");
        tokenSupply[ids[i]] += amounts[i];
        if (bytes(uris[i]).length > 0) {
            _tokenURIs[ids[i]] = uris[i];
            emit URI(uris[i], ids[i]);
        }
        _mint(to[i], ids[i], amounts[i], "");
    }
}

Tool used

Manual Review

Recommendation

• Grant the role in the constructor at deploy time or update the modifier to validate if msg.sender holds roles (MINT_ROLE || ADMIN_ROLE)

[hyperspacebunny]

This is invalid, ADMIN_ROLE inherits all other roles via the overridden hasRole function in GranularRoles.sol. This is also covered by our test suite.

// Admin role has all access granted by default
    function hasRole(bytes32 role, address account)
        public
        view
        virtual
        override
        returns (bool)
    {
        return
            super.hasRole(ADMIN_ROLE, account) || super.hasRole(role, account);
    }