Skip to content

Latest commit

 

History

History
59 lines (36 loc) · 2.23 KB

032.md

File metadata and controls

59 lines (36 loc) · 2.23 KB
Raw

Orbiting Magenta Nuthatch

Medium

Malicious actor will block valid threshold configurations for Safe owners through incorrect validation

Summary

The incorrect equality check in threshold validation will cause transaction reverts for valid configurations as the HatsSignerGate contract will reject any threshold that doesn't exactly match the required signatures.

Root Cause

https://github.com/sherlock-audit/2024-11-hats-protocol/blob/main/hats-zodiac/src/HatsSignerGate.sol#L487

In HatsSignerGate.sol:487 the threshold validation uses an incorrect equality check (!=) instead of the proper comparison operator (<):

Internal pre-conditions

  1. Contract needs to be deployed with Safe multisig integration
  2. Safe contract needs to have multiple owners configured
  3. valid signatures need to be set through _getRequiredValidSignatures()

External pre-conditions

None as this is an internal logical issue

Attack Path

  1. Admin attempts to set a threshold higher than the minimum required signatures
  2. Contract calls _getRequiredValidSignatures() to determine minimum required signatures
  3. Contract compares threshold using != operator
  4. Transaction reverts due to threshold not exactly matching required signatures, even though it's higher and therefore valid

Impact

The Safe contract owners cannot configure higher security thresholds than the minimum required. This prevents legitimate use cases where owners want to require more signatures than the minimum for additional security. While this doesn't result in direct financial loss, it reduces the flexibility and security options available to Safe owners. Specific examples:

  • With 5 owners and minimum required signatures of 3, setting a threshold of 4 or 5 would fail
  • Safe owners cannot implement stricter security policies even when all owners agree

PoC

No response

Mitigation

Replace the equality check with a "less than" comparison

// After
if (threshold < _getRequiredValidSignatures(owners.length)) revert ThresholdTooLow();

This change ensures that:

  • Thresholds below the required minimum are rejected
  • Thresholds equal to or higher than the required minimum are accepted
  • The code behavior matches the documented expectations in the comments