code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-biconomy/blob/main/scw-contracts/contracts/smart-contract-wallet/aa-4337/core/EntryPoint.sol#L176 https://github.com/code-423n4/2023-01-biconomy/blob/main/scw-contracts/contracts/smart-contract-wallet/aa-4337/core/EntryPoint.sol#L265 https://github.com/code-423n4/2023-01-biconomy/blob/main/scw-contracts/contracts/smart-contract-wallet/aa-4337/core/EntryPoint.sol#L319 https://github.com/code-423n4/2023-01-biconomy/blob/main/scw-contracts/contracts/smart-contract-wallet/aa-4337/core/EntryPoint.sol#L363 https://github.com/code-423n4/2023-01-biconomy/blob/main/scw-contracts/contracts/smart-contract-wallet/aa-4337/core/EntryPoint.sol#L455 https://github.com/code-423n4/2023-01-biconomy/blob/main/scw-contracts/contracts/smart-contract-wallet/aa-4337/core/EntryPoint.sol#L458

Vulnerability details

Description

According to the EIP-150 call can consume as most 63/64 of parent calls' gas. That means that it is possible to manipulate the gas amount to be passed into calls mentioned in the "Links to affected code" section. Specifically, if the amount of gas that is left in the current execution flow is smaller than requiredGasForTheCall*64/63 (where requiredGasForTheCall means the amount of gas that should be passed to the following call) then the wrong amount of gas will be passed, but the parent execution can be finished using the rest of the gas (which can be as big as requiredGasForTheCall/64), so the refund for the transaction execution will be provided to the relayer.

Attack scenario

Let's see the _executeUserOp logic:

try this.innerHandleOp(userOp.callData, opInfo, context) returns (uint256 _actualGasCost) {
    collected = _actualGasCost;
} catch {
    uint256 actualGas = preGas - gasleft() + opInfo.preOpGas;
    collected = _handlePostOp(opIndex, IPaymaster.PostOpMode.postOpReverted, opInfo, context, actualGas);
}

According to the 63/64 rule and code above, the innerHandleOp could be reverted due to gas limit issue on the child call and the all remaining gas will be spent on the _handlePostOp, that save the relayer refund. Thus, the relayer can fail execute transactions but invalidate the nonce and take funds for relay.

For a rough estimate of the cost of user operations that are subject to this type of attack, consider operation with 2_000_000 gas limit. Then, the 2_000_000 / 64 == 31_250, that should be enough to execute _handlePostOp without paymaster and rewrite one storage slot.

Impact

Any sufficiently expensive transaction can be forcfully failed by the relayer, but the relayer will receive refund. The problem may be occurs on the malicious relayer or even honest if it will send relay transaction with certain gas.

Recommended Mitigation Steps

Include logic related to the EIP-150 specification to the gas management. You can use the following:

Check that the amount of gas left is great enough:

require(gasleft()*63/64 >= requiredGasForTheCall);

Call with the required gas parameter:

contractAddress.call{gas: requiredGasForTheCall}(dataForTheCall);

c4-judge commented 1 year ago

gzeon-c4 marked the issue as duplicate of #303

c4-sponsor commented 1 year ago

livingrockrises marked the issue as sponsor acknowledged