Address Code Audit

contracts/accounts/Simple7702Account.sol

@@ -37,7 +37,7 @@ contract Simple7702Account is BaseAccount, IERC165, IERC1271, ERC1155Holder, ERC
         return _checkSignature(userOpHash, userOp.signature) ? SIG_VALIDATION_SUCCESS : SIG_VALIDATION_FAILED;
     }
 
-    function isValidSignature(bytes32 hash, bytes memory signature) public view returns (bytes4 magicValue) {
+    function isValidSignature(bytes32 hash, bytes memory signature) public virtual view returns (bytes4 magicValue) {
         return _checkSignature(hash, signature) ? this.isValidSignature.selector : bytes4(0xffffffff);
     }
 
@@ -57,7 +57,7 @@ contract Simple7702Account is BaseAccount, IERC165, IERC1271, ERC1155Holder, ERC
         );
     }
 
-    function supportsInterface(bytes4 id) public override(ERC1155Holder, IERC165) pure returns (bool) {
+    function supportsInterface(bytes4 id) public virtual override(ERC1155Holder, IERC165) pure returns (bool) {
         return
             id == type(IERC165).interfaceId ||
             id == type(IAccount).interfaceId ||

contracts/accounts/SimpleAccount.sol

@@ -71,7 +71,7 @@ contract SimpleAccount is BaseAccount, TokenCallbackHandler, UUPSUpgradeable, In
 
     function _initialize(address anOwner) internal virtual {
         owner = anOwner;
-        emit SimpleAccountInitialized(_entryPoint, owner);
+        emit SimpleAccountInitialized(entryPoint(), owner);
     }
 
     // Require the function call went through EntryPoint or owner
@@ -99,7 +99,7 @@ contract SimpleAccount is BaseAccount, TokenCallbackHandler, UUPSUpgradeable, In
     /**
      * check current account deposit in the entryPoint
      */
-    function getDeposit() public view returns (uint256) {
+    function getDeposit() public virtual view returns (uint256) {
         return entryPoint().balanceOf(address(this));
     }
 
@@ -115,7 +115,7 @@ contract SimpleAccount is BaseAccount, TokenCallbackHandler, UUPSUpgradeable, In
      * @param withdrawAddress target to send to
      * @param amount to withdraw
      */
-    function withdrawDepositTo(address payable withdrawAddress, uint256 amount) public onlyOwner {
+    function withdrawDepositTo(address payable withdrawAddress, uint256 amount) public virtual onlyOwner {
         entryPoint().withdrawTo(withdrawAddress, amount);
     }
 

contracts/accounts/SimpleAccountFactory.sol

@@ -52,7 +52,7 @@ contract SimpleAccountFactory {
     /**
      * calculate the counterfactual address of this account as it would be returned by createAccount()
      */
-    function getAddress(address owner,uint256 salt) public view returns (address) {
+    function getAddress(address owner,uint256 salt) public virtual view returns (address) {
         return Create2.computeAddress(bytes32(salt), keccak256(abi.encodePacked(
                 type(ERC1967Proxy).creationCode,
                 abi.encode(

contracts/core/BasePaymaster.sol

@@ -107,7 +107,7 @@ abstract contract BasePaymaster is IPaymaster, Stakeable {
      * Add a deposit for this paymaster, used for paying for transaction fees.
      */
     function deposit() public payable {
-        _entryPoint.depositTo{value: msg.value}(address(this));
+        entryPoint().depositTo{value: msg.value}(address(this));
     }
 
     /**
@@ -119,21 +119,21 @@ abstract contract BasePaymaster is IPaymaster, Stakeable {
         address payable withdrawAddress,
         uint256 amount
     ) public onlyOwner {
-        _entryPoint.withdrawTo(withdrawAddress, amount);
+        entryPoint().withdrawTo(withdrawAddress, amount);
     }
 
     /**
      * Return current paymaster's deposit on the entryPoint.
      */
-    function getDeposit() public view returns (uint256) {
+    function getDeposit() public virtual view returns (uint256) {
         return _entryPoint.balanceOf(address(this));
     }
 
     /**
      * Validate the call is made from a valid entrypoint
      */
     function _requireFromEntryPoint() internal virtual {
-        require(msg.sender == address(_entryPoint),
+        require(msg.sender == address(entryPoint()),
             NotFromEntryPoint(
                 msg.sender,
                 address(this),

contracts/core/Eip7702Support.sol

@@ -1,5 +1,6 @@
-pragma solidity ^0.8.28;
 // SPDX-License-Identifier: MIT
+pragma solidity ^0.8.28;
+
 // solhint-disable no-inline-assembly
 
 import "../interfaces/PackedUserOperation.sol";
@@ -75,7 +76,6 @@ library Eip7702Support {
         // To be a valid EIP-7702 delegate, the first 3 bytes are EIP7702_PREFIX
         // followed by the delegate address
         if (bytes3(senderCode) != EIP7702_PREFIX) {
-            // instead of just "not an EIP-7702 delegate", if some info.
             require(sender.code.length > 0, Eip7702SenderWithoutCode(sender));
             revert Eip7702SenderNotDelegate(sender);
         }

contracts/core/EntryPoint.sol

@@ -22,6 +22,8 @@ import "@openzeppelin/contracts/utils/introspection/ERC165.sol";
 import "@openzeppelin/contracts/utils/cryptography/EIP712.sol";
 
 /**
+ * Always verify the EntryPoint addresses across multiple trusted sources.
+ * Visit https://docs.erc4337.io/ for instructions and official documentation.
  * Account-Abstraction (EIP-4337) singleton EntryPoint v0.9 implementation.
  * Only one instance required on each chain.
  * @custom:security-contact https://bounty.ethereum.org
@@ -210,6 +212,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ERC165, EIP712 {
      */
     function _compensate(address payable beneficiary, uint256 amount) internal virtual {
         require(beneficiary != address(0), InvalidBeneficiary(beneficiary));
+        currentUserOpHash = bytes32(0);
         (bool success, bytes memory ret) = beneficiary.call{value: amount}("");
         require(success, FailedSendToBeneficiary(beneficiary, amount, ret));
     }
@@ -681,7 +684,6 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ERC165, EIP712 {
         bool success;
         uint256 contextLength;
         uint256 contextOffset;
-        uint256 maxContextLength;
         uint256 len;
         assembly ("memory-safe") {
             success := call(paymasterVerificationGasLimit, paymaster, 0, add(validatePaymasterCall, 0x20), mload(validatePaymasterCall), 0, 0)
@@ -699,15 +701,19 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ERC165, EIP712 {
             returndatacopy(freePtr, 0, len)
             validationData := mload(add(freePtr, 32))
             contextOffset := mload(freePtr)
-            maxContextLength := sub(len, 96)
             context := add(freePtr, 64)
             contextLength := mload(context)
         }
 
         unchecked {
-            if (!success || contextOffset != 64 || contextLength + 31 < maxContextLength) {
+            if (!success) {
                 revert FailedOpWithRevert(opIndex, "AA33 reverted", Exec.getReturnData(REVERT_REASON_MAX_LEN));
             }
+            // for a given 'contextLength', calculate the only valid 'returndatasize' value
+            uint256 expectedReturnDataSize = 96 + ((contextLength + 31) / 32) * 32;
+            if (contextOffset != 64 || len != expectedReturnDataSize) {
+                revert FailedOpWithRevert(opIndex, "AA35 malformed paymaster data", Exec.getReturnData(REVERT_REASON_MAX_LEN));
+            }
         }
         finalizeAllocation(freePtr, len);
     }
@@ -769,7 +775,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ERC165, EIP712 {
         }
         ValidationData memory data = _parseValidationData(validationData);
         // using top bit of 'validAfter' and 'validUntil' to indicate block-range instead of time-range
-        if (data.validAfter > VALIDITY_BLOCK_RANGE_FLAG && data.validUntil > VALIDITY_BLOCK_RANGE_FLAG) {
+        if (data.validAfter >= VALIDITY_BLOCK_RANGE_FLAG && data.validUntil >= VALIDITY_BLOCK_RANGE_FLAG) {
             uint48 validAfterBlock = data.validAfter & VALIDITY_BLOCK_RANGE_MASK;
             uint48 validUntilBlock = data.validUntil & VALIDITY_BLOCK_RANGE_MASK;
             outOfValidityRange = block.number > validUntilBlock || block.number <= validAfterBlock;
@@ -805,7 +811,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ERC165, EIP712 {
         MemoryUserOp memory mUserOp = outOpInfo.mUserOp;
         _copyUserOpToMemory(userOp, mUserOp);
 
-        // getUserOpHash uses temporary allocations, no required after it returns
+        // 'getUserOpHash' uses temporary memory allocation and all data allocated inside can be reused after it returns
         uint256 freePtr = _getFreePtr();
         outOpInfo.userOpHash = getUserOpHash(userOp);
         _restoreFreePtr(freePtr);

contracts/core/Helpers.sol

@@ -108,7 +108,7 @@ function calldataKeccak(bytes calldata data) pure returns (bytes32 ret) {
  * This function copies the first `len` bytes from the given calldata array `data` into memory.
  * The assembly code is equivalent to:
  *      keccak256(abi.encodePacked(data[0:len], suffix))
- * But more efficient, and doesn't leave the copied data in memory.
+ * But more efficient, and doesn't move the free memory pointer, allowing the memory to be reused later.
  *
  * @param data   Calldata byte array to read from.
  * @param len    Number of bytes to copy from `data` starting at its offset.

contracts/core/NonceManager.sol

@@ -15,7 +15,7 @@ abstract contract NonceManager is INonceManager {
 
     /// @inheritdoc INonceManager
     function getNonce(address sender, uint192 key)
-    public view override returns (uint256 nonce) {
+    public virtual view override returns (uint256 nonce) {
         return nonceSequenceNumber[sender][key] | (uint256(key) << 64);
     }
 

contracts/interfaces/IEntryPoint.sol

@@ -127,13 +127,17 @@ interface IEntryPoint is IStakeManager, INonceManager {
     event SignatureAggregatorChanged(address indexed aggregator);
 
     /**
-     * A custom revert error of handleOps andhandleAggregatedOps, to identify the offending op.
-     * Should be caught in off-chain handleOps/handleAggregatedOps simulation and not happen on-chain.
+     * A custom revert error of 'handleOps' and 'handleAggregatedOps', to identify the offending UserOperation.
+     * Should be caught in off-chain 'handleOps'/'handleAggregatedOps' simulation and should not happen on-chain.
+     *
      * Useful for mitigating DoS attempts against batchers or for troubleshooting of factory/account/paymaster reverts.
-     * NOTE: If simulateValidation passes successfully, there should be no reason for handleOps to fail on it.
-     * @param opIndex - Index into the array of ops to the failed one (in simulateValidation, this is always zero).
+     * NOTE: If 'simulateValidation' passes successfully, there should be no reason for 'handleOps' to revert as well.
+     *
+     * @param opIndex - Index into the array of ops to the failed one.
+     *                  When using 'simulateValidation', this value is always zero.
+     *
      * @param reason  - Revert reason. The string starts with a unique code "AAmn",
-     *                  where "m" is "1" for factory, "2" for account and "3" for paymaster issues,
+     *                  where "m" is "1" for factory, "2" for account, "3" for paymaster, and "9" for other issues,
      *                  so a failure can be attributed to the correct entity.
      */
     error FailedOp(uint256 opIndex, string reason);
@@ -146,9 +150,9 @@ interface IEntryPoint is IStakeManager, INonceManager {
 
     /**
      * A custom revert error of handleOps and handleAggregatedOps, to report a revert by account or paymaster.
-     * @param opIndex - Index into the array of ops to the failed one (in simulateValidation, this is always zero).
-     * @param reason  - Revert reason. see FailedOp(uint256,string), above
-     * @param inner   - data from inner cought revert reason
+     * @param opIndex - Index of the failed UserOperation in the array of ops. In simulateValidation, this value is always zero.
+     * @param reason  - Revert reason. See the 'FailedOp(uint256,string)' error above.
+     * @param inner   - Revert data caught from the inner revert reason of an entity contract.
      * @dev note that inner is truncated to 2048 bytes
      */
     error FailedOpWithRevert(uint256 opIndex, string reason, bytes inner);

contracts/interfaces/IEntryPointSimulations.sol

@@ -58,13 +58,11 @@ interface IEntryPointSimulations is IEntryPoint {
     );
 
     /**
-     * Simulate full execution of a UserOperation (including both validation and target execution)
-     * It performs full validation of the UserOperation, but ignores signature error.
-     * An optional target address is called after the userop succeeds,
-     * and its value is returned (before the entire call is reverted).
-     * Note that in order to collect the the success/failure of the target call, it must be executed
-     * with trace enabled to track the emitted events.
-     * @param op The UserOperation to simulate.
+     * Simulate the full execution of a UserOperation, including both validation and target execution.
+     * It performs a full validation of the UserOperation, but ignores signature error.
+     * An optional target address is called after the UserOperation succeeds,
+     * and this call's status and returned data value are returned as part of the `ExecutionResult` struct.
+     * @param op             - The UserOperation to simulate.
      * @param target         - If nonzero, a target address to call after userop simulation. If called,
      *                         the targetSuccess and targetResult are set to the return from that call.
      * @param targetCallData - CallData to pass to target address.

contracts/interfaces/INonceManager.sol

@@ -17,7 +17,7 @@ interface INonceManager {
 
     /**
      * Manually increment the nonce of the sender.
-     * This method is exposed just for completeness..
+     * This method is exposed just for completeness.
      * Account does NOT need to call it, neither during validation, nor elsewhere,
      * as the EntryPoint will update the nonce regardless.
      * Possible use-case is call it with various keys to "initialize" their nonces to one, so that future

contracts/interfaces/PackedUserOperation.sol

@@ -22,7 +22,8 @@ pragma solidity ^0.8.28;
  * - nonce = uint192(key) || uint64(sequence); EntryPoint tracks sequential values of `sequence` separately for each `key` value.
  * - initCode:
  *     * non-7702: `initCode = factory(20) || factoryCalldata`; the factory must return `sender` and deploy code.
- *     * 7702: `0x7702` (magic prefix), optionally padded to 20 bytes and followed by `initizlizationCode`. This optional payload is executed on `sender` to finalise delegate setup.
+ *     * The `initCode` will be ignored if the `sender` is already deployed.
+ *     * 7702: `0x7702` (magic prefix), optionally padded to 20 bytes and followed by the actual `initializationCode` data. This optional payload is executed on `sender` to finalise delegate setup.
  * - callData: executed verbatim; if it starts with `IAccountExecute.executeUserOp.selector` (0x8dd7712f), EntryPoint wraps and forwards `(userOp, userOpHash)`.
  * - accountGasLimits =`uint128(verificationGasLimit) || uint128(callGasLimit)`
  * - gasFees = `uint128(maxPriorityFeePerGas) || uint128(maxFeePerGas)`

contracts/test/TestAggregatedAccountFactory.sol

@@ -39,7 +39,7 @@ contract TestAggregatedAccountFactory {
     /**
      * calculate the counterfactual address of this account as it would be returned by createAccount()
      */
-    function getAddress(address owner,uint256 salt) public view returns (address) {
+    function getAddress(address owner,uint256 salt) public virtual view returns (address) {
         return Create2.computeAddress(bytes32(salt), keccak256(abi.encodePacked(
                 type(ERC1967Proxy).creationCode,
                 abi.encode(

contracts/test/TestExecAccountFactory.sol

@@ -15,7 +15,7 @@ contract TestExecAccountFactory {
         accountImplementation = new TestExecAccount(_entryPoint);
     }
 
-    function createAccount(address owner, uint256 salt) public returns (address ret) {
+    function createAccount(address owner, uint256 salt) public virtual returns (address ret) {
         address addr = getAddress(owner, salt);
         uint256 codeSize = addr.code.length;
         if (codeSize > 0) {