Sitelet https://github.com/robertodf99/account/commit/54692a5c2b09217c817822334af0947770879a8d
Skip to content

Commit 54692a5

Browse files
VectorizedVectorized
authored
Fix delegation GuardedExecutor (ithacaxyz#28)
1 parent 9891acb commit 54692a5

6 files changed

Lines changed: 51 additions & 11 deletions

File tree

‎.gitmodules‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
[submodule "lib/forge-std"]
2-
path = lib/forge-std
3-
url = https://github.com/foundry-rs/forge-std
41
[submodule "lib/solady"]
52
path = lib/solady
63
url = https://github.com/vectorized/solady
74
branch = main
5+
[submodule "lib/forge-std"]
6+
path = lib/forge-std
7+
url = https://github.com/foundry-rs/forge-std

‎lib/forge-std‎

‎script/UpgradeDelegation.s.sol‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ contract UpgradeDelegationScript is Script {
1717
uint256 deployerPrivateKey = vm.envUint("PRIVATE_KEY");
1818
// address deployer = vm.createWallet(deployerPrivateKey).addr;
1919
address proxy = 0xF9a8529Bb95ac7707129700f06343338E4767A27;
20-
address newImplementation = 0xa12767Cada47951f79772d673e53a84133537c87;
20+
address newImplementation = 0x9C4F6D8c0d7AEF8BC997cbac908F1c6166Ce4D13;
2121
vm.startBroadcast(deployerPrivateKey);
2222
IEIP7702ProxyWithAdminABI(proxy).upgrade(newImplementation);
2323
vm.stopBroadcast();

‎src/Delegation.sol‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,10 @@ contract Delegation is EIP712, GuardedExecutor {
432432
////////////////////////////////////////////////////////////////////////
433433

434434
/// @dev Pays `paymentAmount` of `paymentToken` to the Entry Point.
435-
function payEntryPoint(address paymentToken, uint256 paymentAmount, address eoa) public virtual {
435+
function payEntryPoint(address paymentToken, uint256 paymentAmount, address eoa)
436+
public
437+
virtual
438+
{
436439
if (msg.sender != ENTRY_POINT) revert Unauthorized();
437440
if (eoa != address(this)) revert Unauthorized();
438441
TokenTransferLib.safeTransfer(paymentToken, msg.sender, paymentAmount);

‎src/GuardedExecutor.sol‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ contract GuardedExecutor is ERC7821 {
5757
/// @dev Exceeded the daily spend limit.
5858
error ExceededSpendLimit();
5959

60+
/// @dev Super admin keys can execute everything.
61+
error SuperAdminCanExecuteEverything();
62+
6063
////////////////////////////////////////////////////////////////////////
6164
// Events
6265
////////////////////////////////////////////////////////////////////////
@@ -267,14 +270,14 @@ contract GuardedExecutor is ERC7821 {
267270
onlyThis
268271
checkKeyHashIsNonZero(keyHash)
269272
{
273+
if (_isSuperAdmin(keyHash)) revert SuperAdminCanExecuteEverything();
274+
270275
// All calls not from the EOA itself has to go through the single `execute` function.
271276
// For security, only EOA key and super admin keys can call into `execute`.
272277
// Otherwise any low-stakes app key can call super admin functions
273278
// such as like `authorize` and `revoke`.
274279
// This check is for sanity. We will still validate this in `canExecute`.
275-
if (_isSelfExecute(target, fnSel)) {
276-
if (!_isSuperAdmin(keyHash)) revert CannotSelfExecute();
277-
}
280+
if (_isSelfExecute(target, fnSel)) revert CannotSelfExecute();
278281

279282
mapping(bytes32 => bool) storage c = _getGuardedExecutorStorage().canExecute;
280283
c[_hash(keyHash, target, fnSel)] = can;
@@ -330,6 +333,9 @@ contract GuardedExecutor is ERC7821 {
330333
// by the EOA's secp256k1 key itself.
331334
if (keyHash == bytes32(0)) return true;
332335

336+
// Super admin keys can execute everything.
337+
if (_isSuperAdmin(keyHash)) return true;
338+
333339
mapping(bytes32 => bool) storage c = _getGuardedExecutorStorage().canExecute;
334340

335341
bytes4 fnSel = ANY_FN_SEL;
@@ -343,7 +349,7 @@ contract GuardedExecutor is ERC7821 {
343349

344350
// This check is required to ensure that authorizing any function selector
345351
// or any target will still NOT allow for self execution.
346-
if (_isSelfExecute(target, fnSel)) if (!_isSuperAdmin(keyHash)) return false;
352+
if (_isSelfExecute(target, fnSel)) return false;
347353

348354
if (c[_hash(keyHash, target, fnSel)]) return true;
349355
if (c[_hash(keyHash, target, ANY_FN_SEL)]) return true;

‎test/EntryPoint.t.sol‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ contract EntryPointTest is SoladyTest {
128128
(uint8 v, bytes32 r, bytes32 s) = vm.sign(alice, digest);
129129

130130
userOp.signature = abi.encodePacked(r, s, v);
131-
131+
132132
bytes4 err = ep.execute(abi.encode(userOp));
133133
assertEq(EntryPoint.PaymentError.selector, err);
134134
}
@@ -200,4 +200,35 @@ contract EntryPointTest is SoladyTest {
200200
calls[0].data = data;
201201
return abi.encode(calls);
202202
}
203+
204+
function testKeySlots() public {
205+
Delegation eoa = Delegation(payable(0xc2de75891512241015C26dA8fe953Aea05985DE3));
206+
vm.etch(address(eoa), delegation.code);
207+
208+
Delegation.Key memory key;
209+
key.expiry = 0;
210+
key.keyType = Delegation.KeyType.Secp256k1;
211+
key.publicKey = abi.encode(address(0x45a2428367e115E9a8B0898dFB194a4Bdcd09a23));
212+
key.isSuperAdmin = true;
213+
214+
vm.prank(address(eoa));
215+
eoa.authorize(key);
216+
vm.stopPrank();
217+
218+
EntryPoint.UserOp memory op;
219+
op.eoa = address(eoa);
220+
op.executionData = _getExecutionData(address(0), 0, bytes(""));
221+
op.nonce = 0x2;
222+
op.paymentToken = address(0x238c8CD93ee9F8c7Edf395548eF60c0d2e46665E);
223+
op.paymentAmount = 0;
224+
op.paymentMaxAmount = 0;
225+
op.combinedGas = 20000000;
226+
bytes32 digest = ep.computeDigest(op);
227+
(uint8 v, bytes32 r, bytes32 s) = vm.sign(
228+
uint256(0x8ef24acf2c7974d38d2f2c4e1bb63515c57c48707df9831794bac28dbe4aa835), digest
229+
);
230+
op.signature = abi.encodePacked(abi.encodePacked(r, s, v), eoa.hash(key), false);
231+
232+
ep.execute(abi.encode(op));
233+
}
203234
}

0 commit comments

Comments
 (0)