AA-101: validate gasLimit to leave more than callGasLimit to call (#163)

* AA-101: validate gasLimit to leave more than callGasLimit to target call.

Fix vulnerability reported by Richard Meisner in PR #162 

The added tests should show the scenario where a user operation with a high callGasLimit is submitted. In this case it is important that the gasLimit is correctly set, else it is possible to use the 1/64th rule of EIP-150 to make the user operation fail and the account pays for it.

If this is possible it could be used as an attack vector. The attacker would submit a bundle with the high gas usage tx with a too low gas value. Even when the user estimated everything correctly the transaction would fail because not enough gas is available. The costs for the execution would still be deducted from the account. Therefore the submitter could perform a denial of service attack for which the account that is being attacked would pay.

The first tests below shows that high gas transaction can be executed and refunded. The second test checks that the transaction is reverted in case the gas limit is set too low, to avoid the attack described above.

contracts/core/EntryPoint.sol

@@ -25,6 +25,9 @@ contract EntryPoint is IEntryPoint, StakeManager {
     // internal value used during simulation: need to query aggregator.
     address private constant SIMULATE_FIND_AGGREGATOR = address(1);
 
+    // marker for inner call revert on out of gas
+    bytes32 private constant INNER_OUT_OF_GAS = hex'deaddead';
+
     /**
      * for simulation purposes, validateUserOp (and validatePaymasterUserOp) must return this value
      * in case of signature failure, instead of revert.
@@ -56,6 +59,18 @@ contract EntryPoint is IEntryPoint, StakeManager {
         try this.innerHandleOp(userOp.callData, opInfo, context) returns (uint256 _actualGasCost) {
             collected = _actualGasCost;
         } catch {
+            bytes32 innerRevertCode;
+            assembly {
+                returndatacopy(0, 0, 32)
+                innerRevertCode := mload(0)
+            }
+            // handleOps was called with gas limit too low. abort entire bundle.
+            if(innerRevertCode == INNER_OUT_OF_GAS) {
+                //report paymaster, since if it is deliberately caused by the bundler,
+                // it must be a revert caused by paymaster.
+                revert FailedOp(opIndex, opInfo.mUserOp.paymaster, "AA95 out of gas");
+            }
+
             uint256 actualGas = preGas - gasleft() + opInfo.preOpGas;
             collected = _handlePostOp(opIndex, IPaymaster.PostOpMode.postOpReverted, opInfo, context, actualGas);
         }
@@ -191,10 +206,21 @@ contract EntryPoint is IEntryPoint, StakeManager {
         require(msg.sender == address(this), "AA92 internal call only");
         MemoryUserOp memory mUserOp = opInfo.mUserOp;
 
+        uint callGasLimit = mUserOp.callGasLimit;
+    unchecked {
+        // handleOps was called with gas limit too low. abort entire bundle.
+        if (gasleft() < callGasLimit + mUserOp.verificationGasLimit + 5000) {
+            assembly {
+                mstore(0, INNER_OUT_OF_GAS)
+                revert(0, 32)
+            }
+        }
+    }
+
         IPaymaster.PostOpMode mode = IPaymaster.PostOpMode.opSucceeded;
         if (callData.length > 0) {
 
-            (bool success,bytes memory result) = address(mUserOp.sender).call{gas : mUserOp.callGasLimit}(callData);
+            (bool success,bytes memory result) = address(mUserOp.sender).call{gas : callGasLimit}(callData);
             if (!success) {
                 if (result.length > 0) {
                     emit UserOperationRevertReason(opInfo.userOpHash, mUserOp.sender, mUserOp.nonce, result);

reports/gas-checker.txt

@@ -1,35 +1,35 @@
 == gas estimate of direct calling the account's "execFromEntryPoint" method
    the destination is "account.nonce()", which is known to be "hot" address used by this account
    it little higher than EOA call: its an exec from entrypoint (or account owner) into account contract, verifying msg.sender and exec to target)
-- gas estimate "simple" - 31033
+- gas estimate "simple" - 31045
 - gas estimate "big tx 5k" - 127295
 ╔════════════════════════════════╤═══════╤═══════════════╤════════════════╤═════════════════════╗
 ║ handleOps description          │ count │ total gasUsed │ per UserOp gas │ per UserOp overhead ║
 ║                                │       │               │     (delta for │        (compared to ║
 ║                                │       │               │    one UserOp) │     account.exec()) ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple                         │     1 │         77987 │                │                     ║
+║ simple                         │     1 │         78043 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │     2 │               │          43461 │               12428 ║
+║ simple - diff from previous    │     2 │               │          43517 │               12472 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple                         │    10 │        469401 │                │                     ║
+║ simple                         │    10 │        469937 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │    11 │               │          43620 │               12587 ║
+║ simple - diff from previous    │    11 │               │          43664 │               12619 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster               │     1 │         77999 │                │                     ║
+║ simple paymaster               │     1 │         78055 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │     2 │               │          43473 │               12440 ║
+║ simple paymaster with diff     │     2 │               │          43505 │               12460 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster               │    10 │        469401 │                │                     ║
+║ simple paymaster               │    10 │        469937 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │    11 │               │          43632 │               12599 ║
+║ simple paymaster with diff     │    11 │               │          43640 │               12595 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx 5k                      │     1 │        179730 │                │                     ║
+║ big tx 5k                      │     1 │        179774 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx - diff from previous    │     2 │               │         144806 │               17511 ║
+║ big tx - diff from previous    │     2 │               │         144874 │               17579 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx 5k                      │    10 │       1488505 │                │                     ║
+║ big tx 5k                      │    10 │       1489029 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx - diff from previous    │    11 │               │         146311 │               19016 ║
+║ big tx - diff from previous    │    11 │               │         146355 │               19060 ║
 ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝
 

test/entrypoint.test.ts

@@ -434,6 +434,9 @@ describe('EntryPoint', function () {
       })
 
       it('account should pay for high gas usage tx', async function () {
+        if (process.env.COVERAGE != null) {
+          return
+        }
         const iterations = 45
         const count = await counter.populateTransaction.gasWaster(iterations, '')
         const accountExec = await account.populateTransaction.execute(counter.address, 0, count.data!)
@@ -489,7 +492,7 @@ describe('EntryPoint', function () {
         await expect(entryPoint.handleOps([op], beneficiaryAddress, {
           maxFeePerGas: 1e9,
           gasLimit: 12e5
-        })).to.revertedWith('Transaction ran out of gas')
+        })).to.revertedWith('AA95 out of gas')
 
         // Make sure that the user did not pay for the transaction
         expect(await getBalance(account.address)).to.eq(inititalAccountBalance)
@@ -650,7 +653,7 @@ describe('EntryPoint', function () {
         await fund(preAddr)
         createOp = await fillAndSign({
           initCode: getAccountInitCode(accountOwner.address, simpleAccountFactory, salt),
-          callGasLimit: 1e7,
+          callGasLimit: 1e6,
           verificationGasLimit: 2e6
 
         }, accountOwner, entryPoint)

test/paymaster.test.ts

@@ -130,7 +130,7 @@ describe('EntryPoint with paymaster', function () {
       it('should succeed to create account with tokens', async () => {
         createOp = await fillAndSign({
           initCode: getAccountDeployer(entryPoint.address, accountOwner.address, 3),
-          verificationGasLimit: 1e7,
+          verificationGasLimit: 2e6,
           paymasterAndData: paymaster.address,
           nonce: 0
         }, accountOwner, entryPoint)