sherlock-audit / 2023-02-hats-judging

2 stars 0 forks source link

Ace-30 - linking of hat trees can freeze hat operations because of gas limit #86

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Ace-30

high

linking of hat trees can freeze hat operations because of gas limit

Summary

hats._checkAdmin function has numerous external function calls. When making this call, the gas consumption may exceed the transaction and block gas limit of 30m, resulting in failure. This is particularly true when the tree has a large number of levels (linked) and external contract call (eligibility) consumes a significant amount of gas.

Vulnerability Detail

Most of hats functions (like mint, transfer, change details, approve link) check if the msg.sender is admin by calling _checkAdmin

  1. _checkAdmin calls isWearerOfHat for each level of the tree: // search up _hatId's local address space for an admin hat that the _user wears while (adminLocalHatLevel > 0) { if (isWearerOfHat(_user, getAdminAtLocalLevel(_hatId, adminLocalHatLevel))) { return isAdmin = true; }
  2. isWearerOfHat is calling an eligibility contract that can consume a considerable amount of gas.
  3. So the _checkAdminis calling a gas-intensive function multiple times and when the hat tree has lots of levels, it can fail because of the gas limit of a block.

    Impact

    top-level admins can't do any operation on lower-level hats when the tree grows too big.

    Code Snippet

    https://github.com/more-reese/hats-protocol/blob/2e22c755512eaacb619e38b9e57668529cbb9c34/src/Hats.sol#L779-L797

    Tool used

Manual Review

Recommendation

Optimize gas consumption of _checkAdmin

Duplicate of #96