roguereddwarf - Hats.balanceOfBatch returns wrong result

[sherlock-admin]

roguereddwarf

medium

Hats.balanceOfBatch returns wrong result

Summary

The Hats contract inherits from ERC1155.

So it also has the public balanceOfBatch function that is exposed by the ÈRC1155 contract.

This however can return wrong results since it does not check that the wearer is eligible to wear the hat and if the hat is active.

Vulnerability Detail

This is the balanceOfBatch function:

    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]];
            }
        }
    }

As you can see it directly returns the values from the _balanceOf mapping.

The Hats.balanceOf function however first checks _isActive and _isEligible and only then queries the _balanceOf mapping:

    function balanceOf(address _wearer, uint256 _hatId)
        public
        view
        override(ERC1155, IHats)
        returns (uint256 balance)
    {
        Hat storage hat = _hats[_hatId];


        balance = 0;


        if (_isActive(hat, _hatId) && _isEligible(_wearer, hat, _hatId)) {
            balance = super.balanceOf(_wearer, _hatId);
        }
    }

Impact

Contracts that integrate with Hats and use the balanceOfBatch function get wrong results and misbehave.

Code Snippet

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

https://github.com/Hats-Protocol/hats-protocol/blob/fafcfdf046c0369c1f9e077eacd94a328f9d7af0/src/Hats.sol#L1149-L1162

Tool used

Manual Review

Recommendation

The Hats contract should override the balanceOfBatch function and either revert when it is called (effectively disabling the function) or the function should include the _isActive(hat, _hatId) && _isEligible(_wearer, hat, _hatId) check.

Duplicate of #85