diff --git a/modules/4337/certora/specs/Safe4337Module.spec b/modules/4337/certora/specs/Safe4337Module.spec index 4cc2405f2..9754b7e70 100644 --- a/modules/4337/certora/specs/Safe4337Module.spec +++ b/modules/4337/certora/specs/Safe4337Module.spec @@ -25,7 +25,7 @@ methods { function getOperationHash( Safe4337Module.PackedUserOperation userOp ) external returns(bytes32) envfree => PER_CALLEE_CONSTANT; - function _checkSignatureLength(bytes calldata, uint256) internal returns(bool) => ALWAYS(true); + function _checkSignaturesLength(bytes calldata, uint256) internal returns(bool) => ALWAYS(true); } persistent ghost ERC2771MessageSender() returns address; diff --git a/modules/4337/certora/specs/ValidationDataLastBitOne.spec b/modules/4337/certora/specs/ValidationDataLastBitOne.spec index 4ff23e127..7f32f0786 100644 --- a/modules/4337/certora/specs/ValidationDataLastBitOne.spec +++ b/modules/4337/certora/specs/ValidationDataLastBitOne.spec @@ -11,7 +11,7 @@ methods { function getOperationHash( Safe4337Module.PackedUserOperation userOp ) external returns(bytes32) envfree => PER_CALLEE_CONSTANT; - function _checkSignatureLength(bytes calldata, uint256) internal returns(bool) => ALWAYS(true); + function _checkSignaturesLength(bytes calldata, uint256) internal returns(bool) => ALWAYS(true); } rule validationDataLastBitOneIfCheckSignaturesFails(address sender, diff --git a/modules/4337/contracts/Safe4337Module.sol b/modules/4337/contracts/Safe4337Module.sol index 736c29322..adcdea9e6 100644 --- a/modules/4337/contracts/Safe4337Module.sol +++ b/modules/4337/contracts/Safe4337Module.sol @@ -264,19 +264,15 @@ contract Safe4337Module is IAccount, HandlerContext, CompatibilityFallbackHandle // The `checkSignatures` function in the Safe contract does not force a fixed size on signature length. // A malicious bundler can pad the Safe operation `signatures` with additional bytes, causing the account to pay more gas than needed for user operation validation (capped by `verificationGasLimit`). - // `_checkSignatureLength` ensures that there are no additional bytes in the `signature` than are required. - bool validSignature = _checkSignatureLength(signatures, ISafe(payable(userOp.sender)).getThreshold()); + // `_checkSignaturesLength` ensures that there are no additional bytes in the `signature` than are required. + bool validSignature = _checkSignaturesLength(signatures, ISafe(payable(userOp.sender)).getThreshold()); try ISafe(payable(userOp.sender)).checkSignatures(keccak256(operationData), operationData, signatures) {} catch { validSignature = false; } - if (validSignature) { - // The timestamps are validated by the entry point, therefore we will not check them again - validationData = _packValidationData(false, validUntil, validAfter); - } else { - validationData = _packValidationData(true, validUntil, validAfter); - } + // The timestamps are validated by the entry point, therefore we will not check them again + validationData = _packValidationData(!validSignature, validUntil, validAfter); } /**