diff --git a/src/common/Structs.sol b/src/common/Structs.sol index f2986627..ab957155 100644 --- a/src/common/Structs.sol +++ b/src/common/Structs.sol @@ -1,7 +1,7 @@ pragma solidity ^0.8.0; import {IKernelValidator} from "../interfaces/IKernelValidator.sol"; -import {ParamCondition} from "./Enums.sol"; +import {ParamCondition, Operation} from "./Enums.sol"; import {ValidAfter, ValidUntil} from "./Types.sol"; // Defining a struct for execution details @@ -57,6 +57,7 @@ struct Permission { uint256 valueLimit; ParamRule[] rules; ExecutionRule executionRule; + Operation operation; } struct SessionData { diff --git a/src/interfaces/IKernel.sol b/src/interfaces/IKernel.sol index d0029e6e..ec0b98d6 100644 --- a/src/interfaces/IKernel.sol +++ b/src/interfaces/IKernel.sol @@ -80,6 +80,8 @@ interface IKernel { function executeBatch(Call[] memory calls) external payable; + function executeDelegateCall(address to, bytes memory data) external payable; + /// @notice Validates a user operation based on its mode /// @dev This function will validate user operation and be called by EntryPoint /// @param userOp The user operation to be validated diff --git a/src/validator/SessionKeyValidator.sol b/src/validator/SessionKeyValidator.sol index db2d3e02..8021a452 100644 --- a/src/validator/SessionKeyValidator.sol +++ b/src/validator/SessionKeyValidator.sol @@ -7,7 +7,7 @@ import {IKernelValidator} from "../interfaces/IKernelValidator.sol"; import {UserOperation} from "I4337/interfaces/UserOperation.sol"; import {SIG_VALIDATION_FAILED} from "../common/Constants.sol"; -import {ParamCondition} from "../common/Enums.sol"; +import {ParamCondition, Operation} from "../common/Enums.sol"; import {ParamRule, SessionData, Permission, Call, ExecutionRule, ExecutionStatus, Nonces} from "../common/Structs.sol"; import "../common/Types.sol"; @@ -154,29 +154,13 @@ contract SessionKeyValidator is IKernelValidator { } bytes calldata callData = userOp.callData; - if (bytes4(callData[0:4]) == Kernel.execute.selector) { + if ( + bytes4(callData[0:4]) == Kernel.execute.selector + || bytes4(callData[0:4]) == Kernel.executeDelegateCall.selector + ) { (Permission calldata permission, bytes32[] calldata merkleProof) = _getPermission(userOp.signature[85:]); - require( - address(bytes20(userOp.callData[16:36])) == permission.target, "SessionKeyValidator: target mismatch" - ); - require( - uint256(bytes32(userOp.callData[36:68])) <= permission.valueLimit, - "SessionKeyValidator: value limit exceeded" - ); - bytes calldata data; - assembly { - let dataOffset := add(add(callData.offset, 0x04), calldataload(add(callData.offset, 0x44))) - let length := calldataload(dataOffset) - data.offset := add(dataOffset, 32) - data.length := length - } - require(verifyPermission(data, permission), "SessionKeyValidator: permission verification failed"); - ValidAfter validAfter = - _updateValidAfter(permission, keccak256(abi.encodePacked(session.nonce, permission.index))); - if (ValidAfter.unwrap(validAfter) < ValidAfter.unwrap(session.validAfter)) { - validAfter = session.validAfter; - } - if (!MerkleProofLib.verify(merkleProof, session.merkleRoot, keccak256(abi.encode(permission)))) { + (ValidAfter validAfter, bool verifyFailed) = _verifyParam(sessionKey, callData, permission, merkleProof); + if (verifyFailed) { return SIG_VALIDATION_FAILED; } return _verifyUserOpHash(sessionKey, validAfter, session.validUntil); @@ -240,7 +224,9 @@ contract SessionKeyValidator is IKernelValidator { for (i = 0; i < calls.length; i++) { Call calldata call = calls[i]; Permission calldata permission = _permissions[i]; - require(call.to == permission.target, "SessionKeyValidator: target mismatch"); + require( + permission.target == address(0) || call.to == permission.target, "SessionKeyValidator: target mismatch" + ); require(uint256(bytes32(call.value)) <= permission.valueLimit, "SessionKeyValidator: value limit exceeded"); require(verifyPermission(call.data, permission), "SessionKeyValidator: permission verification failed"); ValidAfter validAfter = @@ -254,6 +240,43 @@ contract SessionKeyValidator is IKernelValidator { } } + function _verifyParam( + address sessionKey, + bytes calldata callData, + Permission calldata _permission, + bytes32[] calldata _merkleProof + ) internal returns (ValidAfter validAfter, bool verifyFailed) { + SessionData storage session = sessionData[sessionKey][msg.sender]; + bool isExecute = bytes4(callData[0:4]) == Kernel.execute.selector; + require( + _permission.target == address(0) || address(bytes20(callData[16:36])) == _permission.target, + "SessionKeyValidator: target mismatch" + ); + if (isExecute) { + require( + uint256(bytes32(callData[36:68])) <= _permission.valueLimit, "SessionKeyValidator: value limit exceeded" + ); + } else { + require(_permission.operation == Operation.DelegateCall, "SessionKeyValidator: operation mismatch"); + } + bytes calldata data; + uint8 dataParamOffset = isExecute ? 0x44 : 0x24; + assembly { + let dataOffset := add(add(callData.offset, 0x04), calldataload(add(callData.offset, dataParamOffset))) + let length := calldataload(dataOffset) + data.offset := add(dataOffset, 32) + data.length := length + } + require(verifyPermission(data, _permission), "SessionKeyValidator: permission verification failed"); + validAfter = _updateValidAfter(_permission, keccak256(abi.encodePacked(session.nonce, _permission.index))); + if (ValidAfter.unwrap(validAfter) < ValidAfter.unwrap(session.validAfter)) { + validAfter = session.validAfter; + } + if (!MerkleProofLib.verify(_merkleProof, session.merkleRoot, keccak256(abi.encode(_permission)))) { + return (validAfter, true); + } + } + function verifyPermission(bytes calldata data, Permission calldata permission) internal pure returns (bool) { if (bytes4(data[0:4]) != permission.sig) return false; for (uint256 i = 0; i < permission.rules.length; i++) { diff --git a/test/foundry/mock/TestCallee.sol b/test/foundry/mock/TestCallee.sol index 2dff256d..05b26543 100644 --- a/test/foundry/mock/TestCallee.sol +++ b/test/foundry/mock/TestCallee.sol @@ -9,5 +9,12 @@ contract TestCallee { result = a + b + msg.value; } + function transferErc20Tester(address token, address to, uint256 amount) external { + (bool success, bytes memory data) = token.call( + abi.encodeWithSignature("transfer(address,uint256)", to, amount) + ); + require(success, string(data)); + } + function notThis() external {} } diff --git a/test/foundry/validator/SessionKeyValidator.t.sol b/test/foundry/validator/SessionKeyValidator.t.sol index 3c7927c5..84de33ed 100644 --- a/test/foundry/validator/SessionKeyValidator.t.sol +++ b/test/foundry/validator/SessionKeyValidator.t.sol @@ -21,18 +21,21 @@ import "src/validator/SessionKeyValidator.sol"; import {KernelECDSATest} from "../KernelECDSA.t.sol"; import "../mock/TestCallee.sol"; +import "../mock/TestERC20.sol"; using ERC4337Utils for IEntryPoint; contract SessionKeyValidatorTest is KernelECDSATest { SessionKeyValidator sessionKeyValidator; TestCallee[] callees; + TestERC20[] erc20s; ExecutionRule execRule; bytes32[] data; address sessionKey; uint256 sessionKeyPriv; TestPaymaster paymaster; TestPaymaster unknownPaymaster; + address recipient; function setUp() public override { super.setUp(); @@ -42,24 +45,47 @@ contract SessionKeyValidatorTest is KernelECDSATest { unknownPaymaster = new TestPaymaster(); entryPoint.depositTo{value: 1e18}(address(unknownPaymaster)); entryPoint.depositTo{value: 1e18}(address(paymaster)); + recipient = makeAddr("recipient"); } - function _setup_permission(uint256 _length) internal returns (Permission[] memory permissions) { + function _setup_permission(uint256 _length, bool isDelegateCall) + internal + returns (Permission[] memory permissions) + { permissions = new Permission[](_length); callees = new TestCallee[](_length); + if (isDelegateCall) { + erc20s = new TestERC20[](_length); + } for (uint8 i = 0; i < _length; i++) { - callees[i] = new TestCallee(); + address target; + bytes4 sig; + if (isDelegateCall) { + erc20s[i] = new TestERC20(); + target = address(callees[i]); + sig = TestCallee.transferErc20Tester.selector; + erc20s[i].mint(address(kernel), 200); + } else { + callees[i] = new TestCallee(); + target = address(callees[i]); + sig = TestCallee.addTester.selector; + } ParamRule[] memory paramRules = new ParamRule[](2); - paramRules[0] = ParamRule({offset: 0, condition: ParamCondition(i % 6), param: bytes32(uint256(100))}); + if (isDelegateCall) { + paramRules[0] = ParamRule({offset: 0, condition: ParamCondition(0), param: bytes32(uint256(uint160(recipient)))}); + } else { + paramRules[0] = ParamRule({offset: 0, condition: ParamCondition(i % 6), param: bytes32(uint256(100))}); + } paramRules[1] = ParamRule({offset: 32, condition: ParamCondition((i + 1) % 6), param: bytes32(uint256(100))}); permissions[i] = Permission({ index: i, - target: address(callees[i]), - sig: TestCallee.addTester.selector, + target: target, + sig: sig, valueLimit: 0, rules: paramRules, - executionRule: execRule + executionRule: execRule, + operation: isDelegateCall ? Operation.DelegateCall : Operation.Call }); } } @@ -154,20 +180,32 @@ contract SessionKeyValidatorTest is KernelECDSATest { uint256 indexToUse, uint8 usingPaymasterMode, bool param1Faulty, - bool param2Faulty + bool param2Faulty, + bool isDelegateCall ) internal view returns (UserOperation memory op) { + bytes4 selector = isDelegateCall ? IKernel.executeDelegateCall.selector : IKernel.execute.selector; op = buildUserOperation( - abi.encodeWithSelector( - IKernel.execute.selector, - permissions[indexToUse].target, - 0, - abi.encodeWithSelector( - permissions[indexToUse].sig, - _generateParam(ParamCondition(indexToUse % 6), !param1Faulty), // since EQ - _generateParam(ParamCondition((indexToUse + 1) % 6), !param2Faulty) // since NOT_EQ - ), - Operation.Call - ) + isDelegateCall + ? abi.encodeWithSelector( + selector, + permissions[indexToUse].target, + abi.encodeWithSelector( + permissions[indexToUse].sig, + recipient, + _generateParam(ParamCondition((indexToUse + 1) % 6), !param2Faulty) // since NOT_EQ + ) + ) + : abi.encodeWithSelector( + selector, + permissions[indexToUse].target, + 0, + abi.encodeWithSelector( + permissions[indexToUse].sig, + _generateParam(ParamCondition(indexToUse % 6), !param1Faulty), // since EQ + _generateParam(ParamCondition((indexToUse + 1) % 6), !param2Faulty) // since NOT_EQ + ), + Operation.Call + ) ); if (usingPaymasterMode != 0) { // 0 = no paymaster @@ -186,7 +224,7 @@ contract SessionKeyValidatorTest is KernelECDSATest { sessionData.nonce ); bytes32 digest = getTypedDataHash( - IKernel.execute.selector, + selector, ValidAfter.unwrap(sessionData.validAfter), ValidUntil.unwrap(sessionData.validUntil), address(sessionKeyValidator), @@ -233,6 +271,7 @@ contract SessionKeyValidatorTest is KernelECDSATest { bool param1Faulty; bool param2Faulty; bool wrongProof; + bool isDelegateCall; } struct BatchTestConfig { @@ -247,6 +286,7 @@ contract SessionKeyValidatorTest is KernelECDSATest { config.runs = 0; config.interval = 0; config.validAfter = 0; // TODO: runs not checked with batch + vm.assume(!config.isDelegateCall); vm.assume(config.indexToUse < config.numberOfPermissions && config.numberOfPermissions > 1); vm.assume( config.validAfter < type(uint32).max && config.interval < type(uint32).max && config.runs < type(uint32).max @@ -271,7 +311,7 @@ contract SessionKeyValidatorTest is KernelECDSATest { validAfter: ValidAfter.wrap(config.validAfter), interval: config.interval }); - Permission[] memory permissions = _setup_permission(config.numberOfPermissions); + Permission[] memory permissions = _setup_permission(config.numberOfPermissions, false); _buildHashes(permissions); (uint128 lastNonce,) = sessionKeyValidator.nonces(address(kernel)); SessionData memory sessionData = SessionData({ @@ -365,7 +405,7 @@ contract SessionKeyValidatorTest is KernelECDSATest { config.paymasterMode = config.paymasterMode % 3; config.usingPaymasterMode = config.usingPaymasterMode % 3; bool shouldFail = (config.usingPaymasterMode < config.paymasterMode) || (1000 < config.validAfter) - || config.faultySig || config.param1Faulty || config.param2Faulty || config.wrongProof; + || config.faultySig || (config.param1Faulty && !config.isDelegateCall) || config.param2Faulty || config.wrongProof; config.runs = config.runs % 10; config.earlyRun = config.runs == 0 ? 0 : config.earlyRun % config.runs; if (config.interval == 0 || config.validAfter == 0) { @@ -382,7 +422,7 @@ contract SessionKeyValidatorTest is KernelECDSATest { validAfter: ValidAfter.wrap(config.validAfter), interval: config.interval }); - Permission[] memory permissions = _setup_permission(config.numberOfPermissions); + Permission[] memory permissions = _setup_permission(config.numberOfPermissions, config.isDelegateCall); _buildHashes(permissions); (uint128 lastNonce,) = sessionKeyValidator.nonces(address(kernel)); SessionData memory sessionData = SessionData({ @@ -399,7 +439,8 @@ contract SessionKeyValidatorTest is KernelECDSATest { config.indexToUse, config.usingPaymasterMode, config.param1Faulty, - config.param2Faulty + config.param2Faulty, + config.isDelegateCall ); op.signature = bytes.concat( op.signature,