sherlock-audit / 2023-07-kyber-swap-judging

12 stars 8 forks source link

Bauer - Use native account abstraction over ecrecover for validation on the zkSync #95

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Bauer

high

Use native account abstraction over ecrecover for validation on the zkSync

Summary

Use native account abstraction over ecrecover for validation on zkSync.

Vulnerability Detail

According to the description in the document, the contracts are supposed to be deployed to any EVM-compatible chains.The zkSync Era is the first EVM-compatible chain to implement native account abstraction. The accounts in the zkSync Era can initiate transactions like Externally Owned Accounts (EOAs), but they can also implement any logic within them, like smart contracts. This feature is known as “Account Abstraction” (AA). Therefore, when performing signature verification, one should not rely on the fact that the account has an ECDSA private key, as the account may be managed by multi-signatures and use other signature schemes. https://era.zksync.io/docs/dev/building-on-zksync/best-practices.html#use-native-account-abstraction-over-ecrecover-for-validation It is recommend to use native account abstraction over ecrecover for validation.

 require(spender != owner, 'ERC721Permit: approval to current owner');

    if (Address.isContract(owner)) {
      require(
        IERC1271(owner).isValidSignature(digest, abi.encodePacked(r, s, v)) == 0x1626ba7e,
        'Unauthorized'
      );
    } else {
      address recoveredAddress = ecrecover(digest, v, r, s);
      require(recoveredAddress != address(0), 'Invalid signature');
      require(recoveredAddress == owner, 'Unauthorized');
    }

Impact

Some accounts using different signatures might be unusable

Code Snippet

https://github.com/sherlock-audit/2023-07-kyber-swap/blob/main/ks-elastic-sc/contracts/periphery/base/ERC721Permit.sol#L93

Tool used

Vscode Manual Review

Recommendation

It is recommend to use native account abstraction over ecrecover for validation

sherlock-admin commented 1 year ago

1 comment(s) were left on this issue during the judging contest.

Trumpero commented:

low