Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions audits/2026-06-18-executed-poc-results.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# Zoltar Audit Executed PoC Results

Date: 2026-06-18

Target commit: `a49e15922ed91b317b969a08c67391c5296c0518`

## Setup Performed

The checkout initially had no `node_modules` directories and no generated contract artifacts. To execute the audit PoCs, the following setup was performed:

```bash
bun install --frozen-lockfile
cd solidity && bun install --frozen-lockfile
bun run ensure-contract-artifacts
```

`bun run ensure-contract-artifacts` regenerated shared build output and Solidity/UI contract artifacts needed by the test harness. These generated outputs are intentionally excluded from the audit report source review per repository policy.

## Temporary Test Execution

Two temporary tests were inserted into `solidity/ts/tests/peripherals.test.ts` using the code from `audits/2026-06-18-poc-tests.md`, then removed after execution. The tests asserted the vulnerable behavior directly:

- `audit PoC C-01: truth auction ETH is stranded in the forker`
- `audit PoC C-02: own fork strands parent REP above fork threshold in the migration proxy`

Command executed:

```bash
bun test --timeout 300000 solidity/ts/tests/peripherals.test.ts -t "audit PoC C-0"
```

Observed output:

```text
bun test v1.3.13 (bf2e2cec)

solidity/ts/tests/peripherals.test.ts:
(pass) Peripherals Contract Test Suite > audit PoC C-01: truth auction ETH is stranded in the forker [330.99ms]
(pass) Peripherals Contract Test Suite > audit PoC C-02: own fork strands parent REP above fork threshold in the migration proxy [83.42ms]

2 pass
124 filtered out
0 fail
Ran 2 tests across 1 file. [1295.00ms]
```

## Interpretation

The passing PoC tests confirm both reported issues reproduce on the reviewed checkout:

- C-01: filled truth-auction ETH increases `SecurityPoolForker` balance, while the child pool balance and child `completeSetCollateralAmount` exclude the auction proceeds.
- C-02: after own-fork setup with REP above the fork threshold, `SecurityPoolMigrationProxy` retains old-universe parent REP while `forkData.auctionableRepAtFork` reflects only the Zoltar migration ledger.

The temporary tests asserted vulnerable behavior on the reviewed commit, so they were not kept in the production test suite. During remediation, the same scenarios should be converted into regression tests with inverted assertions:

