Skip to content

Commit 702e7d1

Browse files
kasperpawlowskikasperpawlowski
committed
feat: exclude 0x42... predeploys address space from being a message signer
1 parent dc3be15 commit 702e7d1

File tree

2 files changed

+89
-28
lines changed

2 files changed

+89
-28
lines changed

‎src/EthereumVaultConnector.sol

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ contract EthereumVaultConnector is Events, Errors, TransientStorage, IEVC {
2727
string public constant name = "Ethereum Vault Connector";
2828

2929
uint160 internal constant ACCOUNT_ID_OFFSET = 8;
30+
address internal constant COMMON_PREDEPLOYS = 0x4200000000000000000000000000000000000000;
3031
bytes32 internal constant HASHED_NAME = keccak256(bytes(name));
3132

3233
bytes32 internal constant TYPE_HASH =
@@ -1045,7 +1046,7 @@ contract EthereumVaultConnector is Events, Errors, TransientStorage, IEVC {
10451046
function isSignerValid(address signer) internal pure virtual returns (bool) {
10461047
// not valid if the signer address falls into any of the precompiles/predeploys
10471048
// addresses space (depends on the chain ID).
1048-
return !haveCommonOwnerInternal(signer, address(0));
1049+
return !haveCommonOwnerInternal(signer, address(0)) && !haveCommonOwnerInternal(signer, COMMON_PREDEPLOYS);
10491050
}
10501051

10511052
/// @notice Computes the permit hash for a given set of parameters.

‎test/unit/EthereumVaultConnector/Permit.t.sol

Lines changed: 87 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@ abstract contract EIP712 {
2929
* NOTE: These parameters cannot be changed except through a xref:learn::upgrading-smart-contracts.adoc[smart
3030
* contract upgrade].
3131
*/
32-
constructor(string memory name) {
32+
constructor(
33+
string memory name
34+
) {
3335
_name = name.toShortStringWithFallback(_nameFallback);
3436
_hashedName = keccak256(bytes(name));
3537
}
@@ -60,7 +62,9 @@ abstract contract EIP712 {
6062
* address signer = ECDSA.recover(digest, signature);
6163
* ```
6264
*/
63-
function _hashTypedDataV4(bytes32 structHash) internal view virtual returns (bytes32) {
65+
function _hashTypedDataV4(
66+
bytes32 structHash
67+
) internal view virtual returns (bytes32) {
6468
return ECDSA.toTypedDataHash(_domainSeparatorV4(), structHash);
6569
}
6670
}
@@ -73,11 +77,15 @@ contract SignerECDSA is EIP712, Test {
7377
"Permit(address signer,address sender,uint256 nonceNamespace,uint256 nonce,uint256 deadline,uint256 value,bytes data)"
7478
);
7579

76-
constructor(EthereumVaultConnector _evc) EIP712(_evc.name()) {
80+
constructor(
81+
EthereumVaultConnector _evc
82+
) EIP712(_evc.name()) {
7783
evc = _evc;
7884
}
7985

80-
function setPrivateKey(uint256 _privateKey) external {
86+
function setPrivateKey(
87+
uint256 _privateKey
88+
) external {
8189
privateKey = _privateKey;
8290
}
8391

@@ -111,15 +119,19 @@ contract SignerERC1271 is EIP712, IERC1271 {
111119
"Permit(address signer,address sender,uint256 nonceNamespace,uint256 nonce,uint256 deadline,uint256 value,bytes data)"
112120
);
113121

114-
constructor(EthereumVaultConnector _evc) EIP712(_evc.name()) {
122+
constructor(
123+
EthereumVaultConnector _evc
124+
) EIP712(_evc.name()) {
115125
evc = _evc;
116126
}
117127

118128
function _buildDomainSeparator() internal view override returns (bytes32) {
119129
return keccak256(abi.encode(_TYPE_HASH, _hashedName, block.chainid, address(evc)));
120130
}
121131

122-
function setSignatureHash(bytes calldata signature) external {
132+
function setSignatureHash(
133+
bytes calldata signature
134+
) external {
123135
signatureHash = keccak256(signature);
124136
}
125137

@@ -151,23 +163,31 @@ contract EthereumVaultConnectorWithFallback is EthereumVaultConnectorHarness {
151163
bool internal shouldRevert;
152164
bool public fallbackCalled;
153165

154-
function setExpectedHash(bytes calldata data) external {
166+
function setExpectedHash(
167+
bytes calldata data
168+
) external {
155169
expectedHash = keccak256(data);
156170
}
157171

158-
function setExpectedValue(uint256 value) external {
172+
function setExpectedValue(
173+
uint256 value
174+
) external {
159175
expectedValue = value;
160176
}
161177

162-
function setShouldRevert(bool sr) external {
178+
function setShouldRevert(
179+
bool sr
180+
) external {
163181
shouldRevert = sr;
164182
}
165183

166184
function clearFallbackCalled() external {
167185
fallbackCalled = false;
168186
}
169187

170-
fallback(bytes calldata data) external payable returns (bytes memory) {
188+
fallback(
189+
bytes calldata data
190+
) external payable returns (bytes memory) {
171191
if (shouldRevert) revert("fallback reverted");
172192

173193
if (expectedHash == keccak256(data) && expectedValue == msg.value && address(this) == msg.sender) {
@@ -179,6 +199,7 @@ contract EthereumVaultConnectorWithFallback is EthereumVaultConnectorHarness {
179199
}
180200

181201
contract PermitTest is Test {
202+
address internal constant COMMON_PREDEPLOYS = 0x4200000000000000000000000000000000000000;
182203
EthereumVaultConnectorWithFallback internal evc;
183204
SignerECDSA internal signerECDSA;
184205
SignerERC1271 internal signerERC1271;
@@ -215,7 +236,10 @@ contract PermitTest is Test {
215236
address msgSender = sender == address(0) ? address(uint160(uint256(keccak256(abi.encode(alice))))) : sender;
216237
data = abi.encode(keccak256(data));
217238

218-
vm.assume(!evc.haveCommonOwner(alice, address(0)) && alice != address(evc));
239+
vm.assume(
240+
!evc.haveCommonOwner(alice, address(0)) && !evc.haveCommonOwner(alice, COMMON_PREDEPLOYS)
241+
&& alice != address(evc)
242+
);
219243
vm.assume(msgSender != address(evc));
220244
vm.assume(nonce > 0 && nonce < type(uint256).max);
221245

@@ -264,7 +288,7 @@ contract PermitTest is Test {
264288
data = abi.encode(keccak256(data));
265289

266290
vm.assume(msgSender != address(evc));
267-
vm.assume(!evc.haveCommonOwner(alice, address(0)));
291+
vm.assume(!evc.haveCommonOwner(alice, address(0)) && !evc.haveCommonOwner(alice, COMMON_PREDEPLOYS));
268292
vm.assume(nonce > 0 && nonce < type(uint256).max);
269293

270294
vm.warp(deadline);
@@ -310,7 +334,10 @@ contract PermitTest is Test {
310334
&& privateKey < 115792089237316195423570985008687907852837564279074904382605163141518161494337
311335
);
312336
address alice = vm.addr(privateKey);
313-
vm.assume(!evc.haveCommonOwner(alice, address(0)) && alice != address(evc));
337+
vm.assume(
338+
!evc.haveCommonOwner(alice, address(0)) && !evc.haveCommonOwner(alice, COMMON_PREDEPLOYS)
339+
&& alice != address(evc)
340+
);
314341
bytes19 addressPrefix = evc.getAddressPrefix(alice);
315342
data2 = abi.encode(keccak256(data2));
316343
vm.assume(nonce > 0 && nonce < type(uint256).max - 1);
@@ -351,7 +378,10 @@ contract PermitTest is Test {
351378
&& privateKey < 115792089237316195423570985008687907852837564279074904382605163141518161494337
352379
);
353380
address alice = vm.addr(privateKey);
354-
vm.assume(!evc.haveCommonOwner(alice, address(0)) && alice != address(evc));
381+
vm.assume(
382+
!evc.haveCommonOwner(alice, address(0)) && !evc.haveCommonOwner(alice, COMMON_PREDEPLOYS)
383+
&& alice != address(evc)
384+
);
355385
bytes19 addressPrefix = evc.getAddressPrefix(alice);
356386
data = abi.encode(keccak256(data));
357387
vm.assume(sender != address(0) && sender != address(this));
@@ -369,6 +399,7 @@ contract PermitTest is Test {
369399
}
370400

371401
function test_RevertIfSignerInvalid_Permit(
402+
bool option,
372403
address alice,
373404
uint256 nonceNamespace,
374405
uint256 nonce,
@@ -377,7 +408,9 @@ contract PermitTest is Test {
377408
bytes memory data,
378409
bytes calldata signature
379410
) public {
380-
alice = address(uint160(bound(uint160(alice), 0, 0xFF)));
411+
alice = option
412+
? address(uint160(bound(uint160(alice), 0, 0xFF)))
413+
: address(uint160(bound(uint160(alice), uint160(COMMON_PREDEPLOYS), uint160(COMMON_PREDEPLOYS) + 0xFF)));
381414
bytes19 addressPrefix = evc.getAddressPrefix(alice);
382415
data = abi.encode(keccak256(data));
383416
vm.assume(nonce > 0 && nonce < type(uint256).max);
@@ -404,7 +437,10 @@ contract PermitTest is Test {
404437
) public {
405438
bytes19 addressPrefix = evc.getAddressPrefix(alice);
406439
data = abi.encode(keccak256(data));
407-
vm.assume(!evc.haveCommonOwner(alice, address(0)) && alice != address(evc));
440+
vm.assume(
441+
!evc.haveCommonOwner(alice, address(0)) && !evc.haveCommonOwner(alice, COMMON_PREDEPLOYS)
442+
&& alice != address(evc)
443+
);
408444
vm.assume(nonce < type(uint256).max);
409445
vm.warp(deadline);
410446

@@ -436,7 +472,10 @@ contract PermitTest is Test {
436472
) public {
437473
bytes19 addressPrefix = evc.getAddressPrefix(alice);
438474
data = abi.encode(keccak256(data));
439-
vm.assume(!evc.haveCommonOwner(alice, address(0)) && alice != address(evc));
475+
vm.assume(
476+
!evc.haveCommonOwner(alice, address(0)) && !evc.haveCommonOwner(alice, COMMON_PREDEPLOYS)
477+
&& alice != address(evc)
478+
);
440479
vm.assume(nonce > 0 && nonce < type(uint256).max);
441480
vm.assume(deadline < type(uint256).max);
442481
vm.warp(deadline + 1);
@@ -466,7 +505,10 @@ contract PermitTest is Test {
466505
address alice = vm.addr(privateKey);
467506
bytes19 addressPrefix = evc.getAddressPrefix(alice);
468507
data = abi.encode(keccak256(data));
469-
vm.assume(!evc.haveCommonOwner(alice, address(0)) && alice != address(evc));
508+
vm.assume(
509+
!evc.haveCommonOwner(alice, address(0)) && !evc.haveCommonOwner(alice, COMMON_PREDEPLOYS)
510+
&& alice != address(evc)
511+
);
470512
vm.assume(nonce > 0 && nonce < type(uint256).max);
471513
vm.assume(value > 0);
472514
vm.warp(deadline);
@@ -499,7 +541,10 @@ contract PermitTest is Test {
499541
bytes calldata signature
500542
) public {
501543
bytes19 addressPrefix = evc.getAddressPrefix(alice);
502-
vm.assume(!evc.haveCommonOwner(alice, address(0)) && alice != address(evc));
544+
vm.assume(
545+
!evc.haveCommonOwner(alice, address(0)) && !evc.haveCommonOwner(alice, COMMON_PREDEPLOYS)
546+
&& alice != address(evc)
547+
);
503548
vm.assume(nonce > 0 && nonce < type(uint256).max);
504549
vm.warp(deadline);
505550

@@ -531,7 +576,10 @@ contract PermitTest is Test {
531576
data = abi.encode(keccak256(data));
532577
signerECDSA.setPrivateKey(privateKey);
533578

534-
vm.assume(!evc.haveCommonOwner(alice, address(0)) && alice != address(evc));
579+
vm.assume(
580+
!evc.haveCommonOwner(alice, address(0)) && !evc.haveCommonOwner(alice, COMMON_PREDEPLOYS)
581+
&& alice != address(evc)
582+
);
535583
vm.assume(nonce > 0 && nonce < type(uint256).max);
536584
vm.warp(deadline);
537585
vm.deal(address(evc), value);
@@ -569,7 +617,10 @@ contract PermitTest is Test {
569617
bytes calldata signature,
570618
uint16 value
571619
) public {
572-
vm.assume(!evc.haveCommonOwner(signer, address(0)) && signer != address(evc));
620+
vm.assume(
621+
!evc.haveCommonOwner(signer, address(0)) && !evc.haveCommonOwner(signer, COMMON_PREDEPLOYS)
622+
&& signer != address(evc)
623+
);
573624
vm.assume(nonce > 0 && nonce < type(uint256).max);
574625

575626
bytes19 addressPrefix = evc.getAddressPrefix(signer);
@@ -597,7 +648,7 @@ contract PermitTest is Test {
597648
address alice = vm.addr(privateKey);
598649
signerECDSA.setPrivateKey(privateKey);
599650

600-
vm.assume(!evc.haveCommonOwner(alice, address(0)));
651+
vm.assume(!evc.haveCommonOwner(alice, address(0)) && !evc.haveCommonOwner(alice, COMMON_PREDEPLOYS));
601652
vm.warp(deadline);
602653

603654
// ECDSA signature invalid due to signer.
@@ -695,7 +746,7 @@ contract PermitTest is Test {
695746
address alice = address(new SignerERC1271(evc));
696747
SignerERC1271(alice).setSignatureHash(signature);
697748

698-
vm.assume(!evc.haveCommonOwner(alice, address(0)));
749+
vm.assume(!evc.haveCommonOwner(alice, address(0)) && !evc.haveCommonOwner(alice, COMMON_PREDEPLOYS));
699750
vm.warp(deadline);
700751

701752
// ECDSA signature is always invalid here hence we fall back to ERC-1271 signature
@@ -776,7 +827,9 @@ contract PermitTest is Test {
776827
assertTrue(evc.fallbackCalled());
777828
}
778829

779-
function test_Permit(uint256 privateKey) public {
830+
function test_Permit(
831+
uint256 privateKey
832+
) public {
780833
vm.assume(
781834
privateKey > 0
782835
&& privateKey < 115792089237316195423570985008687907852837564279074904382605163141518161494337
@@ -785,7 +838,10 @@ contract PermitTest is Test {
785838
address bob = address(new SignerERC1271(evc));
786839
address target = address(new Vault(evc));
787840

788-
vm.assume(!evc.haveCommonOwner(alice, address(0)) && !evc.haveCommonOwner(alice, bob));
841+
vm.assume(
842+
!evc.haveCommonOwner(alice, address(0)) && !evc.haveCommonOwner(alice, bob)
843+
&& !evc.haveCommonOwner(alice, COMMON_PREDEPLOYS)
844+
);
789845
vm.deal(address(this), type(uint128).max);
790846
signerECDSA.setPrivateKey(privateKey);
791847

@@ -1273,7 +1329,9 @@ contract PermitTest is Test {
12731329
assertEq(evc.isAccountOperatorAuthorized(bob, operator), false);
12741330
}
12751331

1276-
function test_RevertIfInPermit_SetLockdownMode(uint256 privateKey) public {
1332+
function test_RevertIfInPermit_SetLockdownMode(
1333+
uint256 privateKey
1334+
) public {
12771335
vm.assume(
12781336
privateKey > 0
12791337
&& privateKey < 115792089237316195423570985008687907852837564279074904382605163141518161494337
@@ -1300,7 +1358,9 @@ contract PermitTest is Test {
13001358
evc.permit(alice, address(this), 0, 1, 1, 0, data, signature);
13011359
}
13021360

1303-
function test_RevertIfInPermit_SetPermitDisabledMode(uint256 privateKey) public {
1361+
function test_RevertIfInPermit_SetPermitDisabledMode(
1362+
uint256 privateKey
1363+
) public {
13041364
vm.assume(
13051365
privateKey > 0
13061366
&& privateKey < 115792089237316195423570985008687907852837564279074904382605163141518161494337

0 commit comments

Comments
 (0)