Skip to content

session key validator fixed for batch scenario #45

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
leekt merged 2 commits into from
Nov 14, 2023
Merged

session key validator fixed for batch scenario #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2ffca9b
session key validator fixed for batch scenario
leekt Nov 13, 2023
ab70399
test: fuzz testing for batched options include array
leekt Nov 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions src/Kernel.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ contract Kernel is EIP712, Compatibility, KernelStorage {
default { return(0, returndatasize()) }
}
}

/// @notice Executes a function call to an external contract with delegatecall
/// @param to The address of the target contract
/// @param data The call data to be sent
Expand Down Expand Up @@ -251,7 +251,7 @@ contract Kernel is EIP712, Compatibility, KernelStorage {
}
}

function validateSignature(bytes32 hash, bytes calldata signature) public view returns(ValidationData) {
function validateSignature(bytes32 hash, bytes calldata signature) public view returns (ValidationData) {
return _validateSignature(hash, signature);
}

Expand All @@ -265,7 +265,8 @@ contract Kernel is EIP712, Compatibility, KernelStorage {
address proxyAddress = address(this);

// Construct the domain separator with name, version, chainId, and proxy address.
bytes32 typeHash = keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)");
bytes32 typeHash =
keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)");
return keccak256(abi.encode(typeHash, nameHash, versionHash, block.chainid, proxyAddress));
}

