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

Signature malleability in permit function #44

Open
hats-bug-reporter bot opened this issue Jun 22, 2023 · 3 comments
Open

Signature malleability in permit function #44

hats-bug-reporter bot opened this issue Jun 22, 2023 · 3 comments
Labels
bug Something isn't working wontfix This will not be worked on

Comments

@hats-bug-reporter
Copy link

Github username: --
Submission hash (on-chain): 0xe3c62c6546edd8b652a731dbcbaff6f24d971a29fe18cdbb29ce88ff9487b030
Severity: high severity

Description:

Impact

The function permit contains a vulnerability in its signature verification process that can result in signature collisions. This vulnerability arises due to the symmetrical nature of the elliptic curve used for signatures in Ethereum. As a consequence, multiple valid signatures can produce the same outcome without requiring the possession of the private key. Attackers can exploit this vulnerability to manipulate contract behavior and bypass security measures.

POC

The function aims to implement a cryptographic signature system for verifying signatures in Ethereum contracts. However, it fails to consider the uniqueness of signatures, assuming that a valid signature is always unique. This assumption can lead to severe security vulnerabilities.

The vulnerability allows an attacker to create alternative signatures that, when verified, produce the same valid result as the original signature. Consequently, an attacker can replace a legitimate signature with a colliding signature, potentially bypassing security checks and altering contract behavior.

The vulnerability arises from the fact that for every [v,r,s] there exists another [v,r,s] that returns the same valid result. This symmetrical property of the elliptic curve used for signatures in Ethereum can be exploited by an attacker to craft alternative signatures that resemble with the original one. This resembling signatures, while different from the original, can still produce the same valid result during signature verification.

For more details about how this may cause problem in this function you can check these 3 links below:
https://swcregistry.io/docs/SWC-117
https://swcregistry.io/docs/SWC-121
https://medium.com/immunefi/intro-to-cryptography-and-signatures-in-ethereum-2025b6a4a33d

By leveraging signature resemblance, an attacker can manipulate the behavior of the delegateBySig function and potentially bypass security checks. They can remake a valid signature causing the function to accept the colliding signature as legitimate, leading to unintended delegation of authority or unauthorized access to certain functionalities..

Recommended Mitigation Steps

The easiest and simplest way to prevent this kind of attacks happen is using OpenZeppelin’s ECDSA.sol version 4.7.3 or greater(older versions have bugs in the implementation).

@hats-bug-reporter hats-bug-reporter bot added the bug Something isn't working label Jun 22, 2023
@ksyao2002
Copy link

Aave has utilized the exact same permit function (since this is a fork of aave), and they never had any vulnerabilities related to this. The signature malleability vulnerability you mention is "doubt these pose a serious issue in practice" (reference: https://cronokirby.com/posts/2022/02/on-the-malleability-of-ecdsa/). Otherwise I'd assume aave doesn't just use it directly. However, since this is a valid finding, it may be considered a low vulnerability since no funds are at risk, although it will be decided if it will even be changed from Aave's tried and trusted methods.

@Nabeel-javaid
Copy link

Aave has utilized the exact same permit function (since this is a fork of aave), and they never had any vulnerabilities related to this. The signature malleability vulnerability you mention is "doubt these pose a serious issue in practice" (reference: https://cronokirby.com/posts/2022/02/on-the-malleability-of-ecdsa/). Otherwise I'd assume aave doesn't just use it directly. However, since this is a valid finding, it may be considered a low vulnerability since no funds are at risk, although it will be decided if it will even be changed from Aave's tried and trusted methods.

Thank you for considering this a valid finding, also maybe aave has performed some kind of security measure to prevent this from happening. You can learn more about this from this Link

@ksyao2002
Copy link

ksyao2002 commented Jul 3, 2023

Update on this: I reached out to Aave to see if this is an issue, since we directly forked their implementation of permit, and this was their response:

Hi Kevin,

Thanks for reaching out.

The AToken contract (and others of the Aave codebase) prevents signature malleability and replay attacks by including nonces (and other parameters like chainId, name, and address of the verifying contract) within the signature.

Neither it is susceptible to the issue of OpenZeppelin's ECDSA contract that was patched in version 4.7.3 related to the EIP-2098 compacted version of the signature, because the code using the native `ecrecover` (instead of the library's one). 

Thus, I will close this issue since no fix is needed.

@ksyao2002 ksyao2002 added the wontfix This will not be worked on label Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants