JuggerNaut63 - Signature Malleability in _isValidSignature Function

[sherlock-admin4]

JuggerNaut63

High

Signature Malleability in _isValidSignature Function

Summary

The _isValidSignature function in the SDAO contract uses the built-in ecrecover function, which is susceptible to signature malleability. This vulnerability can allow attackers to manipulate signatures, potentially leading to unauthorized access and loss of funds.

Vulnerability Detail

The ecrecover function is used to validate signatures. However, it is known to be vulnerable to signature malleability, meaning the same message can be signed in multiple ways. This can allow attackers to alter the signature without invalidating it, leading to potential security breaches. https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/endgame-toolkit/src/SDAO.sol#L343

Impact

• Unauthorized Access
• Loss of Funds
• Replay Attacks

Code Snippet

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/endgame-toolkit/src/SDAO.sol#L333-L358

Tool used

Manual Review

Recommendation

1. Import OpenZeppelin's ECDSA Library

2. Replace ecrecover with ECDSA Library

   + import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
   
   function _isValidSignature(address signer, bytes32 digest, bytes memory signature) internal view returns (bool) {
       if (signature.length == 65) {
           bytes32 r;
           bytes32 s;
           uint8 v;
           assembly {
               r := mload(add(signature, 0x20))
               s := mload(add(signature, 0x40))
               v := byte(0, mload(add(signature, 0x60)))
           }
   -           if (signer == ecrecover(digest, v, r, s)) {
   +           if (signer == ECDSA.recover(digest, signature)) {
               return true;
           }
       }
   
       if (signer.code.length > 0) {
           (bool success, bytes memory result) = signer.staticcall(
               abi.encodeCall(IERC1271.isValidSignature, (digest, signature))
           );
           return (success &&
               result.length == 32 &&
               abi.decode(result, (bytes4)) == IERC1271.isValidSignature.selector);
       }
   
       return false;
   }

Duplicate of #8

[telome]

An ECDSA signature should never be used by a smart contract as part of a key meant to represent a unique signed action. This is unrelated to the permit functionality and is entirely the responsibility of the third party contract.