## Findings

### 1. `validUntil` / `validAfter` are not covered by the signature — attacker can extend or bypass user-specified time windows

- **Severity:** High
- **Location:** `src/protocol/wallet/AtomWallet.sol :: _validateSignature` / `_extractValidUntilAndValidAfterFromSignature` (≈L247–361)
- **Root cause:** The ECDSA signature is computed over `keccak256("\x19Ethereum Signed Message:\n32" || userOpHash)` only. The 12-byte `validUntil || validAfter` suffix appended to `userOp.signature` is parsed out *before* recovery and is **never authenticated**. `userOpHash` itself (as computed by the EntryPoint) is derived from `userOp` fields excluding `signature`, so flipping any bytes in the suffix does not change `userOpHash`.
- **Impact:**
  - A user signs a `UserOp` with a restrictive `[validAfter, validUntil]` (e.g., expecting the txn to expire in 5 minutes). Anyone observing the user's signed op in the mempool / bundler pool can strip or mutate the 12-byte suffix (e.g., re-encode as a 65-byte signature, which the contract interprets as `validUntil=0 → no expiry`, or set `validUntil=type(uint48).max`) and re-submit the op through the EntryPoint at an arbitrary later time. The nonce still matches and the underlying ECDSA signature still verifies.
  - Consequence: a user's time-bounded intent (e.g., "only execute this swap if it lands within the next block") becomes executable indefinitely until the nonce is consumed. Attackers / bundlers can hold expired ops and execute them under favorable market conditions, causing direct loss of user funds from the AtomWallet.
- **Preconditions:** Permissionless. Needs to observe a single time-bounded `UserOp` from a user whose nonce has not yet been consumed.
- **PoC sketch:**
  ```solidity
  // user signs userOp where signature = r||s||v || validUntil(=now+60) || validAfter(=0)
  // attacker waits until now > validUntil, then:
  bytes memory fakeSig = abi.encodePacked(r, s, v); // drop the 12-byte suffix → 65-byte path → validUntil=0
  userOp.signature = fakeSig;
  entryPoint.handleOps([userOp], payable(attacker)); // succeeds; same userOpHash, same recovered owner
  ```
- **Remediation:** Include the time-window bytes in the signed payload, e.g. sign `keccak256(abi.encodePacked(userOpHash, validUntil, validAfter))` (and apply the EIP-191 prefix to that). Then the EntryPoint-provided `userOpHash` still determines replay/nonce semantics while the time window is authenticated.

---

## Slither False Positives

- **arbitrary-send-eth (`_call`, `_payPrefund`)**: This is an ERC-4337 smart-account `execute`; calling arbitrary targets with value is the contract's whole purpose, gated by `onlyOwnerOrEntryPoint`. `_payPrefund` sending to `msg.sender == entryPoint` is per the 4337 spec.
- **calls-loop (`executeBatch`)**: Batch execution with per-call revert propagation is the intended semantics; DoS-in-loop concerns don't apply (caller is the owner/EntryPoint).
- **assembly**: All usages are storage-slot reads (ERC-7201 / OZ namespaced storage) and a standard low-level bubble-up revert. Correct.
- **pragma / solc-version**: Inherited from vendored dependencies; AtomWallet itself pins 0.8.29.
- **dead-code (Helpers.sol)**: Upstream account-abstraction lib; out of scope.
- **naming-convention / unused-state (`__gap`)**: `__gap` is storage padding for upgrade safety, deliberately unused.
- **low-level-calls**: Required for generic `execute` and 4337 prefund.

## High-confidence observations

- `owner()` override correctly falls back to `multiVault.getAtomWarden()` until `isClaimed`, and `acceptOwnership` permanently flips `isClaimed` → `_owner` persisted via `_transferOwnership`.
- ECDSA uses `tryRecover`, which rejects high-`s` malleability; explicit handling of `InvalidSignatureLength` / `InvalidSignatureS` / `InvalidSignature` branches is correct.
- `_packValidationData(true, ...)` on `InvalidSignature` returns failed validation (not revert), matching 4337 expectations so bundlers properly drop the op.
- Namespaced-storage slot constants match OZ's ERC-7201 derivations for `Ownable` and `Ownable2Step`, so reading them via `_getAtomWalletOwnerStorage` / `_getAtomWalletPendingOwnerStorage` is consistent with the inherited contracts.
- `receive()` + `addDeposit()` + `withdrawDepositTo` gated on `owner()||address(this)` — safe.
- `nonReentrant` on `execute`/`executeBatch`/`claimAtomWalletDepositFees` prevents cross-function reentrancy into the wallet during an outgoing call.
- 77-byte parser: `metaOffset = 65`, 12-byte meta packed as `uint96` → `validUntil` high 48 bits, `validAfter` low 48 bits — arithmetic is correct (separate from the authentication bug above).
- `transferOwnership` override preserves two-step semantics; pre-claim, only the current atomWarden can initiate transfer (because `onlyOwner` resolves through the overridden `owner()`).