spengrah
commented
1 year ago good standing is a related but separate concept from eligibility. The standing datapoint is a primitive that exists to facilitate other contracts to apply penalties or other accountability measures to hat wearers determined to be in "bad standing" (however that is defined by the DAO / working group / community / etc).
With this in mind, we only want isInGoodStanding() to return false if in fact the wearer was deemed to be in bad standing. Otherwise, we might trigger penalties on wearers who simply no longer are wearing a hat, eg because they moved on to a different role, they renounced the hat, etc..
ksk2345
high
Hats.sol, function isInGoodStanding may return success for a renounced Hat also
Summary
In Hats.sol, the view function isInGoodStanding checks whether a wearer of a Hat is in good standing. However there is no check if the wearer is currently wearing the hat.
Step1: An address is assigned/minted a Hat, say wearer-x assigned to hatid-x Step2: A call to function isInGoodStanding(wearer-x, hatid-x) gives success Step3: wearer-x renounces the hatid-x, so this address is not wearing the hatid-x any more Step4: A call to function isInGoodStanding(wearer-x, hatid-x) still gives success
Vulnerability Detail
If an upstream contract or application uses this function isInGoodStanding, and takes decision to give permissions, etc., then its possible for an address previously in good standing to misuse. Hence marking this issue as High.
Impact
An address earlier having a good standing, but currently not wearer of the hat will be authorized by the Hats protocol
Code Snippet
https://github.com/Hats-Protocol/hats-protocol/blob/fafcfdf046c0369c1f9e077eacd94a328f9d7af0/src/Hats.sol#L971-L998
Tool used
Manual Review
Recommendation
Check if the wearer is currently having balance > 0, by adding this function isWearerOfHat(address _user, uint256 _hatId) inside isInGoodStanding, otherwise return failure.