From 8681504e8932a5dac601886dd5fedada283ae63a Mon Sep 17 00:00:00 2001 From: highskore Date: Wed, 15 Oct 2025 16:46:22 +0100 Subject: [PATCH 1/2] fix(1271): parse policydata from sig --- contracts/SmartSession.sol | 65 +++++++++++++++----------------- test/unit/base/ERC1271Base.t.sol | 24 +++++------- 2 files changed, 40 insertions(+), 49 deletions(-) diff --git a/contracts/SmartSession.sol b/contracts/SmartSession.sol index 2db15a3..9d811a1 100644 --- a/contracts/SmartSession.sol +++ b/contracts/SmartSession.sol @@ -66,10 +66,7 @@ contract SmartSession is ISmartSession, SmartSessionBase, SmartSessionERC7739 { * @param userOpHash The hash of the user operation * @return vd ValidationData containing the validation result */ - function validateUserOp( - PackedUserOperation calldata userOp, - bytes32 userOpHash - ) + function validateUserOp(PackedUserOperation calldata userOp, bytes32 userOpHash) external override returns (ValidationData vd) @@ -166,8 +163,7 @@ contract SmartSession is ISmartSession, SmartSessionBase, SmartSessionERC7739 { // Enable ERC1271 policies $enabledERC7739.enable({ - contexts: enableData.sessionToEnable.erc7739Policies.allowedERC7739Content, - permissionId: permissionId + contexts: enableData.sessionToEnable.erc7739Policies.allowedERC7739Content, permissionId: permissionId }); // Enabel ERC1271 policies @@ -181,9 +177,7 @@ contract SmartSession is ISmartSession, SmartSessionBase, SmartSessionERC7739 { // Enable action policies $actionPolicies.enable({ - permissionId: permissionId, - actionPolicyDatas: enableData.sessionToEnable.actions, - useRegistry: false + permissionId: permissionId, actionPolicyDatas: enableData.sessionToEnable.actions, useRegistry: false }); _setPermit4337Paymaster(permissionId, enableData.sessionToEnable.permitERC4337Paymaster); @@ -279,11 +273,12 @@ contract SmartSession is ISmartSession, SmartSessionBase, SmartSessionERC7739 { // DEFAULT EXEC & BATCH CALL else if (callType == CALLTYPE_BATCH) { vd = vd.intersect( - $actionPolicies.actionPolicies.checkBatch7579Exec({ - userOp: userOp, - permissionId: permissionId, - minPolicies: 1 // minimum of one actionPolicy must be set. - }) + $actionPolicies.actionPolicies + .checkBatch7579Exec({ + userOp: userOp, + permissionId: permissionId, + minPolicies: 1 // minimum of one actionPolicy must be set. + }) ); } // DEFAULT EXEC & SINGLE CALL @@ -291,13 +286,14 @@ contract SmartSession is ISmartSession, SmartSessionBase, SmartSessionERC7739 { (address target, uint256 value, bytes calldata callData) = userOp.callData.decodeUserOpCallData().decodeSingle(); vd = vd.intersect( - $actionPolicies.actionPolicies.checkSingle7579Exec({ - permissionId: permissionId, - target: target, - value: value, - callData: callData, - minPolicies: 1 // minimum of one actionPolicy must be set. - }) + $actionPolicies.actionPolicies + .checkSingle7579Exec({ + permissionId: permissionId, + target: target, + value: value, + callData: callData, + minPolicies: 1 // minimum of one actionPolicy must be set. + }) ); } // DelegateCalls are not supported by SmartSession @@ -319,7 +315,8 @@ contract SmartSession is ISmartSession, SmartSessionBase, SmartSessionERC7739 { ActionId actionId = account.toActionId(bytes4(userOp.callData[:4])); vd = vd.intersect( - $actionPolicies.actionPolicies[actionId].check({ + $actionPolicies.actionPolicies[actionId] + .check({ permissionId: permissionId, callOnIPolicy: abi.encodeCall( IActionPolicy.checkAction, @@ -332,7 +329,7 @@ contract SmartSession is ISmartSession, SmartSessionBase, SmartSessionERC7739 { ) ), minPolicies: 1 // minimum of one actionPolicy must be set. - }) + }) ); } @@ -342,10 +339,7 @@ contract SmartSession is ISmartSession, SmartSessionBase, SmartSessionERC7739 { // perform signature check with ISessionValidator // this function will revert if no ISessionValidator is set for this permissionId bool validSig = $sessionValidators.isValidISessionValidator({ - hash: userOpHash, - account: account, - permissionId: permissionId, - signature: decompressedSignature + hash: userOpHash, account: account, permissionId: permissionId, signature: decompressedSignature }); // if the ISessionValidator signature is invalid, the userOp is invalid @@ -372,11 +366,7 @@ contract SmartSession is ISmartSession, SmartSessionBase, SmartSessionERC7739 { * contentsType, * uint16(contentsType.length)) */ - function isValidSignatureWithSender( - address sender, - bytes32 hash, - bytes calldata signature - ) + function isValidSignatureWithSender(address sender, bytes32 hash, bytes calldata signature) external view override @@ -427,7 +417,6 @@ contract SmartSession is ISmartSession, SmartSessionBase, SmartSessionERC7739 { bytes32 contentHash = string(contents).hashERC7739Content(); // isolate the PermissionId and actual signature from the supplied signature param PermissionId permissionId = PermissionId.wrap(bytes32(signature[0:32])); - signature = signature[32:]; // forgefmt: disable-next-item if ( @@ -437,12 +426,18 @@ contract SmartSession is ISmartSession, SmartSessionBase, SmartSessionERC7739 { || !$enabledERC7739.enabledContentNames[permissionId][appDomainSeparator].contains(msg.sender, contentHash) ) return false; + // Extract the length of the policy data + uint256 policyDataLength = uint256(bytes32(signature[32:64])); + + // Calculate remaining signature offset + uint256 remainingSignatureOffset = 64 + policyDataLength; + // check the ERC-1271 policy bool valid = $erc1271Policies.checkERC1271({ account: msg.sender, requestSender: sender, hash: hash, - signature: signature, + signature: signature[64:remainingSignatureOffset], // extract policy data from the signature permissionId: permissionId, configId: permissionId.toErc1271PolicyId().toConfigId(), minPoliciesToEnforce: 1 @@ -455,7 +450,7 @@ contract SmartSession is ISmartSession, SmartSessionBase, SmartSessionERC7739 { hash: hash, account: msg.sender, permissionId: permissionId, - signature: signature + signature: signature[remainingSignatureOffset:] // extract the validator signature from the signature }); } } diff --git a/test/unit/base/ERC1271Base.t.sol b/test/unit/base/ERC1271Base.t.sol index c0cc507..705349d 100644 --- a/test/unit/base/ERC1271Base.t.sol +++ b/test/unit/base/ERC1271Base.t.sol @@ -64,16 +64,20 @@ contract ERC1271TestBase is BaseTest { bytes memory signature = abi.encodePacked( t.r, t.s, t.v, appDomainSeparator, contents, contentsDescription, uint16(contentsDescription.length) ); - signature = abi.encodePacked(permissionId, signature); + signature = abi.encodePacked( + permissionId, + uint256(32), + bytes32(hex"b4b3b4b3b4b3b4b3b4b3b4b3b4b3b4b3b4b3b4b3b4b3b4b3b4b3b44204206969"), + signature + ); if (is6492) { signature = _erc6492Wrap(signature); } // Success returns `0x1626ba7e`. assertEq( - IERC1271(t.account).isValidSignature( - _toContentsHash(contents), abi.encodePacked(address(smartSession), signature) - ), + IERC1271(t.account) + .isValidSignature(_toContentsHash(contents), abi.encodePacked(address(smartSession), signature)), expectSuccess ? bytes4(0x1626ba7e) : bytes4(0xffffffff) ); } @@ -85,12 +89,7 @@ contract ERC1271TestBase is BaseTest { ); } - function _toERC1271Hash( - address account, - bytes32 contents, - bytes memory contentsType, - bytes memory contentsName - ) + function _toERC1271Hash(address account, bytes32 contents, bytes memory contentsType, bytes memory contentsName) internal view returns (bytes32) @@ -120,10 +119,7 @@ contract ERC1271TestBase is BaseTest { return abi.encode(keccak256(bytes(t.name)), keccak256(bytes(t.version)), t.chainId, t.verifyingContract, t.salt); } - function _typedDataSignTypeHash( - bytes memory contentsType, - bytes memory contentsName - ) + function _typedDataSignTypeHash(bytes memory contentsType, bytes memory contentsName) internal pure returns (bytes32) From da73e6d5339b5cef44c33d74df3e89bf46d58541 Mon Sep 17 00:00:00 2001 From: highskore Date: Wed, 15 Oct 2025 16:58:39 +0100 Subject: [PATCH 2/2] fix(1271): add const and bounds checks --- contracts/DataTypes.sol | 3 +++ contracts/SmartSession.sol | 12 ++++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/contracts/DataTypes.sol b/contracts/DataTypes.sol index ad6fd56..acf88b4 100644 --- a/contracts/DataTypes.sol +++ b/contracts/DataTypes.sol @@ -174,6 +174,9 @@ ValidationData constant ERC4337_VALIDATION_FAILED = ValidationData.wrap(1); bytes4 constant EIP1271_SUCCESS = 0x1626ba7e; bytes4 constant EIP1271_FAILED = 0xFFFFFFFF; +uint256 constant POLICYDATA_LENGTH_OFFSET = 32; +uint256 constant POLICYDATA_OFFSET = 64; + uint256 constant ERC7579_MODULE_TYPE_VALIDATOR = 1; uint256 constant ERC7579_MODULE_TYPE_EXECUTOR = 2; uint256 constant ERC7579_MODULE_TYPE_FALLBACK = 3; diff --git a/contracts/SmartSession.sol b/contracts/SmartSession.sol index 9d811a1..d09dab9 100644 --- a/contracts/SmartSession.sol +++ b/contracts/SmartSession.sol @@ -416,7 +416,7 @@ contract SmartSession is ISmartSession, SmartSessionBase, SmartSessionERC7739 { { bytes32 contentHash = string(contents).hashERC7739Content(); // isolate the PermissionId and actual signature from the supplied signature param - PermissionId permissionId = PermissionId.wrap(bytes32(signature[0:32])); + PermissionId permissionId = PermissionId.wrap(bytes32(signature[0:POLICYDATA_LENGTH_OFFSET])); // forgefmt: disable-next-item if ( @@ -426,9 +426,10 @@ contract SmartSession is ISmartSession, SmartSessionBase, SmartSessionERC7739 { || !$enabledERC7739.enabledContentNames[permissionId][appDomainSeparator].contains(msg.sender, contentHash) ) return false; + // Make sure signature length is sufficient + if (signature.length < POLICYDATA_OFFSET) return false; // Extract the length of the policy data - uint256 policyDataLength = uint256(bytes32(signature[32:64])); - + uint256 policyDataLength = uint256(bytes32(signature[POLICYDATA_LENGTH_OFFSET:POLICYDATA_OFFSET])); // Calculate remaining signature offset uint256 remainingSignatureOffset = 64 + policyDataLength; @@ -437,7 +438,7 @@ contract SmartSession is ISmartSession, SmartSessionBase, SmartSessionERC7739 { account: msg.sender, requestSender: sender, hash: hash, - signature: signature[64:remainingSignatureOffset], // extract policy data from the signature + signature: signature[POLICYDATA_OFFSET:remainingSignatureOffset], // extract policy data from the signature permissionId: permissionId, configId: permissionId.toErc1271PolicyId().toConfigId(), minPoliciesToEnforce: 1 @@ -445,6 +446,9 @@ contract SmartSession is ISmartSession, SmartSessionBase, SmartSessionERC7739 { // if the erc1271 policy check failed, return false if (!valid) return valid; + + // Make sure signature length is sufficient + if (signature.length < remainingSignatureOffset) return false; // this call reverts if the ISessionValidator is not set return $sessionValidators.isValidISessionValidator({ hash: hash,