Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect management of requested gas amount in EIP-4337 logic #513

Closed
code423n4 opened this issue Jan 9, 2023 · 3 comments
Closed

Incorrect management of requested gas amount in EIP-4337 logic #513

code423n4 opened this issue Jan 9, 2023 · 3 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-303 satisfactory satisfies C4 submission criteria; eligible for awards sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons

Comments

@code423n4
Copy link
Contributor

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);
@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Jan 9, 2023
code423n4 added a commit that referenced this issue Jan 9, 2023
@c4-judge
Copy link
Contributor

gzeon-c4 marked the issue as duplicate of #303

@c4-sponsor
Copy link

livingrockrises marked the issue as sponsor acknowledged

@c4-sponsor c4-sponsor added the sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons label Feb 9, 2023
@c4-judge
Copy link
Contributor

gzeon-c4 marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Feb 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-303 satisfactory satisfies C4 submission criteria; eligible for awards sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons
Projects
None yet
Development

No branches or pull requests

3 participants