# Kitty contracts — security audit 2026-11-08

- **Commit audited:** `4c4f991` (branch `main`, repo `~/repos/kitty`)
- **Fix commits:** `393b574` (round 1), `80f5a4a` (round 2, after fix review)
- **Method:** contract-audit skill: scope → system map (`SYSTEM.md`) → invariants → blind hunt with sc-vuln-db prior art → Foundry PoC for every Medium+ → fix → independent fix review (Phase 7) → second fix round → regression tests.
- **Result:** 0 Critical, 2 High, 2 Medium, 4 Low, 6 Info. The fix review found 3 more issues (1 Medium, 2 Low) in the round-1 fixes. All are fixed except the items listed as accepted. 52/52 tests pass: unit, fuzz, invariant, audit regressions and fix-review regressions.

## Scope

| File | Notes |
|---|---|
| `src/KittyRound.sol` | per-round pot: pledges, guards, step buys, pro-rata claims |
| `src/KittyFactory.sol` | deploys rounds, validates config, router/base allowlists |
| `src/TokenRegistry.sol` | curated verified token list |
| `src/interfaces.sol` | V2 + registry interfaces |
| `src/dex/KittyDex.sol` | testnet V2-shaped AMM (reviewed only for how rounds use it) |

Not in scope: the web app, server and off-chain rooms; production chains and routers (none yet; no fork test); token economics. `aderyn` was not available. Slither ran on KittyRound and every result was triaged out. The fan-out threshold was not reached (~470 nSLOC), so a single auditor ran every pass. Prior-art queries: wrong-slippage-check, sandwich, fake token, ticker impersonation, `int-0006`.

## Findings

| ID | Severity | Title | Status |
|---|---|---|---|
| KR-1 | High | In-tx slippage/liquidity checks; admin can sandwich their own round | Fixed (mandatory `maxPrice`), residual accepted |
| KR-2 | High | `reviewWindow = 0` allowed for open targets → same-block impostor buy | Fixed |
| KR-3 | Medium | Flash pledge→veto→claim cancels any round for free | Fixed (round 2: veto weight snapshot) |
| REG-1 | Medium | Registry owner has live power over existing rounds | Fixed |
| KR-4 | Low | Admin stall locks funds until `executeDeadline` | Fixed |
| KR-5 | Low | Re-propose resets vetoes; veto via withdrawals not enforced | Fixed (round 2: no re-proposing any earlier target) |
| KR-6 | Low | Mutable `symbol()` halts ticker rounds mid-execution | Fixed |
| INT-1 | Low | Testnet owned by public anvil key; broadcast not ignored | Fixed |
| FR-1 | Medium | (fix review) One-block capital veto, and withdraw auto-cancel grief | Fixed |
| FR-2 | Low | (fix review) `vetoBps = 10000` lets one holdout block every veto | Fixed (cap 70%) |
| FR-3 | Low | (fix review) Same-second listing and re-listing bypass the registry snapshot | Fixed |
| KR-7 | Info | First step can run before `pledgeDeadline`, which closes pledging | Documented, shown in UI |
| KR-8 | Info | Rounding dust / donations stay in the round | Accepted |
| KR-9 | Info | Fee-on-transfer / rebasing targets are unsupported | Documented |
| KR-10 | Info | `walletCap` is per address | Documented |
| FAC-1 | Info | Factory owner is a single key, no timelock (new rounds only) | Documented |
| DEX-1 | Info | KittyDex matches V2 for every call rounds make | None needed |

### KR-1 (High): admin self-sandwich
**Where:** `KittyRound.executeStep` (slippage floor vs `router.getAmountsOut` in the same tx; `minLiquidity` from live reserves). `KittyFactory._validate` allowed `maxPrice = 0`.
**Attack:** the admin pumps the pair with their own or flash-loaned capital, calls `executeStep` (the floor check passes against the pumped quote), then dumps. PoC on the audited commit: on a 300k pot the admin made 277,409 USDC profit and members got 7% of the fair token amount. A third-party bot could likewise take up to `maxSlippageBps`.
**Fix:** `maxPrice` is mandatory (`BadConfig("max price")`). Every step reverts `PriceTooHigh` if its average fill is above the ceiling. In round 2 the ceiling moved to raw units (`PRICE_SCALE = 1e36`), so it no longer depends on the target's `decimals()` and is precise for micro-priced tokens.
**Residual (accepted):** an admin can still pick any ceiling, even `type(uint256).max`. That is shown to members before they pledge, and the UI flags a ceiling above 3× spot. A V2 TWAP check isn't possible on KittyDex (no cumulative prices) and should be added for production routers.
**Regression:** `test/audit20261108/KR1_adminSelfSandwich.t.sol`.

