code-423n4 / 2023-01-biconomy-findings

10 stars 11 forks source link

Wrong decoding of paymaster data makes validatePaymasterUserOp always fail, DoS #531

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-biconomy/blob/main/scw-contracts/contracts/smart-contract-wallet/paymasters/verifying/singleton/VerifyingSingletonPaymaster.sol#L102

Vulnerability details

Impact

DoS of validatePaymasterUserOp makes UserOperation's with paymaster not executable

Proof of Concept

. decodePaymasterData on line 102 in VerifyingSingletonPaymaster.validatePaymasterUserOp returns wrong data and makes function always fail due to the following checks. This means that UserOperation with paymaster are not executable as will always fail validation from the paymaster.

The problem is that .decodePaymasterData in PaymasterHelpers.sol on line 36 cuts the first 20 byte from the paymasterAndData where the paymasterId is actually located and returns wrong values for (address paymasterId, bytes memory signature)

Tools Used

Manual review

Recommended Mitigation Steps

Fix decoding in PaymasterHelpers.decodePaymasterData

-       (address paymasterId, bytes memory signature) = abi.decode(paymasterAndData[20:], (address, bytes));
+       (address paymasterId, bytes memory signature) = abi.decode(paymasterAndData, (address, bytes));
c4-judge commented 1 year ago

gzeon-c4 marked the issue as primary issue

c4-sponsor commented 1 year ago

livingrockrises marked the issue as sponsor disputed

livingrockrises commented 1 year ago

first 20 bytes of paymasterAndData is the paymasterAddress! then starts the paymasterId packed with signature!

c4-judge commented 1 year ago

gzeon-c4 marked the issue as unsatisfactory: Invalid