Skip to content

Audi fix: Pre-IPO - #120

Merged
razww merged 1 commit into
feat/pre-ipo-claimfrom
fix/pre-ipo-audit
Jul 28, 2026
Merged

Audi fix: Pre-IPO#120
razww merged 1 commit into
feat/pre-ipo-claimfrom
fix/pre-ipo-audit

Conversation

@razww

@razww razww commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

Addresses the HashDit security audit of PreIPODistributor (subscription #118 + settlement/claim #119). Applies the accepted code fixes, records the disposition of every finding, and adds the audit report to the repo. Stacked on feat/pre-ipo-claim (#119) — please set that as the base branch.

Fixes applied (code)

  • M02setPublicRound now reverts ("Settlement started") once a settlement root is pending or finalized, so a public round can't be opened after the settlement snapshot and strand late depositors.
  • I02 — removed the redundant _root != s.pendingRoot check in setSettlementRoot (already covered by the pendingRoot == 0 requirement).
  • I03pause is now a real circuit breaker: require(!sale.paused) added to setSettlementRoot, finalizeSettlement, and claim (previously only deposits were gated).
  • I04 — documented emergencyWithdraw in NatSpec: it does not adjust internal accounting and can strand refunds/claims; totals must be restaged (or the contract upgraded) before claims resume.
  • I09 — cached the duplicate deposits[saleId][msg.sender] storage read in depositWhitelist (single SLOAD).
  • I10 / I14 — completed claim parameter docs; refreshed the stale contract header; documented the tranche constants' delivery semantics and that claim is permissionless (funds always go to the leaf's _account).
  • I13 — added previewClaim(...) read-only view returning (valid, alreadyClaimed, tranche, sendShares), mirroring claim validation/routing.

Not changed (with rationale)

  • M01 (share delivery could drain another sale's deposit backing) — acknowledged; only occurs when concurrent sales share a token. This launch is a single sale, USDT deposits, distinct share token. Operational rule: never set a share token equal to any live sale's deposit token.
  • L03 (setWaitingPeriod could retroactively shorten an in-flight review window) — handled operationally (MANAGER won't change waitingPeriod while a settlement is in flight); global waitingPeriod makes a per-sale on-chain guard awkward.
  • I08 (pack paused to save a slot) / I11 (rename depositswlDeposits) — won't fix: both change the storage layout / public ABI of the already-deployed subscription contract and are not upgrade-safe.
  • L01, I01, I05, I06, I07, I12 — acknowledged (by design or covered by trusted roles / minDeposit); see the response notes.

Testing

forge test --match-contract PreIPODistributorTest — 56 passing, including new cases: setPublicRound after pending/finalized settlement reverts, pause blocks settlement and claim, and previewClaim behavior (pre/post finalize, wrong amount, post-claim).

Also included

  • audits/HashDit-PreIPODistributor-20260727.pdf — the audit report.

🤖 Generated with Claude Code

- M02: setPublicRound reverts once a settlement root is pending/finalized
- I02: remove redundant root inequality check in setSettlementRoot
- I03: pause now gates setSettlementRoot / finalizeSettlement / claim
- I04: document emergencyWithdraw accounting caveat (NatSpec)
- I09: cache duplicate deposit storage read in depositWhitelist
- I10/I14: complete claim param docs, header, tranche + permissionless-claim docs
- I13: add previewClaim read-only helper

L03 handled operationally; M01/L01/I01/I05-I08/I11/I12 acknowledged; L02 pending xKLSH address.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@hashdit-bot

hashdit-bot Bot commented Jul 28, 2026

Copy link
Copy Markdown

Pull Request Review

This Solidity UUPS upgrade adds a two-step Merkle-based settlement and claim lifecycle, pause enforcement, claim previews, and protections against opening public rounds after settlement begins. It also updates documentation and expands Foundry tests covering settlement, claims, access control, and pause behavior, while adding the associated audit report.

Sensitive Content

No sensitive content detected.

Security Issues

🟠 [HIGH] Arbitrary share token delivery can drain assets backing other sales

File: contracts/dao/PreIPODistributor.sol
The newly added claim function accepts _shareToken from the settlement leaf and transfers that token without checking it against a share token configured for the sale. A compromised or erroneous BOT-generated root can therefore authorize an unlocked account to withdraw any ERC-20 held by the shared contract, including deposit tokens or share tokens backing other sales, potentially making their refunds or allocations insolvent. The operational restriction described in the PR does not enforce this invariant on-chain.
Recommendation: Configure and store the expected share token per sale, require _shareToken to match it, and maintain per-sale token liabilities or segregated escrow balances. At minimum, prevent share delivery from using any active sale’s deposit token and ensure claims cannot consume balances reserved for another sale.


Generated by Hashdit Bot. This tool can absolutely NOT replace manual audits.

@razww
razww changed the base branch from master to feat/pre-ipo-claim July 28, 2026 06:20
@razww
razww merged commit a10a14e into feat/pre-ipo-claim Jul 28, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant