Audit/dapphub b02 gas relaying check (#85)

* add SafeMath, add dummy check for gas sufficiency

* gas_overhead const

* custom safemath

* add gasLimit param for testing, safemath credit

packages/contracts/contracts/optimistic-ethereum/OVM/accounts/OVM_ECDSAContractAccount.sol

@@ -9,6 +9,7 @@ import { iOVM_ECDSAContractAccount } from "../../iOVM/accounts/iOVM_ECDSAContrac
 import { Lib_OVMCodec } from "../../libraries/codec/Lib_OVMCodec.sol";
 import { Lib_ECDSAUtils } from "../../libraries/utils/Lib_ECDSAUtils.sol";
 import { Lib_SafeExecutionManagerWrapper } from "../../libraries/wrappers/Lib_SafeExecutionManagerWrapper.sol";
+import { Lib_SafeMathWrapper } from "../../libraries/wrappers/Lib_SafeMathWrapper.sol";
 
 /**
  * @title OVM_ECDSAContractAccount
@@ -16,6 +17,7 @@ import { Lib_SafeExecutionManagerWrapper } from "../../libraries/wrappers/Lib_Sa
 contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
 
     address constant ETH_ERC20_ADDRESS = 0x4200000000000000000000000000000000000006;
+    uint256 constant EXECUTION_VALIDATION_GAS_OVERHEAD = 25000; // TODO: should be the amount sufficient to cover the gas costs of all of the transactions up to and including the CALL/CREATE which forms the entrypoint of the transaction.
 
     /********************
      * Public Functions *
@@ -76,6 +78,12 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
             "Transaction nonce does not match the expected nonce."
         );
 
+        // Need to make sure that the gas is sufficient to execute the transaction.
+        Lib_SafeExecutionManagerWrapper.safeREQUIRE(
+           gasleft() >= Lib_SafeMathWrapper.add(decodedTx.gasLimit, EXECUTION_VALIDATION_GAS_OVERHEAD),
+           "Gas is not sufficient to execute the transaction."
+        );
+
         // Transfer fee to relayer.
         address relayer = Lib_SafeExecutionManagerWrapper.safeCALLER();
         uint256 fee = decodedTx.gasLimit * decodedTx.gasPrice;
@@ -88,7 +96,7 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
         // Contract creations are signalled by sending a transaction to the zero address.
         if (decodedTx.to == address(0)) {
             address created = Lib_SafeExecutionManagerWrapper.safeCREATE(
-                decodedTx.gasLimit - 2000,
+                decodedTx.gasLimit,
                 decodedTx.data
             );
 

packages/contracts/contracts/optimistic-ethereum/libraries/wrappers/Lib_SafeMathWrapper.sol

+// SPDX-License-Identifier: MIT
+// Pulled from @openzeppelin/contracts/math/SafeMath.sol
+pragma solidity ^0.7.0;
+
+/* Library Imports */
+import { Lib_SafeExecutionManagerWrapper } from "./Lib_SafeExecutionManagerWrapper.sol";
+
+/**
+ * @title Lib_SafeMathWrapper
+ */
+
+/**
+ * @dev Wrappers over Solidity's arithmetic operations with added overflow
+ * checks.
+ *
+ * Arithmetic operations in Solidity wrap on overflow. This can easily result
+ * in bugs, because programmers usually assume that an overflow raises an
+ * error, which is the standard behavior in high level programming languages.
+ * `SafeMath` restores this intuition by reverting the transaction when an
+ * operation overflows.
+ *
+ * Using this library instead of the unchecked operations eliminates an entire
+ * class of bugs, so it's recommended to use it always.
+ */
+
+library Lib_SafeMathWrapper {
+    /**
+     * @dev Returns the addition of two unsigned integers, reverting on
+     * overflow.
+     *
+     * Counterpart to Solidity's `+` operator.
+     *
+     * Requirements:
+     *
+     * - Addition cannot overflow.
+     */
+    function add(uint256 a, uint256 b) internal returns (uint256) {
+        uint256 c = a + b;
+        Lib_SafeExecutionManagerWrapper.safeREQUIRE(c >= a, "Lib_SafeMathWrapper: addition overflow");
+
+        return c;
+    }
+
+    /**
+     * @dev Returns the subtraction of two unsigned integers, reverting on
+     * overflow (when the result is negative).
+     *
+     * Counterpart to Solidity's `-` operator.
+     *
+     * Requirements:
+     *
+     * - Subtraction cannot overflow.
+     */
+    function sub(uint256 a, uint256 b) internal returns (uint256) {
+        return sub(a, b, "Lib_SafeMathWrapper: subtraction overflow");
+    }
+
+    /**
+     * @dev Returns the subtraction of two unsigned integers, reverting with custom message on
+     * overflow (when the result is negative).
+     *
+     * Counterpart to Solidity's `-` operator.
+     *
+     * Requirements:
+     *
+     * - Subtraction cannot overflow.
+     */
+    function sub(uint256 a, uint256 b, string memory errorMessage) internal returns (uint256) {
+        Lib_SafeExecutionManagerWrapper.safeREQUIRE(b <= a, errorMessage);
+        uint256 c = a - b;
+
+        return c;
+    }
+
+    /**
+     * @dev Returns the multiplication of two unsigned integers, reverting on
+     * overflow.
+     *
+     * Counterpart to Solidity's `*` operator.
+     *
+     * Requirements:
+     *
+     * - Multiplication cannot overflow.
+     */
+    function mul(uint256 a, uint256 b) internal returns (uint256) {
+        // Gas optimization: this is cheaper than requiring 'a' not being zero, but the
+        // benefit is lost if 'b' is also tested.
+        // See: https://github.com/OpenZeppelin/openzeppelin-contracts/pull/522
+        if (a == 0) {
+            return 0;
+        }
+
+        uint256 c = a * b;
+        Lib_SafeExecutionManagerWrapper.safeREQUIRE(c / a == b, "Lib_SafeMathWrapper: multiplication overflow");
+
+        return c;
+    }
+
+    /**
+     * @dev Returns the integer division of two unsigned integers. Reverts on
+     * division by zero. The result is rounded towards zero.
+     *
+     * Counterpart to Solidity's `/` operator. Note: this function uses a
+     * `revert` opcode (which leaves remaining gas untouched) while Solidity
+     * uses an invalid opcode to revert (consuming all remaining gas).
+     *
+     * Requirements:
+     *
+     * - The divisor cannot be zero.
+     */
+    function div(uint256 a, uint256 b) internal returns (uint256) {
+        return div(a, b, "Lib_SafeMathWrapper: division by zero");
+    }
+
+    /**
+     * @dev Returns the integer division of two unsigned integers. Reverts with custom message on
+     * division by zero. The result is rounded towards zero.
+     *
+     * Counterpart to Solidity's `/` operator. Note: this function uses a
+     * `revert` opcode (which leaves remaining gas untouched) while Solidity
+     * uses an invalid opcode to revert (consuming all remaining gas).
+     *
+     * Requirements:
+     *
+     * - The divisor cannot be zero.
+     */
+    function div(uint256 a, uint256 b, string memory errorMessage) internal returns (uint256) {
+        Lib_SafeExecutionManagerWrapper.safeREQUIRE(b > 0, errorMessage);
+        uint256 c = a / b;
+        // assert(a == b * c + a % b); // There is no case in which this doesn't hold
+
+        return c;
+    }
+
+    /**
+     * @dev Returns the remainder of dividing two unsigned integers. (unsigned integer modulo),
+     * Reverts when dividing by zero.
+     *
+     * Counterpart to Solidity's `%` operator. This function uses a `revert`
+     * opcode (which leaves remaining gas untouched) while Solidity uses an
+     * invalid opcode to revert (consuming all remaining gas).
+     *
+     * Requirements:
+     *
+     * - The divisor cannot be zero.
+     */
+    function mod(uint256 a, uint256 b) internal returns (uint256) {
+        return mod(a, b, "Lib_SafeMathWrapper: modulo by zero");
+    }
+
+    /**
+     * @dev Returns the remainder of dividing two unsigned integers. (unsigned integer modulo),
+     * Reverts with custom message when dividing by zero.
+     *
+     * Counterpart to Solidity's `%` operator. This function uses a `revert`
+     * opcode (which leaves remaining gas untouched) while Solidity uses an
+     * invalid opcode to revert (consuming all remaining gas).
+     *
+     * Requirements:
+     *
+     * - The divisor cannot be zero.
+     */
+    function mod(uint256 a, uint256 b, string memory errorMessage) internal returns (uint256) {
+        Lib_SafeExecutionManagerWrapper.safeREQUIRE(b != 0, errorMessage);
+        return a % b;
+    }
+}
\ No newline at end of file

packages/contracts/test/contracts/OVM/accounts/OVM_ECDSAContractAccount.spec.ts

@@ -19,8 +19,16 @@ const callPrecompile = async (
   Helper_PrecompileCaller: Contract,
   precompile: Contract,
   functionName: string,
-  functionParams?: any[]
+  functionParams?: any[],
+  gasLimit?: number
 ): Promise<any> => {
+  if (gasLimit) {
+    return Helper_PrecompileCaller.callPrecompile(
+      precompile.address,
+      precompile.interface.encodeFunctionData(functionName, functionParams || []),
+      {gasLimit}
+    )
+  }
   return Helper_PrecompileCaller.callPrecompile(
     precompile.address,
     precompile.interface.encodeFunctionData(functionName, functionParams || [])
@@ -177,8 +185,10 @@ describe('OVM_ECDSAContractAccount', () => {
     })
 
     it(`should revert on incorrect nonce`, async () => {
-      const alteredNonceTx = DEFAULT_EIP155_TX
-      alteredNonceTx.nonce = 99
+      const alteredNonceTx = {
+        ...DEFAULT_EIP155_TX,
+        nonce : 99
+      }
       const message = serializeNativeTransaction(alteredNonceTx)
       const sig = await signNativeTransaction(wallet, alteredNonceTx)
 
@@ -227,5 +237,34 @@ describe('OVM_ECDSAContractAccount', () => {
         'Transaction chainId does not match expected OVM chainId.'
       )
     })
+
+    it(`should revert on insufficient gas`, async () => {
+      const alteredInsufficientGasTx = {
+        ...DEFAULT_EIP155_TX,
+        gasLimit : 200000000
+      }
+      const message = serializeNativeTransaction(alteredInsufficientGasTx)
+      const sig = await signNativeTransaction(wallet, alteredInsufficientGasTx)
+
+      await callPrecompile(
+        Helper_PrecompileCaller,
+        OVM_ECDSAContractAccount,
+        'execute',
+        [
+          message,
+          0, //isEthSignedMessage
+          `0x${sig.v}`, //v
+          `0x${sig.r}`, //r
+          `0x${sig.s}`, //s
+        ],
+        40000000,
+      )
+
+      const ovmREVERT: any =
+        Mock__OVM_ExecutionManager.smocked.ovmREVERT.calls[0]
+      expect(ethers.utils.toUtf8String(ovmREVERT._data)).to.equal(
+        'Gas is not sufficient to execute the transaction.'
+      )
+    })
   })
 })