Skip to content

Commit

Permalink
[#754] Rename function, remove if-else
Browse files Browse the repository at this point in the history
  • Loading branch information
akshay-ap committed Jul 11, 2024
1 parent be6a6f5 commit 2743e53
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 10 deletions.
2 changes: 1 addition & 1 deletion modules/4337/certora/specs/Safe4337Module.spec
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion modules/4337/certora/specs/ValidationDataLastBitOne.spec
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
12 changes: 4 additions & 8 deletions modules/4337/contracts/Safe4337Module.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down

0 comments on commit 2743e53

Please sign in to comment.