rvierdiiev - Hat wearer can call function with limited amount of gas in order to make toggle call revert and use previous active status

[sherlock-admin]

rvierdiiev

high

Hat wearer can call function with limited amount of gas in order to make toggle call revert and use previous active status

Summary

Hat wearer can call function with limited amount of gas in order to make toggle call revert and use previous active status

Vulnerability Detail

Each hat has toggle address, which controls if has is active. Once hat is not active, then it's not calculated inside user's balance. In other words, in case if hat is not active, then user any user that has this hat token, doesn't actually wear a hat and his balance is 0.

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

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

This is how _isActive is implemented.
https://github.com/Hats-Protocol/hats-protocol/blob/fafcfdf046c0369c1f9e077eacd94a328f9d7af0/src/Hats.sol#L890-L917

    function _isActive(Hat storage _hat, uint256 _hatId) internal view returns (bool active) {
        (bool success, bytes memory returndata) =
            _hat.toggle.staticcall(abi.encodeWithSignature("getHatStatus(uint256)", _hatId));


        /* 
        * if function call succeeds with data of length == 32, then we know the contract exists 
        * and has the getHatStatus function.
        * But — since function selectors don't include return types — we still can't assume that the return data is a boolean, 
        * so we treat it as a uint so it will always safely decode without throwing.
        */
        if (success && returndata.length == 32) {
            // check the returndata manually
            uint256 uintReturndata = uint256(bytes32(returndata));
            // false condition
            if (uintReturndata == 0) {
                active = false;
                // true condition
            } else if (uintReturndata == 1) {
                active = true;
            }
            // invalid condition
            else {
                active = _getHatStatus(_hat);
            }
        } else {
            active = _getHatStatus(_hat);
        }
    }

As you can see, it tries to get result from toggle address, which should say if hat is still active.
It's possible that toggle is eoa or address 0. In this case it will just call _getHatStatus(_hat) in order to retrieve info from hat.config.
Also toggle can be smart contract. In this case it will return result which then will be returned.

It's also possible for attacker to run this function with limited amount of gas in order to revert toggle call with out of gas error. In that case, function will use _getHatStatus function to get status. For this call to succeed it's needed to have toggle contract with big logic, so function _isActive calls toggle.getHatStatus with 63/64 of gas sent to the _isActive function, which is not enough, so toggle.getHatStatus reverts and remaining 1/64 gas is still enough to finish function that attacker called.

Suppose, that hat is active now and there is toggle contract for that hat. The next call to toggle.getHatStatus will return false and will freeze all hat wearers.
So maybe someone is going to call checkHatStatus, which will reset hat status in config to non active.
But attacker(which is any hat wearer) can frontrun that call.
He can call next function with limited amount of gas(to make toggle call revert) and still be hat wearer.

This gives attacker ability to still call function as he is wearing a hat, because of that he can pass _checkAdmin restriction, so he can make changes like changeHatToggle, changeHatEligibility, mintHat.

Using this trick he can try to call approveLinkTopHatToTree which is using _checkAdminOrWearer function, which calls isWearerOfHat function, which checks user's balance.

After checkHatStatus function will be called, then hat status will be set to false and wearer will not be able to use that trick anymore.

Impact

User can pass toggle checking.

Code Snippet

Provided above

Tool used

Manual Review

Recommendation

Maybe in case of toggle call is reverted, you need to revert too.

[zobront]

Escalate for 10 USDC

This is a valid attack but isn't a dup of #34. They should be separated into two unqiue issues.

[sherlock-admin]

Escalate for 10 USDC

This is a valid attack but isn't a dup of #34. They should be separated into two unqiue issues.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[hrishibhat]

Escalation accepted

Not a duplicate of #34.
Considering this issue a valid medium given that toggle status check can be bypassed in the above scenario

[sherlock-admin]

Escalation accepted

Not a duplicate of #34.
Considering this issue a valid medium given that toggle status check can be bypassed in the above scenario

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.