Allows malleable SECP256K1 signatures

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/paymasters/verifying/singleton/VerifyingSingletonPaymaster.sol#L107
https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/interfaces/IERC1271Wallet.sol#L18
https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/interfaces/IERC1271Wallet.sol#L34
https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L347-L350

Vulnerability details

Impact

It is possible to use compact signatures and this allows malleability of the signature.

Description

Here, the ecrecover() method doesn't check the s range and also compact signatures must not be allowed in order to prevent signature malleability.

Homestead (EIP-2) added this limitation, however the precompile remained unaltered. The majority of libraries, including OpenZeppelin, do this check.

Since an order can only be confirmed once and its hash is saved, there doesn't seem to be a serious danger in existing use cases.

Reference:

• GHSA-4h98-2769-gh6h
• Fix ECDSA signature malleability OpenZeppelin/openzeppelin-contracts#3610
• https://github.com/OpenZeppelin/openzeppelin-contracts/blob/7201e6707f6631d9499a569f492870ebdd4133cf/contracts/utils/cryptography/ECDSA.sol#L138-L149

Affected source code:

• VerifyingSingletonPaymaster.sol:107

Additionally, the following interfaces are susceptible to being implemented incorrectly:

• IERC1271Wallet.sol:18
• IERC1271Wallet.sol:34

ecrecover:

• SmartAccount.sol:347-350

Recommended Mitigation Steps

As Open Zeppelin did, it's better to remove the compact signatures features.

[c4-judge]

gzeon-c4 marked the issue as primary issue

[gzeoneth]

#194 suggested the OZ lib used prevented this from happening

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory:
Invalid