- C-01 should require auction proceeds to reach the child pool before `setPoolFinancials`.
- C-02 should require zero unaccounted raw parent REP in the migration proxy after own-fork setup, unless a documented recovery bucket is implemented and tested.
77 changes: 77 additions & 0 deletions audits/2026-06-18-findings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
{
"target": {
"repositoryPath": "/workspace/.t3/worktrees/zoltar/t3code-c5f51db1",
"branch": "t3code/c5f51db1",
"commit": "a49e15922ed91b317b969a08c67391c5296c0518",
"finalHeadAfterMainMerge": "c420a3ac",
"date": "2026-06-18"
},
"findings": [
{
"id": "C-01",
"severity": "Critical",
"status": "remediated_on_latest_main",
"reviewedCommitStatus": "valid",
"remediatedAtFinalHead": "c420a3ac",
"title": "Truth auction proceeds are sent to SecurityPoolForker and never credited to the child pool",
"impactedContracts": [
"UniformPriceDualCapBatchAuction",
"SecurityPoolForker"
],
"impactedFunctions": [
"UniformPriceDualCapBatchAuction.finalize",
"SecurityPoolForker._consumeTruthAuctionRep",
"SecurityPoolForker._captureUnclaimedCollateralForAuction",
"SecurityPoolForker.receive"
],
"sourceLocations": [
"solidity/contracts/peripherals/UniformPriceDualCapBatchAuction.sol:113",
"solidity/contracts/peripherals/SecurityPoolForker.sol:467",
"solidity/contracts/peripherals/SecurityPoolForker.sol:717",
"solidity/contracts/peripherals/factories/SecurityPoolFactory.sol:100"
],
"impact": "Truth auction ETH can be permanently stranded in SecurityPoolForker while child pools finalize with understated collateral.",
"recommendation": "Route auction proceeds directly to the child pool or transfer the forker balance delta to the child pool during finalization before setting pool financials.",
"pocArtifact": "audits/2026-06-18-poc-tests.md#c-01-truth-auction-eth-is-stranded-in-securitypoolforker",
"executedPocArtifact": "audits/2026-06-18-executed-poc-results.md",
"invariantArtifact": "audits/2026-06-18-invariant-checklist.md#c-01-truth-auction-eth-conservation",
"fuzzInvariantArtifact": "audits/2026-06-18-fuzz-invariant-results.md",
"remediationArtifact": "audits/2026-06-18-remediation-verification.md#c-01-status-on-latest-main",
"qaArtifact": "audits/2026-06-18-qa-results.md"
},
{
"id": "C-02",
"severity": "Critical",
"status": "open",
"reviewedCommitStatus": "valid",
"title": "Own-fork path strands parent REP above the Zoltar fork threshold in the migration proxy",
"impactedContracts": [
"SecurityPoolForker",
"SecurityPool",
"SecurityPoolMigrationProxy",
"Zoltar"
],
"impactedFunctions": [
"SecurityPoolForker.forkZoltarWithOwnEscalationGame",
"SecurityPool.activateForkMode",
"SecurityPoolMigrationProxy.forkUniverse",
"SecurityPoolMigrationProxy.lockRep",
"Zoltar.forkUniverse"
],
"sourceLocations": [
"solidity/contracts/peripherals/SecurityPool.sol:642",
"solidity/contracts/peripherals/SecurityPoolForker.sol:568",
"solidity/contracts/Zoltar.sol:80",
"solidity/contracts/peripherals/SecurityPoolMigrationProxy.sol:33"
],
"impact": "Parent REP above the fork threshold can remain permanently stuck in the migration proxy and omitted from child REP allocation.",
"recommendation": "Transfer only the REP needed for the fork or immediately lock leftover proxy REP into Zoltar's migration balance and include it in child allocation accounting.",
"pocArtifact": "audits/2026-06-18-poc-tests.md#c-02-own-fork-excess-parent-rep-is-stranded-in-securitypoolmigrationproxy",
"executedPocArtifact": "audits/2026-06-18-executed-poc-results.md",
"invariantArtifact": "audits/2026-06-18-invariant-checklist.md#c-02-own-fork-rep-conservation",
"fuzzInvariantArtifact": "audits/2026-06-18-fuzz-invariant-results.md",
"remediationArtifact": "audits/2026-06-18-remediation-verification.md#c-02-status-on-latest-main",
"qaArtifact": "audits/2026-06-18-qa-results.md"
}
]
}
87 changes: 87 additions & 0 deletions audits/2026-06-18-fuzz-invariant-results.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
# Zoltar Audit Fuzz And Invariant Results

Date: 2026-06-18

Final branch head after merging latest `main`: `c420a3ac`

## Auction Tick-Math Fuzz

Command executed:

```bash
bun run test:auction-fuzz
```

Result:

```text
solidity/ts/fuzz/auctionTickMath.fuzz.ts:
(pass) Auction tick math fuzz > tickToPrice matches the TypeScript model across deterministic fuzz ticks [960.10ms]
(pass) Auction tick math fuzz > tickToPrice rejects ticks outside the finite domain [2112.70ms]

2 pass
0 fail
Ran 2 tests across 1 file. [3.88s]
```

Coverage:

- 2,000 deterministic fuzz ticks across the finite auction tick domain.
- Explicit rejection checks for ticks below `MIN_TICK` and above `MAX_TICK`.

## Temporary Fork-Accounting Sweep

