Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Taiger4526 - _countValidSignatures Allows Duplicate Signatures, Leading to Incorrect Signature Count #54

Open
sherlock-admin2 opened this issue Nov 23, 2024 · 0 comments

Comments

@sherlock-admin2
Copy link

sherlock-admin2 commented Nov 23, 2024

Taiger4526

High

_countValidSignatures Allows Duplicate Signatures, Leading to Incorrect Signature Count

Summary

The checkTransaction method is used in the HatsSignerGate contract to check transactions. The checkTransaction method calls the _countValidSignatures method to verify the number of signatures, but the _countValidSignatures method does not check for duplicate signatures in the signatures array. An attacker can provide multiple identical signatures to forge the number of signatures to meet the requirement and bypass the signature verification logic.

Root Cause

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

The HatsSignerGate contract uses the checkTransaction method to check transactions. The checkTransaction method calls the _countValidSignatures method to verify the number of signatures. In the _countValidSignatures method, the signatures in the passed signatures array are not checked for duplication. After parsing the signatures one by one, the method only verifies the legitimacy of the signer through isValidSigner(currentOwner), does not record the verified signer, and allows duplicate counting.

Internal pre-conditions

The attacker constructs a signatures array containing multiple identical signatures.
Each duplicate signature in signatures must pass the isValidSigner(currentOwner) check.
The system does not enforce uniqueness of signatures in the signatures array.

External pre-conditions

When the checkTransaction method is called, the constructed signatures array is used to bypass verification.

Attack Path

The attacker calls a function requiring signature validation, such as checkTransaction.
Supplies a signatures array containing multiple identical signatures.
The _countValidSignatures method iterates through signatures, validates each signature without checking uniqueness.
The duplicate signatures are counted multiple times, allowing the attacker to meet the signature threshold (threshold) and bypass the validation logic.

Impact

Affected Party: Safe Signature Verification Mechanism
This vulnerability allows an attacker to bypass signature count validation, potentially leading to:

Approval of unauthorized transactions by inflating the valid signature count to meet the threshold.
Severe security degradation, enabling an attacker to manipulate Safe transaction approval logic.

PoC

No response

Mitigation

Introduce a deduplication mechanism in _countValidSignatures, such as tracking validated signer addresses to avoid counting duplicates.
Add signature format validation to enforce uniqueness within the signatures array.
Add external constraints during signature validation to prevent the construction of duplicate signatures.

@sherlock-admin2 sherlock-admin2 changed the title Dizzy Tan Parakeet - _countValidSignatures Allows Duplicate Signatures, Leading to Incorrect Signature Count Taiger4526 - _countValidSignatures Allows Duplicate Signatures, Leading to Incorrect Signature Count Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant