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 issue #52

Closed
c4-submissions opened this issue Nov 5, 2023 · 4 comments
Closed

Signature malleability issue #52

c4-submissions opened this issue Nov 5, 2023 · 4 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-94 insufficient quality report This report is not of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-submissions
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-10-badger/blob/main/packages/contracts/contracts/EBTCToken.sol#L218

Vulnerability details

Impact

ecrecover() is vulnerable to signature malleability.Attackers can compute a valid signature without knowing the signer's private key

Proof of Concept

The elliptic curve used in Ethereum for signatures is symmetrical, hence for every [v,r,s] there exists another [v,r,s] that returns the same valid result. Therefore two valid signatures exist which allows attackers to compute a valid signature without knowing the signer's private key. ecrecover() is vulnerable to signature malleability so it can be dangerous to use it directly.

 function permit(
        address owner,
        address spender,
        uint256 amount,
        uint256 deadline,
        uint8 v,
        bytes32 r,
        bytes32 s
    ) external override {
        require(deadline >= block.timestamp, "EBTC: expired deadline");
        bytes32 digest = keccak256(
            abi.encodePacked(
                "\x19\x01",
                domainSeparator(),
                keccak256(
                    abi.encode(_PERMIT_TYPEHASH, owner, spender, amount, _nonces[owner]++, deadline)
                )
            )
        );
        address recoveredAddress = ecrecover(digest, v, r, s);
        require(recoveredAddress == owner, "EBTC: invalid signature");
        _approve(owner, spender, amount);
    }

An attacker can compute another corresponding [v,r,s] that will make this check pass due to the symmetrical nature of the elliptic curve. The easiest way to prevent this issue is to use OpenZeppelin’s ECDSA.sol library and reading the comments above ECDSA's tryRecover() function provides very useful information on correctly implementing signature checks to prevent signature malleability vulnerabilities.

When using OpenZeppelin's ECDSA library, special care must be taken to use version 4.7.3 or greater, since previous versions contained a signature malleability bug.

Tools Used

Vscode

Recommended Mitigation Steps

Use OpenZeppelin’s ECDSA.sol

Assessed type

Invalid Validation

@c4-submissions c4-submissions added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Nov 5, 2023
c4-submissions added a commit that referenced this issue Nov 5, 2023
@c4-pre-sort
Copy link

bytes032 marked the issue as insufficient quality report

@c4-pre-sort c4-pre-sort added the insufficient quality report This report is not of sufficient quality label Nov 15, 2023
@c4-pre-sort
Copy link

bytes032 marked the issue as duplicate of #94

@c4-judge
Copy link

jhsagd76 marked the issue as unsatisfactory:
Out of scope

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Nov 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-94 insufficient quality report This report is not of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

4 participants