Two temporary integration tests were inserted into `solidity/ts/tests/peripherals.test.ts`, executed, and removed. The tests asserted the vulnerable accounting behavior directly across multiple parameter values.

Command executed:

```bash
bun test --timeout 300000 solidity/ts/tests/peripherals.test.ts -t "audit accounting sweep"
```

Result:

```text
solidity/ts/tests/peripherals.test.ts:
(pass) Peripherals Contract Test Suite > audit accounting sweep C-01: truth auction ETH remains stranded across bid sizes [1091.20ms]
(pass) Peripherals Contract Test Suite > audit accounting sweep C-02: own fork leaves excess parent REP in the migration proxy [301.81ms]

2 pass
124 filtered out
0 fail
Ran 2 tests across 1 file. [2.31s]
```

### C-01 Sweep

The temporary C-01 sweep ran the truth-auction stranded-ETH reproduction across three `repAtFork` purchase sizes:

- `repAtFork / 2`
- `repAtFork / 3`
- `repAtFork / 4`

For each variant it asserted:

- `SecurityPoolForker` ETH increased by the filled auction ETH.
- The child pool ETH balance did not increase.
- The child pool `completeSetCollateralAmount` excluded the filled auction ETH.

### C-02 Sweep

The temporary C-02 sweep ran the own-fork stranded-REP reproduction across three excess parent-REP levels:

- `2 * forkThreshold`
- `4 * forkThreshold`
- `8 * forkThreshold`

The simulator account was explicitly funded for each variant using the same storage-override technique used by the repository's test setup. For each variant it asserted:

- `SecurityPoolMigrationProxy` retained old-universe parent REP.
- `forkData.auctionableRepAtFork` matched only the Zoltar migration ledger.
- The raw parent REP left in the proxy was not represented in fork accounting.

## Interpretation

The fuzz run increases confidence that the latest auction tick-domain changes are not the source of the reported auction issue. The fork-accounting sweep increases confidence that both critical findings are invariant violations rather than narrow single-parameter examples.

The temporary tests were removed after execution because they assert current vulnerable behavior. Remediation should convert these scenarios into permanent regression tests with inverted assertions.
129 changes: 129 additions & 0 deletions audits/2026-06-18-invariant-checklist.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
# Zoltar Audit Invariant Checklist

This checklist records the accounting invariants used to validate the two reported findings and to judge whether a proposed remediation is complete. It is scoped to the reviewed fork, migration, and truth-auction flows.

## C-01: Truth-Auction ETH Conservation

### Broken invariant

After `finalizeTruthAuction(childPool)` completes, all filled truth-auction ETH must be either:

- credited to the child pool's ETH balance and included in `completeSetCollateralAmount`, or
- refunded to bidders.

The vulnerable implementation leaves filled ETH at `SecurityPoolForker`, which is neither the child collateral holder nor a documented refund holder.

### Concrete reconciliation

Before finalization:

```text
forkerEthBefore = ETH(SecurityPoolForker)
childEthBefore = ETH(childPool)
auctionEthFilled = min(ethRaised, ethRaiseCap)
```

Required after finalization:

```text
ETH(SecurityPoolForker) == forkerEthBefore
ETH(childPool) >= childEthBefore + auctionEthFilled
completeSetCollateralAmount(childPool) >= childEthBefore + auctionEthFilled - feesOwed(childPool)
```

Observed vulnerable behavior:

```text
ETH(SecurityPoolForker) == forkerEthBefore + auctionEthFilled
ETH(childPool) == childEthBefore
completeSetCollateralAmount(childPool) excludes auctionEthFilled
```

### Existing test anchor

`solidity/ts/tests/peripherals.test.ts` has `simple truth auction: participant buys rep and can claim proceeds`. That test already:

- creates open interest,
- triggers an external security-pool fork,
- migrates the parent vault into the `Yes` child,
- starts a truth auction,
- submits a clearing bid,
- finalizes the auction,
- verifies the bidder receives child-pool ownership.

