Skip to content

Commit e7817ac

Browse files
ScreamingHawkScreamingHawk
authored
Length checks (#98)
1 parent fa0fae3 commit e7817ac

8 files changed

Lines changed: 301 additions & 12 deletions

File tree

‎src/extensions/passkeys/Passkeys.sol‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ contract Passkeys is ISapientCompact {
1717
WebAuthn.WebAuthnAuth _webAuthnAuth, bool _requireUserVerification, bytes32 _x, bytes32 _y
1818
);
1919

20+
/// @notice Error thrown when the signature length is invalid
21+
error InvalidSignatureLength();
22+
2023
function _rootForPasskey(
2124
bool _requireUserVerification,
2225
bytes32 _x,
@@ -96,6 +99,10 @@ contract Passkeys is ISapientCompact {
9699

97100
(_x, pointer) = LibBytes.readBytes32(_signature, pointer);
98101
(_y, pointer) = LibBytes.readBytes32(_signature, pointer);
102+
103+
if (pointer != _signature.length) {
104+
revert InvalidSignatureLength();
105+
}
99106
} else {
100107
(_webAuthnAuth, _requireUserVerification, _x, _y, _metadata) =
101108
abi.decode(_signature[1:], (WebAuthn.WebAuthnAuth, bool, bytes32, bytes32, bytes32));

‎src/extensions/sessions/SessionErrors.sol‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ library SessionErrors {
2020
error InvalidValue();
2121
/// @notice Invalid node type in session configuration
2222
error InvalidNodeType(uint256 flag);
23+
/// @notice Error thrown when the signature length is invalid
24+
error InvalidSignatureLength();
2325
/// @notice Error thrown when the payload kind is invalid
2426
error InvalidPayloadKind();
2527
/// @notice Error thrown when the calls length is invalid

‎src/extensions/sessions/SessionSig.sol‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,11 @@ library SessionSig {
179179
}
180180
}
181181

182+
if (pointer != encodedSignature.length) {
183+
// Invalid signature length
184+
revert SessionErrors.InvalidSignatureLength();
185+
}
186+
182187
return sig;
183188
}
184189

‎src/modules/Payload.sol‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ library Payload {
1313
/// @notice Error thrown when the kind is invalid
1414
error InvalidKind(uint8 kind);
1515

16+
/// @notice Error thrown when the encoding is invalid
17+
error InvalidPackedLength();
18+
1619
/// @dev keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)")
1720
bytes32 private constant EIP712_DOMAIN_TYPEHASH = 0x8b73c3c69bb8fe3d512ecc4cf759cc79239f7b179b0ffacaa9a75d522b39400f;
1821

@@ -212,6 +215,12 @@ library Payload {
212215
// Last 2 bits are directly mapped to the behavior on error
213216
_decoded.calls[i].behaviorOnError = (flags & 0xC0) >> 6;
214217
}
218+
219+
if (pointer != packed.length) {
220+
revert InvalidPackedLength();
221+
}
222+
223+
return _decoded;
215224
}
216225

217226
function hashCall(

‎test/extensions/passkeys/Passkeys.t.sol‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,73 @@ contract PasskeysTest is AdvTest {
402402
assertEq(vars.recoveredRoot, vars.expectedRoot, "Recovered root should match expected root");
403403
}
404404

405+
function test_recoverSapientSignatureCompact_paddedFails(RecoverParams memory params, bytes memory padding) public {
406+
vm.assume(padding.length > 0);
407+
vm.etch(P256_VERIFIER, P256_VERIFIER_RUNTIME_CODE);
408+
409+
recoverSapientSignatureCompact_valid_vars memory vars;
410+
411+
params.pkSeed = boundP256Pk(params.pkSeed);
412+
vars.wallet = vm.createWallet(params.pkSeed);
413+
414+
if (params.embedMetadata) {
415+
vm.assume(params.metadataHash != bytes32(0));
416+
} else {
417+
params.metadataHash = bytes32(0);
418+
}
419+
420+
(vars.pubX, vars.pubY) = vm.publicKeyP256(vars.wallet.privateKey);
421+
vars.pkParams.x = bytes32(vars.pubX);
422+
vars.pkParams.y = bytes32(vars.pubY);
423+
vars.pkParams.requireUserVerification = params.requireUserVerification;
424+
vars.pkParams.metadataHash = params.metadataHash;
425+
vars.pkParams.credentialId = "";
426+
427+
uint8 flags = 0x01;
428+
if (params.requireUserVerification) {
429+
flags |= 0x04;
430+
}
431+
432+
vars.generatedAuthenticatorData = abi.encodePacked(params.rpIdHash, flags, params.signCount);
433+
434+
string memory raw = vm.toBase64URL(abi.encodePacked(params.digest));
435+
if (bytes(raw)[bytes(raw).length - 1] == "=") {
436+
assembly {
437+
mstore(raw, sub(mload(raw), 1))
438+
}
439+
}
440+
vars.base64UrlChallenge = raw;
441+
vars.typeValue = "webauthn.get";
442+
vars.originValue = generateRandomString(params.originValueSeed);
443+
vm.assume(bytes(vars.originValue).length > 0);
444+
445+
vars.clientDataJSON = string.concat(
446+
'{"type":"', vars.typeValue, '","challenge":"', vars.base64UrlChallenge, '","origin":"', vars.originValue, '"}'
447+
);
448+
vars.sigParams.clientDataJson = vars.clientDataJSON;
449+
vars.sigParams.authenticatorData = vars.generatedAuthenticatorData;
450+
451+
vars.clientDataJSONHash = sha256(bytes(vars.clientDataJSON));
452+
vars.messageHash = sha256(abi.encodePacked(vars.generatedAuthenticatorData, vars.clientDataJSONHash));
453+
(bytes32 rVal, bytes32 sVal) = vm.signP256(vars.wallet.privateKey, vars.messageHash);
454+
455+
bytes32 halfN = bytes32(0x7fffffff800000007fffffffffffffffde737d56d38bcf4279dce5617e3192a8);
456+
if (sVal > halfN) {
457+
sVal = bytes32(0xffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632551 - uint256(sVal));
458+
}
459+
460+
vars.sigParams.r = bytes32(rVal);
461+
vars.sigParams.s = bytes32(sVal);
462+
vars.encodedSignature =
463+
PrimitivesRPC.passkeysEncodeSignature(vm, vars.pkParams, vars.sigParams, params.embedMetadata);
464+
465+
vars.expectedRoot = PrimitivesRPC.passkeysComputeRoot(vm, vars.pkParams);
466+
467+
bytes memory paddedSignature = abi.encodePacked(vars.encodedSignature, padding);
468+
vm.expectRevert(abi.encodeWithSelector(Passkeys.InvalidSignatureLength.selector));
469+
passkeysImp.recoverSapientSignatureCompact(params.digest, paddedSignature);
470+
}
471+
405472
struct recoverSapientSignatureCompact_invalidSignature_vars {
406473
Vm.Wallet wallet;
407474
Vm.Wallet wrongWallet;

‎test/extensions/recovery/Recovery.t.sol‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,82 @@ contract RecoveryTest is AdvTest {
437437
assertEq(vars.recoveredRoot, vars.rpcRoot);
438438
}
439439

440+
function test_fail_padded_signature(
441+
test_recover_sapient_signature_compact_params memory params,
442+
bytes memory padding
443+
) external {
444+
vm.assume(padding.length > 0);
445+
446+
boundToLegalPayload(params.payload);
447+
params.startTime = bound(params.startTime, 1, type(uint64).max / 2); // Safer upper bound perhaps
448+
params.requiredDeltaTime = bound(params.requiredDeltaTime, 0, type(uint24).max);
449+
params.minTimestamp = bound(params.minTimestamp, 0, params.startTime);
450+
uint256 minPassedTime = params.requiredDeltaTime == type(uint64).max ? type(uint64).max : params.requiredDeltaTime;
451+
452+
params.passedTime = bound(params.passedTime, minPassedTime, type(uint64).max);
453+
vm.warp(params.startTime);
454+
455+
params.signerPk = boundPk(params.signerPk);
456+
457+
test_recover_sapient_signature_compact_vars memory vars;
458+
vars.recoveryPayloadHash = recovery.recoveryPayloadHash(params.wallet, params.payload);
459+
vars.payloadHash = Payload.hashFor(params.payload, params.wallet);
460+
(vars.v, vars.r, vars.s) = vm.sign(params.signerPk, vars.recoveryPayloadHash);
461+
462+
vars.yParityAndS = bytes32((uint256(vars.v - 27) << 255) | uint256(vars.s));
463+
vars.signature = abi.encodePacked(vars.r, vars.yParityAndS);
464+
465+
vars.signerAddr = vm.addr(params.signerPk);
466+
467+
recovery.queuePayload(params.wallet, vars.signerAddr, params.payload, vars.signature);
468+
469+
vm.warp(block.timestamp + params.passedTime);
470+
471+
vars.parts = "";
472+
for (uint256 i = 0; i < params.suffixes.length; i++) {
473+
vars.parts = string.concat(
474+
vars.parts,
475+
"signer:",
476+
vm.toString(params.suffixes[i].signer),
477+
":",
478+
vm.toString(params.suffixes[i].requiredDeltaTime),
479+
":",
480+
vm.toString(params.suffixes[i].minTimestamp),
481+
" "
482+
);
483+
}
484+
485+
vars.parts = string.concat(
486+
vars.parts,
487+
"signer:",
488+
vm.toString(vars.signerAddr),
489+
":",
490+
vm.toString(params.requiredDeltaTime),
491+
":",
492+
vm.toString(params.minTimestamp)
493+
);
494+
495+
for (uint256 i = 0; i < params.prefixes.length; i++) {
496+
vars.parts = string.concat(
497+
vars.parts,
498+
" signer:",
499+
vm.toString(params.prefixes[i].signer),
500+
":",
501+
vm.toString(params.prefixes[i].requiredDeltaTime),
502+
":",
503+
vm.toString(params.prefixes[i].minTimestamp)
504+
);
505+
}
506+
507+
vars.rpcRoot = PrimitivesRPC.recoveryHashFromLeaves(vm, vars.parts);
508+
509+
vars.encoded = PrimitivesRPC.recoveryTrim(vm, vars.parts, vars.signerAddr);
510+
bytes memory signature = abi.encodePacked(vars.encoded, padding);
511+
vm.prank(params.wallet);
512+
vm.expectRevert(); // Unspecified revert
513+
vars.recoveredRoot = recovery.recoverSapientSignatureCompact(vars.payloadHash, signature);
514+
}
515+
440516
function test_recover_sapient_signature_compact_fail_minTimestamp(
441517
test_recover_sapient_signature_compact_params memory params
442518
) external {

‎test/extensions/sessions/SessionManager.t.sol‎

Lines changed: 82 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,15 @@ contract SessionManagerTest is SessionTestBase {
3737
emitter = new Emitter();
3838
}
3939

40-
/// @notice Valid explicit session test.
41-
function testValidExplicitSessionSignature(
40+
function _prepareTestValidExplicitSessionSignature(
4241
address wallet,
4342
bytes4 selector,
4443
uint256 param,
4544
uint256 value,
4645
address explicitTarget,
4746
address explicitTarget2,
4847
bool useChainId
49-
) public {
48+
) public returns (bytes32 imageHash, Payload.Decoded memory payload, bytes memory encodedSig) {
5049
vm.assume(explicitTarget != explicitTarget2);
5150
vm.assume(value > 0);
5251
vm.assume(param > 0);
@@ -92,7 +91,7 @@ contract SessionManagerTest is SessionTestBase {
9291
// Call 1: call not requiring incrementUsageLimit
9392
// Call 2: call requiring incrementUsageLimit
9493
// Call 0: the required incrementUsageLimit call (self–call)
95-
Payload.Decoded memory payload = _buildPayload(3);
94+
payload = _buildPayload(3);
9695

9796
// --- Explicit Call 1 ---
9897
payload.calls[1] = Payload.Call({
@@ -141,22 +140,61 @@ contract SessionManagerTest is SessionTestBase {
141140
permissionIdxs[1] = 0; // Call 1
142141
permissionIdxs[2] = 1; // Call 2
143142

144-
(bytes32 imageHash, bytes memory encodedSig) =
145-
_validExplicitSessionSignature(wallet, payload, sessionPerms, permissionIdxs);
143+
(imageHash, encodedSig) = _validExplicitSessionSignature(wallet, payload, sessionPerms, permissionIdxs);
144+
}
145+
146+
/// @notice Valid explicit session test.
147+
function testValidExplicitSessionSignature(
148+
address wallet,
149+
bytes4 selector,
150+
uint256 param,
151+
uint256 value,
152+
address explicitTarget,
153+
address explicitTarget2,
154+
bool useChainId
155+
) public {
156+
(bytes32 imageHash, Payload.Decoded memory payload, bytes memory encodedSig) =
157+
_prepareTestValidExplicitSessionSignature(
158+
wallet, selector, param, value, explicitTarget, explicitTarget2, useChainId
159+
);
146160

147161
vm.prank(wallet);
148162
bytes32 actualImageHash = sessionManager.recoverSapientSignature(payload, encodedSig);
149163
assertEq(imageHash, actualImageHash);
150164
}
151165

152-
/// @notice Valid explicit session test with multiple signers.
153-
function testValidExplicitSessionMixing(
166+
/// @notice Valid explicit session test, fails when padding added.
167+
function testValidExplicitSessionSignature_padded_fails(
154168
address wallet,
155169
bytes4 selector,
156170
uint256 param,
171+
uint256 value,
157172
address explicitTarget,
158-
bool useChainId
173+
address explicitTarget2,
174+
bool useChainId,
175+
bytes memory padding
159176
) public {
177+
vm.assume(padding.length > 0);
178+
179+
(bytes32 imageHash, Payload.Decoded memory payload, bytes memory encodedSig) =
180+
_prepareTestValidExplicitSessionSignature(
181+
wallet, selector, param, value, explicitTarget, explicitTarget2, useChainId
182+
);
183+
184+
bytes memory paddedSignature = abi.encodePacked(encodedSig, padding);
185+
186+
vm.prank(wallet);
187+
vm.expectRevert(abi.encodeWithSelector(SessionErrors.InvalidSignatureLength.selector));
188+
sessionManager.recoverSapientSignature(payload, paddedSignature);
189+
}
190+
191+
function _prepareTestValidExplicitSessionMixing(
192+
address wallet,
193+
bytes4 selector,
194+
uint256 param,
195+
address explicitTarget,
196+
bool useChainId
197+
) internal returns (bytes32 imageHash, Payload.Decoded memory payload, bytes memory encodedSig) {
160198
Vm.Wallet memory sessionWallet2 = vm.createWallet("session2");
161199
vm.assume(param > 0);
162200
vm.assume(explicitTarget != wallet);
@@ -225,7 +263,7 @@ contract SessionManagerTest is SessionTestBase {
225263
// Call 1: using session 1 (non cumulative signer)
226264
// Call 2: using session 2 (cumulative signer)
227265
// Call 0: the required incrementUsageLimit call (self–call)
228-
Payload.Decoded memory payload = _buildPayload(3);
266+
payload = _buildPayload(3);
229267

230268
// --- Explicit Call 1 ---
231269
payload.calls[1] = Payload.Call({
@@ -266,7 +304,7 @@ contract SessionManagerTest is SessionTestBase {
266304
topology = PrimitivesRPC.sessionExplicitAdd(vm, sessionPermsJson, topology);
267305
sessionPermsJson = _sessionPermissionsToJSON(sessionPerms2);
268306
topology = PrimitivesRPC.sessionExplicitAdd(vm, sessionPermsJson, topology);
269-
bytes32 imageHash = PrimitivesRPC.sessionImageHash(vm, topology);
307+
imageHash = PrimitivesRPC.sessionImageHash(vm, topology);
270308

271309
string[] memory callSignatures = new string[](3);
272310
// Sign call 1 with signer 1
@@ -284,14 +322,46 @@ contract SessionManagerTest is SessionTestBase {
284322
explicitSigners[0] = sessionWallet.addr;
285323
explicitSigners[1] = sessionWallet2.addr;
286324
address[] memory implicitSigners = new address[](0);
287-
bytes memory encodedSig =
325+
encodedSig =
288326
PrimitivesRPC.sessionEncodeCallSignatures(vm, topology, callSignatures, explicitSigners, implicitSigners);
327+
}
328+
329+
/// @notice Valid explicit session test with multiple signers.
330+
function testValidExplicitSessionMixing(
331+
address wallet,
332+
bytes4 selector,
333+
uint256 param,
334+
address explicitTarget,
335+
bool useChainId
336+
) public {
337+
(bytes32 imageHash, Payload.Decoded memory payload, bytes memory encodedSig) =
338+
_prepareTestValidExplicitSessionMixing(wallet, selector, param, explicitTarget, useChainId);
289339

290340
vm.prank(wallet);
291341
bytes32 actualImageHash = sessionManager.recoverSapientSignature(payload, encodedSig);
292342
assertEq(imageHash, actualImageHash);
293343
}
294344

345+
/// @notice Valid explicit session test with multiple signers fails when padding added.
346+
function testValidExplicitSessionMixing_padded_fails(
347+
address wallet,
348+
bytes4 selector,
349+
uint256 param,
350+
address explicitTarget,
351+
bool useChainId,
352+
bytes memory padding
353+
) public {
354+
vm.assume(padding.length > 0);
355+
(bytes32 imageHash, Payload.Decoded memory payload, bytes memory encodedSig) =
356+
_prepareTestValidExplicitSessionMixing(wallet, selector, param, explicitTarget, useChainId);
357+
358+
bytes memory paddedSignature = abi.encodePacked(encodedSig, padding);
359+
360+
vm.prank(wallet);
361+
vm.expectRevert(abi.encodeWithSelector(SessionErrors.InvalidSignatureLength.selector));
362+
sessionManager.recoverSapientSignature(payload, paddedSignature);
363+
}
364+
295365
function testIncrementReentrancy() external {
296366
MockERC20 token = new MockERC20();
297367
CanReenter canReenter = new CanReenter();

0 commit comments

Comments
 (0)