GimelSec - An inconsistency in the behaviour of balanceOf() and balanceOfBatch().

[sherlock-admin]

GimelSec

medium

An inconsistency in the behaviour of balanceOf() and balanceOfBatch().

Summary

balanceOf() checks _isActive() and _isEligible(), but balanceOfBatch() returns static _balanceOf directly.

Vulnerability Detail

The hats protocol overrides balanceOf() to checks _isActive() and _isEligible(), but it doesn't override balanceOfBatch(). The balanceOfBatch() returns static _balanceOf directly:

    function balanceOfBatch(address[] calldata owners, uint256[] calldata ids)
        public
        view
        virtual
        returns (uint256[] memory balances)
    {
        require(owners.length == ids.length, "LENGTH_MISMATCH");

        balances = new uint256[](owners.length);

        // Unchecked because the only math done is incrementing
        // the array index counter which cannot possibly overflow.
        unchecked {
            for (uint256 i = 0; i < owners.length; ++i) {
                balances[i] = _balanceOf[owners[i]][ids[i]];
            }
        }
    }

Impact

It's inconsistency in the behaviour of balanceOf() and balanceOfBatch().
It would fail if someone creates a new version of singer gates and misuses balanceOfBatch(). Also, some protocols will be broken if a third party protocol integrates the Hats protocol with balanceOfBatch().

Code Snippet

https://github.com/Hats-Protocol/hats-protocol/blob/fafcfdf046c0369c1f9e077eacd94a328f9d7af0/lib/ERC1155/ERC1155.sol#L122-L139

Tool used

Manual Review

Recommendation

Override balanceOfBatch() and check _isActive() and _isEligible().

Duplicate of #85