diff --git a/audits/2026-06-18-executed-poc-results.md b/audits/2026-06-18-executed-poc-results.md new file mode 100644 index 00000000..a4428868 --- /dev/null +++ b/audits/2026-06-18-executed-poc-results.md @@ -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. diff --git a/audits/2026-06-18-findings.json b/audits/2026-06-18-findings.json new file mode 100644 index 00000000..cbc75349 --- /dev/null +++ b/audits/2026-06-18-findings.json @@ -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" + } + ] +} diff --git a/audits/2026-06-18-fuzz-invariant-results.md b/audits/2026-06-18-fuzz-invariant-results.md new file mode 100644 index 00000000..09326686 --- /dev/null +++ b/audits/2026-06-18-fuzz-invariant-results.md @@ -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. diff --git a/audits/2026-06-18-invariant-checklist.md b/audits/2026-06-18-invariant-checklist.md new file mode 100644 index 00000000..7f7fb353 --- /dev/null +++ b/audits/2026-06-18-invariant-checklist.md @@ -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. diff --git a/audits/2026-06-18-poc-tests.md b/audits/2026-06-18-poc-tests.md new file mode 100644 index 00000000..4f901fde --- /dev/null +++ b/audits/2026-06-18-poc-tests.md @@ -0,0 +1,101 @@ +# Zoltar Audit PoC Test Appendix + +These proof-of-concept tests are written as copy-ready additions to `solidity/ts/tests/peripherals.test.ts`. They are intentionally kept outside the production test tree as audit artifacts, but they use the repository's existing test helpers and should fail on commit `a49e15922ed91b317b969a08c67391c5296c0518`. + +## C-01: Truth-Auction ETH Is Stranded In SecurityPoolForker + +Insert this assertion block into the existing `simple truth auction: participant buys rep and can claim proceeds` test after the auction bid is submitted and before/after `finalizeTruthAuction`. + +```ts +const forkerAddress = getInfraContractAddresses().securityPoolForker +const forkerEthBeforeFinalize = await getETHBalance(client, forkerAddress) +const childEthBeforeFinalize = await getETHBalance(client, yesSecurityPool.securityPool) + +await mockWindow.advanceTime(7n * DAY + DAY) +await finalizeTruthAuction(client, yesSecurityPool.securityPool) + +const forkerEthAfterFinalize = await getETHBalance(client, forkerAddress) +const childEthAfterFinalize = await getETHBalance(client, yesSecurityPool.securityPool) +const childCollateralAfterFinalize = await getCompleteSetCollateralAmount(client, yesSecurityPool.securityPool) + +strictEqualTypeSafe( + forkerEthAfterFinalize - forkerEthBeforeFinalize, + expectedEthToBuy, + 'vulnerable behavior: truth-auction proceeds are paid to the forker', +) +strictEqualTypeSafe( + childEthAfterFinalize, + childEthBeforeFinalize, + 'vulnerable behavior: child pool receives none of the auction proceeds', +) +strictEqualTypeSafe( + childCollateralAfterFinalize, + childEthBeforeFinalize, + 'vulnerable behavior: child collateral is computed without auction proceeds', +) +``` + +Expected vulnerable result: + +- The forker balance increases by the filled auction ETH. +- The child pool ETH balance does not increase by the auction proceeds. +- `completeSetCollateralAmount` excludes the auction proceeds even though auction buyers later receive child-pool REP ownership. + +Expected patched result: + +- The child pool receives the auction ETH before `setPoolFinancials`. +- The forker does not retain the proceeds. +- `completeSetCollateralAmount` includes the ETH raised by the truth auction. + +## C-02: Own-Fork Excess Parent REP Is Stranded In SecurityPoolMigrationProxy + +Insert this as a new test near the existing own-fork tests. It uses the local `getMigrationProxyAddressAbi` helper already present in `peripherals.test.ts`. + +```ts +test('PoC C-02: own fork strands parent REP above fork threshold in the migration proxy', async () => { + const endTime = await getQuestionEndDate(client, questionId) + await mockWindow.setTime(endTime + 10000n) + + const forkThreshold = (await getTotalTheoreticalSupply(client, await getRepToken(client, securityPoolAddresses.securityPool))) / 20n / securityMultiplier + const excessVaultRep = 4n * forkThreshold + await depositRep(client, securityPoolAddresses.securityPool, excessVaultRep) + + await depositToEscalationGame(client, securityPoolAddresses.securityPool, QuestionOutcome.Yes, forkThreshold) + await depositToEscalationGame(client, securityPoolAddresses.securityPool, QuestionOutcome.No, forkThreshold) + + const poolRepBeforeFork = await getERC20Balance(client, getRepTokenAddress(genesisUniverse), securityPoolAddresses.securityPool) + const migrationProxyAddress = await client.readContract({ + abi: getMigrationProxyAddressAbi, + address: getInfraContractAddresses().securityPoolForker, + functionName: 'getMigrationProxyAddress', + args: [securityPoolAddresses.securityPool], + }) + + await forkZoltarWithOwnEscalationGame(client, securityPoolAddresses.securityPool) + + const forkData = await getSecurityPoolForkerForkData(client, securityPoolAddresses.securityPool) + const proxyParentRep = await getERC20Balance(client, getRepTokenAddress(genesisUniverse), migrationProxyAddress) + const proxyMigrationBalance = await getMigrationRepBalance(client, genesisUniverse, migrationProxyAddress) + + assert.ok(proxyParentRep > 0n, 'vulnerable behavior: migration proxy retains old-universe parent REP') + strictEqualTypeSafe( + proxyMigrationBalance, + forkData.auctionableRepAtFork, + 'forkData only reflects the Zoltar migration ledger, not leftover proxy REP', + ) + assert.ok( + poolRepBeforeFork > forkData.auctionableRepAtFork, + 'test setup should prove more parent REP was taken than was represented in child allocation', + ) +}) +``` + +Expected vulnerable result: + +- `proxyParentRep > 0`. +- `forkData.auctionableRepAtFork` equals only the Zoltar migration ledger balance created by `forkUniverse`. +- The old-universe parent REP left in the proxy is not split to child pools and is not recoverable through reviewed public functions. + +Expected patched result: + +- The migration proxy has no leftover old-universe REP after own-fork setup, or any leftover is explicitly represented by a documented and tested recovery bucket. diff --git a/audits/2026-06-18-qa-results.md b/audits/2026-06-18-qa-results.md new file mode 100644 index 00000000..9a68374b --- /dev/null +++ b/audits/2026-06-18-qa-results.md @@ -0,0 +1,115 @@ +# Zoltar Audit QA Results + +Date: 2026-06-18 + +Final branch head after merging latest `main`: `c420a3ac` + +## Executed Checks + +The following checks were run after the audit artifacts and package metadata fix were in place: + +```bash +bun run tsc +``` + +Result: passed. + +```bash +bun run test +``` + +Result: passed. + +Summary: + +```text +1313 pass +1 skip +0 fail +3614 expect() calls +Ran 1314 tests across 143 files. [118.62s] +``` + +```bash +bun run format +``` + +Result: passed. No fixes applied. + +```bash +bun run check +``` + +Result: passed. Biome reported no fixes, and Solidity Prettier check passed. + +```bash +bun run knip +``` + +Result: passed. + +## Additional Audit-Specific Validation + +The two temporary audit PoC tests were executed separately before removal from the source tree: + +```bash +bun test --timeout 300000 solidity/ts/tests/peripherals.test.ts -t "audit PoC C-0" +``` + +Result: + +```text +2 pass +124 filtered out +0 fail +Ran 2 tests across 1 file. [1295.00ms] +``` + +The audit artifacts were also checked for: + +- valid JSON in `audits/2026-06-18-findings.json`, +- ASCII-only content, +- no trailing whitespace. + +## Fuzz And Invariant Sweep + +The repository auction fuzz script was run: + +```bash +bun run test:auction-fuzz +``` + +Result: + +```text +2 pass +0 fail +Ran 2 tests across 1 file. [3.88s] +``` + +Temporary fork-accounting sweep tests were also inserted, executed, and removed: + +```bash +bun test --timeout 300000 solidity/ts/tests/peripherals.test.ts -t "audit accounting sweep" +``` + +Result: + +```text +2 pass +124 filtered out +0 fail +Ran 2 tests across 1 file. [2.31s] +``` + +The sweep covered three C-01 bid-size variants and three C-02 excess-REP variants. + +## Package Metadata Fix + +`bun run knip` initially reported unlisted imports of `@zoltar/shared/*` from UI files scanned by the root configuration. The root `package.json` now lists the local shared package: + +```json +"@zoltar/shared": "file:shared" +``` + +`bun.lock` was refreshed with `bun install`. After this metadata fix, `bun run knip` passed with zero warnings. diff --git a/audits/2026-06-18-remediation-verification.md b/audits/2026-06-18-remediation-verification.md new file mode 100644 index 00000000..7cc5850c --- /dev/null +++ b/audits/2026-06-18-remediation-verification.md @@ -0,0 +1,57 @@ +# Zoltar Audit Remediation Verification + +Date: 2026-06-18 + +Final branch head after merging latest `main`: `c420a3ac` + +## C-01 Status On Latest Main + +`origin/main` advanced after the original audit with commit `8d2a2aa0`, merged as `c420a3ac`, titled `Forward finalized truth-auction ETH to child security pool`. + +The merged code measures the `SecurityPoolForker` ETH balance delta around `truthAuction.finalize()` and forwards that ETH to the child `SecurityPool` before child pool financials are captured: + +```solidity +uint256 balanceBeforeFinalize = address(this).balance; +data.truthAuction.finalize(); +uint256 ethReceived = address(this).balance - balanceBeforeFinalize; +if (ethReceived > 0) { + (bool sent, ) = payable(address(securityPool)).call{ value: ethReceived }(''); + require(sent, 'truth auction ETH transfer failed'); +} +``` + +The repository test `simple truth auction: participant buys rep and can claim proceeds` now asserts: + +- child pool ETH increases by `expectedEthToBuy`, +- child pool `completeSetCollateralAmount` includes `expectedEthToBuy`, +- `SecurityPoolForker` does not retain truth-auction ETH. + +Command executed: + +```bash +bun test --timeout 300000 solidity/ts/tests/peripherals.test.ts -t "simple truth auction: participant buys rep and can claim proceeds" +``` + +Result: + +```text +(pass) Peripherals Contract Test Suite > simple truth auction: participant buys rep and can claim proceeds [359.06ms] + +1 pass +123 filtered out +0 fail +Ran 1 test across 1 file. [81.18s] +``` + +Conclusion: C-01 remains a valid critical finding against reviewed commit `a49e15922ed91b317b969a08c67391c5296c0518`, and it is verified as remediated on final branch head `c420a3ac`. + +## C-02 Status On Latest Main + +The latest `main` merge only changed the truth-auction ETH forwarding path and its test coverage. The own-fork migration-proxy REP accounting path reported in C-02 remains structurally unchanged in the reviewed code paths: + +- `SecurityPool.activateForkMode` still transfers the parent pool's full REP balance out of the pool. +- `SecurityPoolForker.forkZoltarWithOwnEscalationGame` still transfers the full received REP amount to the migration proxy. +- `Zoltar.forkUniverse` still consumes only `forkThreshold`. +- No post-own-fork call path was added to lock leftover proxy REP into the Zoltar migration ledger. + +Conclusion: C-02 remains open on final branch head `c420a3ac`. diff --git a/audits/2026-06-18-solidity-audit.md b/audits/2026-06-18-solidity-audit.md new file mode 100644 index 00000000..cf061fe2 --- /dev/null +++ b/audits/2026-06-18-solidity-audit.md @@ -0,0 +1,394 @@ +# Zoltar Solidity Security Audit + +## Target + +- Repository path: `/workspace/.t3/worktrees/zoltar/t3code-c5f51db1` +- Branch: `t3code/c5f51db1` +- Reviewed commit: `a49e15922ed91b317b969a08c67391c5296c0518` +- Final branch head after merging latest `main`: `c420a3ac` +- Date: 2026-06-18 +- Intended network: mainnet appears intended from `docs/mainnet-deployment-addresses.md`, while `solidity/default-config.json` only points at `https://ethereum.dark.florist`. This review treats the documented mainnet deployment manifest as the production reference and the JSON RPC URL as a local validation default. +- Solidity compiler: primary contracts use `pragma solidity 0.8.35`; `solidity/package.json` pins `solc` to `0.8.35`. +- Compiler settings: `solidity/ts/compile.ts` compiles main contracts with `viaIR: true`, optimizer enabled, `runs: 200`; no explicit EVM version is set for main contracts. OpenOracle settings use optimizer runs `50000` and EVM version `cancun`. +- Production constants reviewed: `Constants.GENESIS_REPUTATION_TOKEN = 0x221657776846890989a759BA2973e427DfF5C9bB`, `Constants.BURN_ADDRESS = 0xDeaDbeefdEAdbeefdEadbEEFdeadbeEFdEaDbeeF`, `NUM_OUTCOMES = 3`. +- Frozen protocol config from `docs/mainnet-deployment-addresses.json`: `forkThresholdDivisor = 20`, `forkBurnDivisor = 5`, `initialEscalationGameDeposit = 1000000000000000000`. +- Expected deterministic mainnet dependencies from `docs/mainnet-deployment-addresses.json`: `openOracle = 0x51DED022c087758c187ce636aa5f6adE6B919abB`, `zoltarQuestionData = 0xeadF91d12F549786891350B3D535638713651207`, `zoltar = 0xd282Ae3cC11423c740afD608e715CD4e22831A29`, `securityPoolForker = 0x290BF23Dd1912AdEDBdfd7419b85605C66e3d24B`, `securityPoolFactory = 0x0070464ef3Fb90B5D3e128e47D5ffB20e12f24E6`. + +## Executive Summary + +This review originally found two critical issues in the fork and truth-auction exit paths on reviewed commit `a49e15922ed91b317b969a08c67391c5296c0518`: + +- `C-01`: Truth-auction ETH is paid to `SecurityPoolForker` and never credited to the child pool, permanently stranding auction proceeds and finalizing the child pool with understated collateral. +- `C-02`: In the own-fork path, the forker moves all parent-pool and escalation-game REP to the migration proxy but `Zoltar.forkUniverse` consumes only the fork threshold. Any excess parent REP remains stranded in the proxy and is omitted from child REP allocation. + +Current status on latest merged `main` at `c420a3ac`: + +- `C-01`: Remediated. `SecurityPoolForker._consumeTruthAuctionRep` now forwards the ETH received from `truthAuction.finalize()` to the child security pool before collateral is captured, and the repository truth-auction test asserts this behavior. +- `C-02`: Open. The own-fork path still transfers all received REP to the migration proxy while `Zoltar.forkUniverse` only consumes `forkThreshold`; no latest-main change accounts for leftover raw parent REP in the proxy. + +The remaining open issue affects a required post-fork user-exit flow and can cause permanent stranding of material REP during normal protocol operation. + +## Scope Reviewed + +Files explicitly reviewed: + +- `audit_instructions.md` +- `solidity/default-config.json` +- `solidity/contracts/Constants.sol` +- `solidity/contracts/Zoltar.sol` +- `solidity/contracts/peripherals/SecurityPool.sol` +- `solidity/contracts/peripherals/SecurityPoolForker.sol` +- `solidity/contracts/peripherals/SecurityPoolForkerStorage.sol` +- `solidity/contracts/peripherals/SecurityPoolForkerTypes.sol` +- `solidity/contracts/peripherals/SecurityPoolForkerVaultMigrationBase.sol` +- `solidity/contracts/peripherals/SecurityPoolForkerVaultMigrationDelegate.sol` +- `solidity/contracts/peripherals/SecurityPoolMigrationProxy.sol` +- `solidity/contracts/peripherals/EscalationGame.sol` +- `solidity/contracts/peripherals/UniformPriceDualCapBatchAuction.sol` +- `solidity/contracts/peripherals/SecurityPoolOracleCoordinator.sol` +- `solidity/contracts/peripherals/factories/SecurityPoolFactory.sol` +- `solidity/contracts/peripherals/factories/PriceOracleManagerAndOperatorQueuerFactory.sol` +- `solidity/contracts/peripherals/factories/ShareTokenFactory.sol` +- `solidity/contracts/peripherals/factories/UniformPriceDualCapBatchAuctionFactory.sol` +- `solidity/contracts/peripherals/factories/SecurityPoolDeployer.sol` +- `docs/mainnet-deployment-addresses.md` +- `docs/mainnet-deployment-addresses.json` +- Relevant truth-auction and fork-migration tests in `solidity/ts/tests/peripherals.test.ts` and `solidity/ts/tests/auction.test.ts` + +Tools used: + +- Manual source review +- `rg`, `sed`, `nl`, `git` + +Review focus: + +- Zoltar fork initiation, migration balances, child universe deployment, and security-pool migration proxy use. +- Parent and child security-pool ETH/REP accounting across `Operational`, `PoolForked`, `ForkMigration`, and `ForkTruthAuction`. +- Truth-auction finalization, bid settlement, and child-pool ownership allocation. +- Escalation-game non-decision, fork continuation, carried deposits, and unresolved escrow export. +- Oracle-staged operations, liquidation snapshots, pending slots, and one-time coordinator initialization. + +No symbolic execution was performed. No production contract code changes were made by this audit. Copy-ready proof-of-concept test snippets are available in `audits/2026-06-18-poc-tests.md`, executed PoC results are available in `audits/2026-06-18-executed-poc-results.md`, accounting invariants plus remediation acceptance criteria are available in `audits/2026-06-18-invariant-checklist.md`, fuzz and invariant sweep results are available in `audits/2026-06-18-fuzz-invariant-results.md`, remediation verification is available in `audits/2026-06-18-remediation-verification.md`, and QA results are available in `audits/2026-06-18-qa-results.md`. + +Machine-readable findings are available in `audits/2026-06-18-findings.json`. + +## Findings + +### C-01: Truth auction proceeds are sent to `SecurityPoolForker` and never credited to the child pool + +Severity: Critical + +Status: Valid on reviewed commit `a49e15922ed91b317b969a08c67391c5296c0518`; verified remediated on final branch head `c420a3ac`. + +Impacted contracts and functions: + +- `UniformPriceDualCapBatchAuction.finalize` +- `SecurityPoolForker._consumeTruthAuctionRep` +- `SecurityPoolForker._captureUnclaimedCollateralForAuction` +- `SecurityPoolForker.receive` + +#### Summary + +This finding documents the vulnerable behavior on reviewed commit `a49e15922ed91b317b969a08c67391c5296c0518`. It is verified fixed on final branch head `c420a3ac`; see `audits/2026-06-18-remediation-verification.md`. + +Truth-auction ETH proceeds are paid to the auction `owner`, which is the `SecurityPoolForker`. The forker accepts ETH from trusted auction contracts, but it has no function that forwards those proceeds into the child `SecurityPool`. Immediately after finalizing the auction, the forker computes the child pool's collateral from `address(securityPool).balance`, which does not include the ETH just paid to the forker. + +As a result, winning bidders can pay ETH into the truth auction and receive child-pool REP ownership, while the ETH that should collateralize the child pool remains permanently stuck in the forker. The child pool is finalized with too little `completeSetCollateralAmount`, causing share redemptions and pool accounting to be undercollateralized. + +#### Evidence + +`UniformPriceDualCapBatchAuction.finalize` transfers the filled auction ETH to `owner`: + +```solidity +(bool sent, ) = payable(owner).call{ value: ethToSend }(''); +require(sent, 'Failed to send Ether'); +``` + +In protocol-created truth auctions, the auction owner is the `SecurityPoolForker`: + +```solidity +truthAuction = uniformPriceDualCapBatchAuctionFactory.deployUniformPriceDualCapBatchAuction( + address(securityPoolForker), + securityPoolSalt +); +``` + +`SecurityPoolForker._consumeTruthAuctionRep` finalizes the auction but does not collect or forward the ETH to the child pool: + +```solidity +if (data.truthAuction.auctionStarted() != 0) { + data.truthAuction.finalize(); + repPurchased = data.truthAuction.totalRepPurchased(); +} +securityPool.setSystemState(SystemState.Operational); +``` + +The next step, `_captureUnclaimedCollateralForAuction`, computes child collateral using only the child pool's ETH balance: + +```solidity +uint256 balance = address(securityPool).balance; +uint256 feesOwed = securityPool.totalFeesOwedToVaults(); +uint256 collateralAmount = balance >= feesOwed ? balance - feesOwed : 0; +securityPool.setPoolFinancials(collateralAmount, parentTotalSecurityBondAllowance); +``` + +The forker can receive the ETH: + +```solidity +receive() external payable { + require(trustedAuctionAddresses[msg.sender], 'fa'); +} +``` + +However, no reviewed function transfers the received auction ETH from `SecurityPoolForker` to the child `SecurityPool`. The tests also only assert that authorized auction ETH can increase the forker balance; they do not assert that finalized auction ETH becomes child collateral. + +Relevant source locations: + +- `solidity/contracts/peripherals/UniformPriceDualCapBatchAuction.sol:113-150` +- `solidity/contracts/peripherals/SecurityPoolForker.sol:467-501` +- `solidity/contracts/peripherals/SecurityPoolForker.sol:717-719` +- `solidity/contracts/peripherals/factories/SecurityPoolFactory.sol:101-104` + +#### Preconditions and attacker capabilities + +- A parent security pool has forked. +- A child pool reaches `ForkTruthAuction`. +- A bidder submits a winning or partially winning bid. +- Anyone calls `finalizeTruthAuction` after the auction duration. + +No privileged access or malicious external dependency is required. + +#### Exploit or failure scenario + +1. Users create complete sets in a parent pool, producing collateral that must be represented in the winning child pool after fork migration and truth auction settlement. +2. The parent pool forks and a child pool is created for one outcome. +3. Not all parent REP migrates into the child pool, so `startTruthAuction` starts a truth auction to sell child REP for the missing ETH collateral. +4. A bidder submits a bid that clears the auction. +5. After the auction ends, `SecurityPoolForker.finalizeTruthAuction(childPool)` calls `truthAuction.finalize()`. +6. `truthAuction.finalize()` sends `ethToSend` to the forker because the forker is the auction owner. +7. The forker does not forward this ETH to the child pool. +8. `_captureUnclaimedCollateralForAuction` sets the child pool's `completeSetCollateralAmount` from `address(childPool).balance`, excluding the auction proceeds. +9. Auction bidders can still claim purchased REP ownership through `claimAuctionProceeds`, but the ETH they paid is not backing child-pool shares or collateral accounting. +10. The ETH remains stuck in `SecurityPoolForker`, and child-pool share holders face permanent undercollateralization. + +#### Impact + +- Permanent loss or stranding of all ETH raised by a truth auction. +- Child pool finalizes with understated collateral. +- Winning auction bidders receive REP ownership without their ETH being credited to pool collateral. +- Share redemption and post-fork accounting can become materially insolvent. + +This is critical because it affects a required fork-exit path and can permanently strand significant ETH under normal protocol operation. + +#### Recommended remediation + +Make auction proceeds flow directly into the child pool's collateral before `setPoolFinancials` is called. Suitable fixes include: + +- Change the truth auction recipient so `finalize()` sends filled ETH directly to the child `SecurityPool`. +- Or have `SecurityPoolForker._consumeTruthAuctionRep` measure the forker's ETH balance delta from `truthAuction.finalize()` and immediately transfer that delta to the `securityPool`. + +After the transfer, assert the accounting invariant: + +```solidity +address(securityPool).balance >= securityPool.totalFeesOwedToVaults() + expectedCollateral +``` + +Also add an explicit no-stranded-ETH invariant for `SecurityPoolForker`, or a narrowly scoped recovery path that can only forward auction proceeds to the correct child pool. + +#### Proof-of-concept test guidance + +Copy-ready test code is provided in `audits/2026-06-18-poc-tests.md`. Executed PoC results are recorded in `audits/2026-06-18-executed-poc-results.md`. The reproduction extends `solidity/ts/tests/peripherals.test.ts` around the existing `simple truth auction: participant buys rep and can claim proceeds` scenario: + +1. Record `forkerBalanceBefore`, `childBalanceBefore`, and `expectedEthToBuy`. +2. Submit a bid that fully or partially clears the truth auction. +3. Call `finalizeTruthAuction`. +4. Assert the vulnerable behavior: + - `getETHBalance(forkerAddress) - forkerBalanceBefore == auctionEthRaised` + - `getETHBalance(yesSecurityPool.securityPool) - childBalanceBefore == 0` or otherwise excludes auction proceeds + - `getCompleteSetCollateralAmount(yesSecurityPool.securityPool)` does not include `auctionEthRaised` +5. After remediation, invert the assertions so the auction ETH is credited to the child pool and the forker balance does not retain it. + +The violated ETH-conservation invariant and fix acceptance criteria are documented in `audits/2026-06-18-invariant-checklist.md`. + +Remediation verification for the latest merged `main` branch is documented in `audits/2026-06-18-remediation-verification.md`. + +### C-02: Own-fork path strands parent REP above the Zoltar fork threshold in the migration proxy + +Severity: Critical + +Status: Open on final branch head `c420a3ac`. + +Impacted contracts and functions: + +- `SecurityPoolForker.forkZoltarWithOwnEscalationGame` +- `SecurityPool.activateForkMode` +- `SecurityPoolMigrationProxy.forkUniverse` +- `SecurityPoolMigrationProxy.lockRep` +- `Zoltar.forkUniverse` + +#### Summary + +This finding remains open on final branch head `c420a3ac`. + +The own-fork path transfers all REP held by the parent `SecurityPool` and all REP drained from the non-decision `EscalationGame` into a `SecurityPoolMigrationProxy`. The proxy then calls `Zoltar.forkUniverse`, but `Zoltar.forkUniverse` burns only `forkThreshold` REP and credits only `forkThreshold - forkThreshold / forkBurnDivisor` to the proxy's migration balance. + +If the parent pool plus escalation game hold more REP than the current Zoltar fork threshold, the excess parent REP remains in the migration proxy as the old-universe token. That excess is not added to Zoltar's migration ledger, is not included in `auctionableRepAtFork`, is not split to any child universe, and has no reviewed recovery path. Vault owners and escalation participants are then allocated child REP only from the reduced `auctionableRepAtFork` amount, while the remaining parent REP is permanently stranded. + +This is not limited to a contrived state. Existing test setups commonly deposit more than the fork threshold into the security pool before triggering an own fork, and production pools are expected to accumulate arbitrary vault REP above the minimum fork-triggering amount. + +#### Evidence + +`SecurityPool.activateForkMode` sends the parent pool's entire REP balance to the forker: + +```solidity +IERC20(address(repToken)).safeTransfer(msg.sender, repToken.balanceOf(address(this))); +``` + +`SecurityPoolForker.forkZoltarWithOwnEscalationGame` also drains all escalation-game REP, then transfers all REP received by the forker into the migration proxy: + +```solidity +uint256 poolRepToFork = rep.balanceOf(address(securityPool)); +securityPool.activateForkMode(); +uint256 escalationRepToFork = escalationGame.drainAllRep(address(this)); +... +uint256 repToFork = repBalanceAfter - repBalanceBefore; +if (repToFork > 0) IERC20(address(rep)).safeTransfer(address(migrationProxy), repToFork); +migrationProxy.forkUniverse(securityPool.questionId()); +``` + +`Zoltar.forkUniverse`, called through the proxy, consumes only `forkThreshold` REP and credits only the post-burn threshold amount to the caller's migration balance: + +```solidity +uint256 forkThreshold = getForkThreshold(universeId); +burnRep(universes[universeId].reputationToken, msg.sender, forkThreshold); +... +migrationRepBalances[msg.sender][universeId].migrationRepBalance = + forkThreshold - forkThreshold / forkBurnDivisor; +``` + +After the call returns, the forker records only the migration ledger amount as `auctionableRepAtFork` and derives vault and escalation child-REP buckets from that amount: + +```solidity +uint256 auctionableRepAtFork = zoltar.getMigrationRepBalance( + address(migrationProxy), + securityPool.universeId() +); +uint256 totalRepBeforeBurn = poolRepToFork + escalationRepToFork; +uint256 vaultRepAtFork = + totalRepBeforeBurn == 0 ? 0 : (poolRepToFork * auctionableRepAtFork) / totalRepBeforeBurn; +``` + +`SecurityPoolMigrationProxy` has a `lockRep` method that could add post-fork REP to the migration balance, but it is `onlyOwner`, and the reviewed `SecurityPoolForker` exposes no post-own-fork entry point that calls it for the leftover parent REP. + +Relevant source locations: + +- `solidity/contracts/peripherals/SecurityPool.sol:642-647` +- `solidity/contracts/peripherals/SecurityPoolForker.sol:568-605` +- `solidity/contracts/Zoltar.sol:80-98` +- `solidity/contracts/peripherals/SecurityPoolMigrationProxy.sol:33-40` + +#### Preconditions and attacker capabilities + +- A security pool has an escalation game that reached non-decision. +- The pool's REP balance plus the escalation game's REP balance exceeds `Zoltar.getForkThreshold(universeId)`. +- Anyone calls `forkZoltarWithOwnEscalationGame`. + +No malicious external dependency or privileged role is required. A normal vault-heavy pool is enough. + +#### Exploit or failure scenario + +1. Vaults deposit enough REP that the parent pool holds more than the fork threshold. +2. The escalation game reaches non-decision with additional REP escrowed. +3. Anyone calls `forkZoltarWithOwnEscalationGame`. +4. The forker moves all pool REP and all escalation-game REP into the migration proxy. +5. The proxy calls `Zoltar.forkUniverse`, which burns only `forkThreshold` and creates a migration balance of `forkThreshold - forkThreshold / forkBurnDivisor`. +6. The proxy still holds `poolRepToFork + escalationRepToFork - forkThreshold` old-universe REP. +7. `SecurityPoolForker` ignores that proxy token balance and allocates child migration buckets only from `auctionableRepAtFork`. +8. Future `migrateRepToZoltar`, `migrateVault`, and own-fork escalation claim paths split only those reduced buckets to children. +9. The excess old-universe REP remains in the migration proxy permanently, and users receive too little child REP relative to the REP taken from the pool and escalation game. + +#### Impact + +- Permanent stranding of parent REP in `SecurityPoolMigrationProxy`. +- Under-allocation of child REP to vault owners and escalation-game participants. +- Incorrect post-fork ownership, truth-auction sizing, collateral transfer, and escalation claim conversion because all are based on the reduced `auctionableRepAtFork`. +- The loss can be much larger than the fork threshold whenever a pool holds substantial vault REP. + +This is critical because it permanently removes user REP from all normal redemption and migration paths during an intended protocol fork. + +#### Recommended remediation + +Do not move more parent REP into the migration proxy than the amount the path will actually burn or lock into Zoltar's migration ledger. Viable fixes include: + +- Transfer only the exact `forkThreshold` REP needed for `forkUniverse`, leaving non-forking parent REP in the parent accounting path. +- Or, after `forkUniverse`, immediately call `migrationProxy.lockRep(leftoverProxyRep)` and include the resulting migration balance in `auctionableRepAtFork`. +- Or explicitly define and implement a separate recovery or redemption path for old-universe REP that remains after own-fork initiation. + +After the fix, add an invariant that the migration proxy has zero parent REP balance after own-fork setup, unless every remaining token is accounted for in a documented recovery bucket. + +#### Proof-of-concept test guidance + +Copy-ready test code is provided in `audits/2026-06-18-poc-tests.md`. Executed PoC results are recorded in `audits/2026-06-18-executed-poc-results.md`. The reproduction adds a test around an existing own-fork setup: + +1. Deposit `2 * forkThreshold` or more into the security pool. +2. Reach non-decision in the escalation game. +3. Call `forkZoltarWithOwnEscalationGame`. +4. Read `migrationProxyAddress = getMigrationProxyAddress(securityPool)`. +5. Assert the vulnerable behavior: + - `repToken.balanceOf(migrationProxyAddress) > 0` + - `repToken.balanceOf(migrationProxyAddress) == poolRepToFork + escalationRepToFork - forkThreshold`, ignoring any genesis burn-address transfer quirks + - `forkData.auctionableRepAtFork == forkThreshold - forkThreshold / forkBurnDivisor` +6. After remediation, assert that any leftover proxy REP is either zero or fully represented in an explicit migration or recovery accounting bucket. + +The violated REP-conservation invariant and fix acceptance criteria are documented in `audits/2026-06-18-invariant-checklist.md`. + +## Areas Partially Reviewed + +- Zoltar REP fork migration was reviewed for the security-pool proxy flow. The duplicate child-REP minting across fork outcomes appears intentional and was not reported as an issue. +- Oracle coordinator initialization was reviewed for front-running. The factory sets the pool in the same transaction after deployment, so no finding was raised. +- Auction settlement was reviewed for major accounting divergence. No separate finding was raised beyond `C-01`; minor rounding-order effects in underfunded pro-rata claims were not classified as material. +- Escalation-game MMR, nullifier proof, forked escrow, and unresolved export logic were reviewed for obvious double-claim and stuck-fund paths, but not formally verified. +- No symbolic-execution campaign was run. Auction tick-domain fuzzing and temporary fork-accounting sweep tests were run; deeper recursive fork fuzzing remains a residual recommendation. + +## Notable Non-Findings + +- `Zoltar.splitMigrationRep` can mint the same migration balance into multiple child outcomes. This is consistent with the documented fork semantics and was not classified as an over-mint by itself. +- `SecurityPoolOracleCoordinator.setSecurityPool` is externally callable before initialization, but the reviewed factory deploys the coordinator and sets the pool in the same transaction, leaving no mempool front-run window for factory-created pools. +- `ShareToken.authorize` is broad for already-authorized callers, but the origin factory and forker-mediated child authorization model intentionally relies on authorized pools/factory components. No arbitrary external caller can mint or burn share tokens without first being authorized by an existing authorized component. +- Failed staged oracle operations are consumed after execution attempt. This is a liveness/design tradeoff already covered by tests, not a direct fund-loss issue by itself. + +## Residual Risk + +The protocol has complex cross-contract accounting across forks, auctions, REP migration, collateral transfer, and escalation-game carry. The findings above show that the existing test suite can validate local REP allocation while missing global ETH and REP reconciliation. Before production deployment, add end-to-end invariants for: + +- Total ETH: parent pool, child pools, truth auctions, forker, refunds, and redeemed shares. +- Total REP: parent pool, migration proxy, child pools, escalation games, auction claims, and vault ownership. +- No stranded ETH or REP in coordinator/proxy contracts after finalization and migration flows. +- Fork migration conservation across own forks, external forks, unresolved escalation migration, and recursive child forks. + +## Confidence Assessment + +Confidence is high for the two reported findings because both are direct accounting breaks in reviewed production code paths, require no privileged attacker, and were reproduced with targeted temporary integration tests on the reviewed checkout. Additional temporary accounting sweeps reproduced the C-01 and C-02 invariant violations across multiple bid-size and excess-REP parameters, and the repository auction tick-math fuzz test passed across 2,000 deterministic ticks. + +The audit remains conservative: issues reviewed but not supported by a direct fund-loss or stuck-fund path were left as non-findings. The remaining quality gap is formal depth: remediation should commit the PoC and sweep scenarios as permanent regression tests with inverted assertions, and a follow-up property campaign should cover deeper recursive fork and mixed auction/migration orderings. + +## Validation + +Temporary audit PoC tests were inserted into `solidity/ts/tests/peripherals.test.ts`, executed, and removed. Both PoCs reproduced the vulnerable behavior on the reviewed checkout. + +Additional fuzz and invariant-style checks were run: + +- `bun run test:auction-fuzz`: passed, `2 pass`, `0 fail`. +- Temporary `audit accounting sweep`: passed, `2 pass`, `0 fail`, covering three C-01 bid-size variants and three C-02 excess-REP variants. + +The full repository QA sequence was run after audit artifact updates and the root package metadata fix: + +- `bun run tsc`: passed. +- `bun run test`: passed, `1313 pass`, `1 skip`, `0 fail` after merging latest `main`. +- `bun run format`: passed, no fixes applied. +- `bun run check`: passed. +- `bun run knip`: passed. + +The `knip` run initially exposed a root package metadata issue: UI files scanned by the root configuration import `@zoltar/shared/*`, while root `package.json` did not list the local shared package. The root manifest and `bun.lock` were updated with `"@zoltar/shared": "file:shared"`, after which `knip` passed with zero warnings. + +Audit artifacts were also validated for JSON parseability, ASCII-only content, and absence of trailing whitespace. diff --git a/bun.lock b/bun.lock index da78479a..d9576a34 100644 --- a/bun.lock +++ b/bun.lock @@ -5,6 +5,7 @@ "": { "dependencies": { "@preact/signals": "2.0.1", + "@zoltar/shared": "file:shared", "tevm": "1.0.0-next.149", "viem": "2.38.3", }, @@ -495,6 +496,8 @@ "@xtuc/long": ["@xtuc/long@4.2.2", "", {}, "sha512-NuHqBY1PB/D8xU6s/thBgOAiAP7HOYDQ32+BFZILJ8ivkUkAHQnWfn6WhL79Owj1qmUnoN/YPhktdIoucipkAQ=="], + "@zoltar/shared": ["@zoltar/shared@file:shared", { "dependencies": { "viem": "2.38.3" } }], + "abitype": ["abitype@1.1.0", "", { "peerDependencies": { "typescript": ">=5.0.4", "zod": "^3.22.0 || ^4.0.0" }, "optionalPeers": ["zod"] }, "sha512-6Vh4HcRxNMLA0puzPjM5GBgT4aAcFGKZzSgAXvuZ27shJP6NEpielTuqbBmZILR5/xd0PizkBGy5hReKz9jl5A=="], "acorn": ["acorn@8.16.0", "", { "bin": { "acorn": "bin/acorn" } }, "sha512-UVJyE9MttOsBQIDKw1skb9nAwQuR5wuGD3+82K6JgJlm/Y+KI92oNsMNGZCYdDsVtRHSak0pcV5Dno5+4jh9sw=="], diff --git a/package.json b/package.json index ead3b388..a644acef 100644 --- a/package.json +++ b/package.json @@ -17,6 +17,7 @@ }, "dependencies": { "@preact/signals": "2.0.1", + "@zoltar/shared": "file:shared", "tevm": "1.0.0-next.149", "viem": "2.38.3" },