### KR-2 (High): zero review on open targets
**Where:** `KittyFactory._validate`. **Attack:** in a TICKER-only round with `reviewWindow = 0`, the admin deploys a fake "PEPE", proposes it and buys it in one block, then sells the retained supply (PoC: admin netted 299,999 of 300k).
**Fix:** when `GUARD_ADDRESS` is unset the factory requires `reviewWindow ≥ MIN_REVIEW (10 min)` and `vetoBps > 0`. **Regression:** `KR2_tickerImpostorZeroReview.t.sol`.

### KR-3 / FR-1 (Medium): free veto cancellation
**Where:** `veto()`. **Attack (audited commit):** pledge, veto, claim in one tx with flash-loaned funds. This cancelled any round at no cost.
**Round-1 fix:** the veto required a pledge from an earlier block. **The fix review broke it:** capital held for a single block still worked, and a non-vetoing whale could tip a veto just by withdrawing (auto-cancel).
**Round-2 fix:** veto weight is the pledge a member already held when the current target was named (lazy per-member snapshot). The threshold is measured against `eligibleVetoTotal`, the pot as it stood at proposal minus later withdrawals of that money. Late money can't veto and can't dilute vetoes. Withdrawals never auto-cancel; a passed veto blocks `executeStep` and opens a permissionless `cancel()`. Fixed-address rounds have no veto (the factory requires `vetoBps = 0`), because members knew the target when they pledged and can withdraw.
**Regressions:** `KR3_flashPledgeVeto.t.sol`, `test_lateMoneyHasNoVetoAndCannotDilute`, `test_vetoPassedByWithdrawals_cancelOpensAndExecuteBlocked`, `fixreview20261108/*KR3*`.

### REG-1 / FR-3 (Medium): registry live control
**Where:** the registry was re-read on every proposal and step. **Impact:** a revoke mid-execution froze the round until the deadline, and a late listing opened new targets in old rounds.
**Fix:** the registry records `verifiedAt`. A round accepts only tokens listed strictly before its `createdAt`, and target checks (ticker + registry) run only at proposal and at the first step. Re-listing an already listed token reverts `AlreadyListed`. A revoke before the first buy still blocks that buy, which is the intended curator safety lever; members can withdraw.
**Regressions:** `REG1_registryLiveControl.t.sol`, `test_registry_relistReverts_sameSecondListingRejected`, `fixreview20261108/*REG1*`.

### KR-4 (Low): stall
Anyone may `finalize()` once `lastStepAt + stepCooldown + STALL_GRACE (1 day)` has passed. Test: `test_stalledAdmin_anyoneFinalizesAfterGrace`, `test_KR4_stallFinalize`.

### KR-5 (Low): veto reset by re-proposal
A target can never be proposed twice (`wasProposed`), which also blocks A→B→A. Tests: `test_cannotReproposeAnyEarlierTarget`, `test_KR5_fixed_ABAreverts`.

### KR-6 (Low): mutable symbol
Covered by checking only at proposal and the first step.

### INT-1 (Low): public deployer key
The testnet is deployed with a freshly generated private key, kept only in `/srv/kitty/kitty.env` (mode 600) on Strato. Anvil's default accounts come from a private mnemonic, and the public RPC proxy blocks `anvil_*`, `eth_sendTransaction` and every non-allowlisted method. `contracts/broadcast/` is gitignored.

### FR-2 (Low): veto cap
`vetoBps ≤ 7000`.

## Promises vs code (summary; full table in SYSTEM.md)

| Promise | Holds? |
|---|---|
| Admin can only spend the pot by buying the target via the allowlisted router, path [base, target] | Yes |
| Bought tokens go to the round, never the admin | Yes |
| Same average price for every member | Yes (pro-rata on `startTotal`; invariant-tested) |
| Withdraw any time before the first buy | Yes |
| Schedule fixed at creation | Yes |
| Ticker lock / verified list / exact address enforced | Yes (at proposal + first buy) |
| Honeypot target can't trap the refund | Yes (`claimRefund`) |
| Factory owner has no power over existing rounds | Yes |
| Registry owner has no power over running rounds | Yes after fix (can block before the first buy only) |
| Admin can't extract value via timing | Bounded by `maxPrice` only (residual KR-1) |

## Centralisation
- **Factory owner:** allowlists routers and base tokens for new rounds. Single key on testnet; use a multisig + timelock for production.
- **Registry curator:** lists and delists tokens; can block a round's first buy by delisting. Should be a multisig.
- **Room admin:** picks timing and `minOut` within slippage and `maxPrice`; names the target in open rounds (subject to review + veto).
- **Router:** fully trusted. Only allowlisted routers are accepted.

## Refuted leads
Reentrancy (every mutating function is `nonReentrant`), double claims, `vetoWeight` underflow, rounding overpay (fuzzed), cancelling an Executing round, bytes32 `symbol()` parsing, immutables read in the constructor.