search

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

10 stars 11 forks source link

Griefing attacks on `handleOps` and `multiSend` logic #499

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-biconomy/blob/5df2e8f8c0fd3393b9ecdad9ef356955f07fbbdd/scw-contracts/contracts/smart-contract-wallet/aa-4337/core/EntryPoint.sol#L68 https://github.com/code-423n4/2023-01-biconomy/blob/5df2e8f8c0fd3393b9ecdad9ef356955f07fbbdd/scw-contracts/contracts/smart-contract-wallet/libs/MultiSend.sol#L26

Vulnerability details

Description

The handleOps function executes an array of UserOperation. If at least one user operation fails the whole transaction will revert. That means the error on one user ops will fully reverts the other executed ops.

The multiSend function reverts if at least one of the transactions fails, so it is also vulnerable to such type of attacks.

Attack scenario

Relayer offchain verify the batch of UserOperations, convinced that they will receive fees, then send the handleOps transaction to the mempool. An attacker front-run the relayers transaction with another handleOps transaction that executes only one UserOperation, the last user operation from the relayers handleOps operations. An attacker will receive the funds for one UserOperation. Original relayers transaction will consume gas for the execution of all except one, user ops, but reverts at the end.

Impact

Griefing attacks on the gas used for handleOps and multiSend function calls.

Please note, that while an attacker have no direct incentive to make such an attacks, they could short the token before the attack.

Recommended Mitigation Steps

Remove redundant require-like checks from internal functions called from the handleOps function and add the non-atomic execution logic to the multiSend function.

c4-judge commented 1 year ago

gzeon-c4 marked the issue as duplicate of #90

c4-judge commented 1 year ago

gzeon-c4 marked the issue as not a duplicate

c4-judge commented 1 year ago

gzeon-c4 marked the issue as primary issue

livingrockrises commented 1 year ago

once public will double check with infinitism community. marked acknowledged for now. and for multisend non-atomic does not make sense!

c4-sponsor commented 1 year ago

livingrockrises marked the issue as sponsor acknowledged

c4-judge commented 1 year ago

gzeon-c4 marked the issue as selected for report