Expand All @@ -285,9 +286,10 @@ contract Kernel is EIP712, Compatibility, KernelStorage {
(ValidAfter validAfter, ValidUntil validUntil, address result) = parseValidationData(validationData);

// Check if the signature is valid within the specified time frame and the result is successful
if (ValidAfter.unwrap(validAfter) <= block.timestamp &&
ValidUntil.unwrap(validUntil) >= block.timestamp &&
result == address(0)) {
if (
ValidAfter.unwrap(validAfter) <= block.timestamp && ValidUntil.unwrap(validUntil) >= block.timestamp
&& result == address(0)
) {
// If all checks pass, return the ERC1271 magic value for a valid signature
return 0x1626ba7e;
} else {
Expand Down
47 changes: 19 additions & 28 deletions src/validator/SessionKeyValidator.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,9 @@ contract SessionKeyValidator is IKernelValidator {
}

function invalidateNonce(uint128 nonce) public {
require(
nonce > nonces[msg.sender].invalidNonce && nonce <= nonces[msg.sender].lastNonce,
"SessionKeyValidator: invalid nonce"
);
require(nonce > nonces[msg.sender].invalidNonce, "SessionKeyValidator: invalid nonce");
nonces[msg.sender].invalidNonce = nonce;
nonces[msg.sender].lastNonce = nonce;
}

function disable(bytes calldata _data) external payable {
Expand Down Expand Up @@ -113,7 +111,14 @@ contract SessionKeyValidator is IKernelValidator {
function _getPermissions(bytes calldata _sig) internal pure returns (Permission[] calldata permissions) {
assembly {
permissions.offset := add(add(_sig.offset, 0x20), calldataload(_sig.offset))
permissions.length := calldataload(permissions.offset)
permissions.length := calldataload(add(_sig.offset, calldataload(_sig.offset)))
}
}

function _getProofs(bytes calldata _sig) internal pure returns (bytes32[][] calldata proofs) {
assembly {
proofs.length := calldataload(add(_sig.offset, calldataload(add(_sig.offset, 0x20))))
proofs.offset := add(add(_sig.offset, 0x20), calldataload(add(_sig.offset, 0x20)))
}
}

Expand Down Expand Up @@ -152,8 +157,7 @@ contract SessionKeyValidator is IKernelValidator {
if (bytes4(callData[0:4]) == Kernel.execute.selector) {
(Permission calldata permission, bytes32[] calldata merkleProof) = _getPermission(userOp.signature[85:]);
require(
address(bytes20(userOp.callData[16:36])) == permission.target,
"SessionKeyValidator: target mismatch"
address(bytes20(userOp.callData[16:36])) == permission.target, "SessionKeyValidator: target mismatch"
);
require(
uint256(bytes32(userOp.callData[36:68])) <= permission.valueLimit,
Expand All @@ -178,10 +182,9 @@ contract SessionKeyValidator is IKernelValidator {
return _verifyUserOpHash(sessionKey, validAfter, session.validUntil);
} else if (bytes4(callData[0:4]) == Kernel.executeBatch.selector) {
Permission[] calldata permissions = _getPermissions(userOp.signature[85:]);
(, bytes32[] memory merkleProof, bool[] memory flags, uint256[] memory index) =
abi.decode(userOp.signature[85:], (Permission[], bytes32[], bool[], uint256[]));
(bytes32[] memory leaves, ValidAfter validAfter) = _verifyParams(sessionKey, callData, permissions, index);
if (!MerkleProofLib.verifyMultiProof(merkleProof, session.merkleRoot, leaves, flags)) {
bytes32[][] calldata merkleProof = _getProofs(userOp.signature[85:]);
(ValidAfter validAfter, bool verifyFailed) = _verifyParams(sessionKey, callData, permissions, merkleProof);
if (verifyFailed) {
return SIG_VALIDATION_FAILED;
}
return _verifyUserOpHash(sessionKey, validAfter, session.validUntil);
Expand Down Expand Up @@ -224,43 +227,31 @@ contract SessionKeyValidator is IKernelValidator {
address sessionKey,
bytes calldata callData,
Permission[] calldata _permissions,
uint256[] memory index
) internal returns (bytes32[] memory leaves, ValidAfter maxValidAfter) {
bytes32[][] calldata _merkleProof
) internal returns (ValidAfter maxValidAfter, bool verifyFailed) {
Call[] calldata calls;
assembly {
calls.offset := add(add(callData.offset, 0x24), calldataload(add(callData.offset, 4)))
calls.length := calldataload(add(add(callData.offset, 4), calldataload(add(callData.offset, 4))))
}
uint256 i = 0;
leaves = _generateLeaves(index);
SessionData storage session = sessionData[sessionKey][msg.sender];
maxValidAfter = session.validAfter;
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(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 =
_updateValidAfter(permission, keccak256(abi.encodePacked(session.nonce, permission.index)));
if (ValidAfter.unwrap(validAfter) > ValidAfter.unwrap(maxValidAfter)) {
maxValidAfter = validAfter;
}
leaves[index[i]] = keccak256(abi.encode(permission));
}
}

function _generateLeaves(uint256[] memory _indexes) internal pure returns (bytes32[] memory leaves) {
uint256 maxIndex;
uint256 i = 0;
for (i = 0; i < _indexes.length; i++) {
if (_indexes[i] > maxIndex) {
maxIndex = _indexes[i];
if (!MerkleProofLib.verify(_merkleProof[i], session.merkleRoot, keccak256(abi.encode(permission)))) {
return (maxValidAfter, true);
}
}
leaves = new bytes32[](maxIndex + 1);
}

function verifyPermission(bytes calldata data, Permission calldata permission) internal pure returns (bool) {
Expand Down
4 changes: 3 additions & 1 deletion src/validator/WeightedECDSAValidator.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,9 @@ contract WeightedECDSAValidator is EIP712, IKernelValidator {
for (uint256 i = 0; i < sigCount; i++) {
// last sig is for userOpHash verification
signer = ECDSA.recover(
_hashTypedData(keccak256(abi.encode(keccak256("Approve(bytes32 callDataAndNonceHash)"), callDataAndNonceHash))),
_hashTypedData(
keccak256(abi.encode(keccak256("Approve(bytes32 callDataAndNonceHash)"), callDataAndNonceHash))
),
sig[i * 65:(i + 1) * 65]
);
vote = voteStatus[callDataAndNonceHash][signer][msg.sender];
Expand Down
3 changes: 1 addition & 2 deletions test/foundry/KernelLiteECDSA.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,7 @@ contract KernelECDSATest is KernelTestBase {
function test_transfer_ownership() external {
address newOwner = makeAddr("new owner");
UserOperation memory op = entryPoint.fillUserOp(
address(kernel),
abi.encodeWithSelector(KernelLiteECDSA.transferOwnership.selector, newOwner)
address(kernel), abi.encodeWithSelector(KernelLiteECDSA.transferOwnership.selector, newOwner)
);
op.signature = signUserOp(op);
UserOperation[] memory ops = new UserOperation[](1);
Expand Down
15 changes: 12 additions & 3 deletions test/foundry/KernelTestBase.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,15 @@ abstract contract KernelTestBase is Test {
kernel.execute(validCallers[i], 0, "", Operation.Call);
}
}

function test_external_call_execute_delegatecall_success() external {
address[] memory validCallers = getOwners();
for (uint256 i = 0; i < validCallers.length; i++) {
vm.prank(validCallers[i]);
kernel.executeDelegateCall(validCallers[i], "");
}
}

function test_external_call_execute_delegatecall_fail() external {
address[] memory validCallers = getOwners();
for (uint256 i = 0; i < validCallers.length; i++) {
Expand Down Expand Up @@ -196,15 +197,23 @@ abstract contract KernelTestBase is Test {
function test_validate_signature() external {
Kernel kernel2 = Kernel(payable(factory.createAccount(address(kernelImpl), getInitializeData(), 3)));
bytes32 hash = keccak256(abi.encodePacked("hello world"));
bytes32 digest = keccak256(abi.encodePacked("\x19\x01", ERC4337Utils._buildDomainSeparator(KERNEL_NAME, KERNEL_VERSION, address(kernel)), hash));
bytes32 digest = keccak256(
abi.encodePacked(
"\x19\x01", ERC4337Utils._buildDomainSeparator(KERNEL_NAME, KERNEL_VERSION, address(kernel)), hash
)
);
bytes memory sig = signHash(digest);
assertEq(kernel.isValidSignature(hash, sig), Kernel.isValidSignature.selector);
assertEq(kernel2.isValidSignature(hash, sig), bytes4(0xffffffff));
}

function test_fail_validate_wrongsignature() external {
bytes32 hash = keccak256(abi.encodePacked("hello world"));
bytes32 digest = keccak256(abi.encodePacked("\x19\x01", ERC4337Utils._buildDomainSeparator(KERNEL_NAME, KERNEL_VERSION, address(kernel)), hash));
bytes32 digest = keccak256(
abi.encodePacked(
"\x19\x01", ERC4337Utils._buildDomainSeparator(KERNEL_NAME, KERNEL_VERSION, address(kernel)), hash
)
);
bytes memory sig = getWrongSignature(hash);
assertEq(kernel.isValidSignature(hash, sig), bytes4(0xffffffff));
}
Expand Down
5 changes: 4 additions & 1 deletion test/foundry/utils/Merkle.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ function _getRoot(bytes32[] memory data) pure returns (bytes32) {
return data[0];
}

function _getProof(bytes32[] memory data, uint256 nodeIndex) pure returns (bytes32[] memory) {
function _getProof(bytes32[] memory data, uint256 nodeIndex, bool wrongProof) pure returns (bytes32[] memory) {
require(data.length > 1);

bytes32[] memory result = new bytes32[](64);
Expand All @@ -33,6 +33,9 @@ function _getProof(bytes32[] memory data, uint256 nodeIndex) pure returns (bytes
assembly {
mstore(result, pos)
}
if (wrongProof) {
result[0] = result[0] ^ bytes32(uint256(0x01));
}

return result;
}
Expand Down
Loading