Skip to content

fix(1271): separate policyData and signature #177

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

Open
highskore wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(1271): separate policyData and signature #177

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8681504
fix(1271): parse policydata from sig
highskore Oct 15, 2025
da73e6d
fix(1271): add const and bounds checks
highskore Oct 15, 2025
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
65 changes: 30 additions & 35 deletions contracts/SmartSession.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand Down Expand Up @@ -279,25 +273,27 @@ 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
else if (callType == CALLTYPE_SINGLE) {
(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
Expand All @@ -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,
Expand All @@ -332,7 +329,7 @@ contract SmartSession is ISmartSession, SmartSessionBase, SmartSessionERC7739 {
)
),
minPolicies: 1 // minimum of one actionPolicy must be set.
})
})
);
}

Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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 (
Expand All @@ -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;
Comment thread
highskore marked this conversation as resolved.
Comment thread
highskore marked this conversation as resolved.

// 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
Comment thread
highskore marked this conversation as resolved.
Outdated
Comment thread
highskore marked this conversation as resolved.
Outdated

@zeroknots zeroknots Oct 15, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

permissionId: permissionId,
configId: permissionId.toErc1271PolicyId().toConfigId(),
minPoliciesToEnforce: 1
Expand All @@ -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
Comment thread
highskore marked this conversation as resolved.
Comment thread
highskore marked this conversation as resolved.
});
}
}
24 changes: 10 additions & 14 deletions test/unit/base/ERC1271Base.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);
}
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down