Skip to content
This repository was archived by the owner on Apr 12, 2021. It is now read-only.

Commit 6a4d48a

Browse files
authored
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
1 parent 244424f commit 6a4d48a

File tree

3 files changed

+218
-4
lines changed

3 files changed

+218
-4
lines changed

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,15 @@ import { iOVM_ECDSAContractAccount } from "../../iOVM/accounts/iOVM_ECDSAContrac
99
import { Lib_OVMCodec } from "../../libraries/codec/Lib_OVMCodec.sol";
1010
import { Lib_ECDSAUtils } from "../../libraries/utils/Lib_ECDSAUtils.sol";
1111
import { Lib_SafeExecutionManagerWrapper } from "../../libraries/wrappers/Lib_SafeExecutionManagerWrapper.sol";
12+
import { Lib_SafeMathWrapper } from "../../libraries/wrappers/Lib_SafeMathWrapper.sol";
1213

1314
/**
1415
* @title OVM_ECDSAContractAccount
1516
*/
1617
contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
1718

1819
address constant ETH_ERC20_ADDRESS = 0x4200000000000000000000000000000000000006;
20+
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.
1921

2022
/********************
2123
* Public Functions *
@@ -76,6 +78,12 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
7678
"Transaction nonce does not match the expected nonce."
7779
);
7880

81+
// Need to make sure that the gas is sufficient to execute the transaction.
82+
Lib_SafeExecutionManagerWrapper.safeREQUIRE(
83+
gasleft() >= Lib_SafeMathWrapper.add(decodedTx.gasLimit, EXECUTION_VALIDATION_GAS_OVERHEAD),
84+
"Gas is not sufficient to execute the transaction."
85+
);
86+
7987
// Transfer fee to relayer.
8088
address relayer = Lib_SafeExecutionManagerWrapper.safeCALLER();
8189
uint256 fee = decodedTx.gasLimit * decodedTx.gasPrice;
@@ -88,7 +96,7 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
8896
// Contract creations are signalled by sending a transaction to the zero address.
8997
if (decodedTx.to == address(0)) {
9098
address created = Lib_SafeExecutionManagerWrapper.safeCREATE(
91-
decodedTx.gasLimit - 2000,
99+
decodedTx.gasLimit,
92100
decodedTx.data
93101
);
94102

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
// SPDX-License-Identifier: MIT
2+
// Pulled from @openzeppelin/contracts/math/SafeMath.sol
3+
pragma solidity ^0.7.0;
4+
5+
/* Library Imports */
6+
import { Lib_SafeExecutionManagerWrapper } from "./Lib_SafeExecutionManagerWrapper.sol";
7+
8+
/**
9+
* @title Lib_SafeMathWrapper
10+
*/
11+
12+
/**
13+
* @dev Wrappers over Solidity's arithmetic operations with added overflow
14+
* checks.
15+
*
16+
* Arithmetic operations in Solidity wrap on overflow. This can easily result
17+
* in bugs, because programmers usually assume that an overflow raises an
18+
* error, which is the standard behavior in high level programming languages.
19+
* `SafeMath` restores this intuition by reverting the transaction when an
20+
* operation overflows.
21+
*
22+
* Using this library instead of the unchecked operations eliminates an entire
23+
* class of bugs, so it's recommended to use it always.
24+
*/
25+
26+
library Lib_SafeMathWrapper {
27+
/**
28+
* @dev Returns the addition of two unsigned integers, reverting on
29+
* overflow.
30+
*
31+
* Counterpart to Solidity's `+` operator.
32+
*
33+
* Requirements:
34+
*
35+
* - Addition cannot overflow.
36+
*/
37+
function add(uint256 a, uint256 b) internal returns (uint256) {
38+
uint256 c = a + b;
39+
Lib_SafeExecutionManagerWrapper.safeREQUIRE(c >= a, "Lib_SafeMathWrapper: addition overflow");
40+
41+
return c;
42+
}
43+
44+
/**
45+
* @dev Returns the subtraction of two unsigned integers, reverting on
46+
* overflow (when the result is negative).
47+
*
48+
* Counterpart to Solidity's `-` operator.
49+
*
50+
* Requirements:
51+
*
52+
* - Subtraction cannot overflow.
53+
*/
54+
function sub(uint256 a, uint256 b) internal returns (uint256) {
55+
return sub(a, b, "Lib_SafeMathWrapper: subtraction overflow");
56+
}
57+
58+
/**
59+
* @dev Returns the subtraction of two unsigned integers, reverting with custom message on
60+
* overflow (when the result is negative).
61+
*
62+
* Counterpart to Solidity's `-` operator.
63+
*
64+
* Requirements:
65+
*
66+
* - Subtraction cannot overflow.
67+
*/
68+
function sub(uint256 a, uint256 b, string memory errorMessage) internal returns (uint256) {
69+
Lib_SafeExecutionManagerWrapper.safeREQUIRE(b <= a, errorMessage);
70+
uint256 c = a - b;
71+
72+
return c;
73+
}
74+
75+
/**
76+
* @dev Returns the multiplication of two unsigned integers, reverting on
77+
* overflow.
78+
*
79+
* Counterpart to Solidity's `*` operator.
80+
*
81+
* Requirements:
82+
*
83+
* - Multiplication cannot overflow.
84+
*/
85+
function mul(uint256 a, uint256 b) internal returns (uint256) {
86+
// Gas optimization: this is cheaper than requiring 'a' not being zero, but the
87+
// benefit is lost if 'b' is also tested.
88+
// See: https://github.com/OpenZeppelin/openzeppelin-contracts/pull/522
89+
if (a == 0) {
90+
return 0;
91+
}
92+
93+
uint256 c = a * b;
94+
Lib_SafeExecutionManagerWrapper.safeREQUIRE(c / a == b, "Lib_SafeMathWrapper: multiplication overflow");
95+
96+
return c;
97+
}
98+
99+
/**
100+
* @dev Returns the integer division of two unsigned integers. Reverts on
101+
* division by zero. The result is rounded towards zero.
102+
*
103+
* Counterpart to Solidity's `/` operator. Note: this function uses a
104+
* `revert` opcode (which leaves remaining gas untouched) while Solidity
105+
* uses an invalid opcode to revert (consuming all remaining gas).
106+
*
107+
* Requirements:
108+
*
109+
* - The divisor cannot be zero.
110+
*/
111+
function div(uint256 a, uint256 b) internal returns (uint256) {
112+
return div(a, b, "Lib_SafeMathWrapper: division by zero");
113+
}
114+
115+
/**
116+
* @dev Returns the integer division of two unsigned integers. Reverts with custom message on
117+
* division by zero. The result is rounded towards zero.
118+
*
119+
* Counterpart to Solidity's `/` operator. Note: this function uses a
120+
* `revert` opcode (which leaves remaining gas untouched) while Solidity
121+
* uses an invalid opcode to revert (consuming all remaining gas).
122+
*
123+
* Requirements:
124+
*
125+
* - The divisor cannot be zero.
126+
*/
127+
function div(uint256 a, uint256 b, string memory errorMessage) internal returns (uint256) {
128+
Lib_SafeExecutionManagerWrapper.safeREQUIRE(b > 0, errorMessage);
129+
uint256 c = a / b;
130+
// assert(a == b * c + a % b); // There is no case in which this doesn't hold
131+
132+
return c;
133+
}
134+
135+
/**
136+
* @dev Returns the remainder of dividing two unsigned integers. (unsigned integer modulo),
137+
* Reverts when dividing by zero.
138+
*
139+
* Counterpart to Solidity's `%` operator. This function uses a `revert`
140+
* opcode (which leaves remaining gas untouched) while Solidity uses an
141+
* invalid opcode to revert (consuming all remaining gas).
142+
*
143+
* Requirements:
144+
*
145+
* - The divisor cannot be zero.
146+
*/
147+
function mod(uint256 a, uint256 b) internal returns (uint256) {
148+
return mod(a, b, "Lib_SafeMathWrapper: modulo by zero");
149+
}
150+
151+
/**
152+
* @dev Returns the remainder of dividing two unsigned integers. (unsigned integer modulo),
153+
* Reverts with custom message when dividing by zero.
154+
*
155+
* Counterpart to Solidity's `%` operator. This function uses a `revert`
156+
* opcode (which leaves remaining gas untouched) while Solidity uses an
157+
* invalid opcode to revert (consuming all remaining gas).
158+
*
159+
* Requirements:
160+
*
161+
* - The divisor cannot be zero.
162+
*/
163+
function mod(uint256 a, uint256 b, string memory errorMessage) internal returns (uint256) {
164+
Lib_SafeExecutionManagerWrapper.safeREQUIRE(b != 0, errorMessage);
165+
return a % b;
166+
}
167+
}

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

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,16 @@ const callPrecompile = async (
1919
Helper_PrecompileCaller: Contract,
2020
precompile: Contract,
2121
functionName: string,
22-
functionParams?: any[]
22+
functionParams?: any[],
23+
gasLimit?: number
2324
): Promise<any> => {
25+
if (gasLimit) {
26+
return Helper_PrecompileCaller.callPrecompile(
27+
precompile.address,
28+
precompile.interface.encodeFunctionData(functionName, functionParams || []),
29+
{gasLimit}
30+
)
31+
}
2432
return Helper_PrecompileCaller.callPrecompile(
2533
precompile.address,
2634
precompile.interface.encodeFunctionData(functionName, functionParams || [])
@@ -177,8 +185,10 @@ describe('OVM_ECDSAContractAccount', () => {
177185
})
178186

179187
it(`should revert on incorrect nonce`, async () => {
180-
const alteredNonceTx = DEFAULT_EIP155_TX
181-
alteredNonceTx.nonce = 99
188+
const alteredNonceTx = {
189+
...DEFAULT_EIP155_TX,
190+
nonce : 99
191+
}
182192
const message = serializeNativeTransaction(alteredNonceTx)
183193
const sig = await signNativeTransaction(wallet, alteredNonceTx)
184194

@@ -227,5 +237,34 @@ describe('OVM_ECDSAContractAccount', () => {
227237
'Transaction chainId does not match expected OVM chainId.'
228238
)
229239
})
240+
241+
it(`should revert on insufficient gas`, async () => {
242+
const alteredInsufficientGasTx = {
243+
...DEFAULT_EIP155_TX,
244+
gasLimit : 200000000
245+
}
246+
const message = serializeNativeTransaction(alteredInsufficientGasTx)
247+
const sig = await signNativeTransaction(wallet, alteredInsufficientGasTx)
248+
249+
await callPrecompile(
250+
Helper_PrecompileCaller,
251+
OVM_ECDSAContractAccount,
252+
'execute',
253+
[
254+
message,
255+
0, //isEthSignedMessage
256+
`0x${sig.v}`, //v
257+
`0x${sig.r}`, //r
258+
`0x${sig.s}`, //s
259+
],
260+
40000000,
261+
)
262+
263+
const ovmREVERT: any =
264+
Mock__OVM_ExecutionManager.smocked.ovmREVERT.calls[0]
265+
expect(ethers.utils.toUtf8String(ovmREVERT._data)).to.equal(
266+
'Gas is not sufficient to execute the transaction.'
267+
)
268+
})
230269
})
231270
})

0 commit comments

Comments
 (0)