It does not assert where the filled ETH went. The C-01 PoC adds exactly that missing assertion.

### Fix acceptance criteria

A complete fix should make all of these true:

- `finalizeTruthAuction` cannot leave filled auction ETH in `SecurityPoolForker`.
- The child pool receives filled auction ETH before `setPoolFinancials`.
- The child pool's `completeSetCollateralAmount` includes the received proceeds net of fees.
- Any recovery path for stranded ETH is child-pool-specific and cannot redirect proceeds to an arbitrary receiver.

## C-02: Own-Fork REP Conservation

### Broken invariant

After `forkZoltarWithOwnEscalationGame(parentPool)` completes, every unit of parent-universe REP taken from the parent pool or escalation game must be represented in one of these buckets:

- burned by `Zoltar.forkUniverse`,
- credited to `Zoltar` migration balance,
- retained in parent-pool accounting with a documented redemption path,
- or assigned to an explicit recovery bucket with a tested owner and destination.

The vulnerable implementation sends all pool and escalation REP to `SecurityPoolMigrationProxy`, but `Zoltar.forkUniverse` consumes only `forkThreshold`. Excess old-universe REP remains as a raw token balance in the proxy and is not included in `auctionableRepAtFork`.

### Concrete reconciliation

Before own fork:

```text
poolRepToFork = REP(parentPool)
escalationRepToFork = REP(escalationGame)
totalRepSentToProxy = poolRepToFork + escalationRepToFork
forkThreshold = Zoltar.getForkThreshold(parentUniverse)
postBurnMigrationRep = forkThreshold - forkThreshold / forkBurnDivisor
```

Required after own fork:

```text
REP(parentUniverse, migrationProxy) == 0
auctionableRepAtFork >= postBurnMigrationRep
all REP sent to proxy is either burned, migration-ledgered, or recoverably accounted
```

Observed vulnerable behavior when `totalRepSentToProxy > forkThreshold`:

```text
REP(parentUniverse, migrationProxy) == totalRepSentToProxy - forkThreshold
auctionableRepAtFork == postBurnMigrationRep
leftover proxy REP is omitted from vaultRepAtFork and escalation child-REP buckets
```

### Existing test anchor

`solidity/ts/tests/peripherals.test.ts` has `migration proxy balances match the expected lock and sweep flow`, which correctly checks that external-fork initiation does not leave parent REP in the proxy after `lockRep`.

Own-fork tests such as `own-fork unlocked vault migration values child ownership against the vault REP bucket` already create the more dangerous setup with extra vault REP and escalation REP, but they validate only internal bucket consistency after the reduced `auctionableRepAtFork` has been recorded. The C-02 PoC adds the missing proxy parent-REP balance assertion immediately after `forkZoltarWithOwnEscalationGame`.

### Fix acceptance criteria

A complete fix should make all of these true:

- `SecurityPoolMigrationProxy` has zero raw parent-universe REP after own-fork setup, unless the remainder is in an explicit tested recovery bucket.
- `auctionableRepAtFork` includes every non-burned unit of REP that should be split into child pools.
- `vaultRepAtFork + unallocatedEscrowChildRep` reconciles to the accounted child-REP amount after burn treatment and rounding.
- Own-fork and external-fork migration proxy tests both assert zero unaccounted parent REP after setup.

## Cross-Flow Regression Tests

Add or keep regression tests covering:

- External fork, partial migration, truth auction with a clearing bid, then finalization and auction proceeds claim.
- Own fork with `poolRepToFork + escalationRepToFork > forkThreshold`, then immediate proxy-balance reconciliation.
- Own fork with all vault REP migrated to one child, then child REP and child ETH reconciliation.
- Own fork with split escalation claims in multiple orders, verifying child REP allocation is order independent and globally conserved.
- Recursive child fork after a parent truth auction, verifying no ETH or REP remains stranded in the old forker or proxy contracts.
Loading
Loading