Contracts internal security audit #424
Labels
No labels
agent:fix_bugfix
agent:fix_conflicts
agent:fix_security
agent:gap_analysis
agent:implement
agent:implement
agent:implement
agent:open_issues
agent:ready
agent:research
agent:security_audit
agent:verify
architecture
backend
blocker:hybrid
blocker:launch
blocker:limit-orders
blocker:v2
block:log_only
block:security
bug
ci
contracts
correctness
deploy
dev
devops
docs
documentation
duplicate
e2e
enhancement
epic
feature
frontend
functional-completion
gas
good first issue
governance
help wanted
high-risk
hooks
hybrid
indexer
infra
infrastructure
integrators
invalid
launch-blocker
limit-orders
localnet
localterra
low priority
missing-implementation
needs-design
ops
performance
priority
high
priority
medium
product
qa
QA
question
ready
ready
research
scripts
security
security-hardening
smartcontracts
tech-debt
testing
ux
UX
v2
verification
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
code/cl8y-dex-terraclassic#424
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Full security reviw and audit of the smart contacts, inspecting coverage, fuzz testing, adversarial scenario tests, common exploits, access control, liquidity drainage/order book drainage via looped trading, traders receiving excess funds (must not), rounding delivering more tokens to traders than expected, boundary conditions, and more - resea4ch and list more areas to cover first, then complete the audit
Internal Full-Stack Security Audit — GitLab #424
1782692388main@ workspace snapshotExecutive summary
CL8Y DEX is a mature, defense-in-depth Terra Classic DeFi stack. Under the documented trust model (honest governance, CW20 whitelist, treasury as sink), no critical on-chain fund-drain or unauthorized-privilege bugs were identified in this pass. Core economic invariants — k-monotonicity, pool-favorable rounding, escrow accounting, hook caller allowlists, router hop deltas, hybrid limit-book matching, and access control — are extensively tested (388 contract lib tests, 7 audit-invariant regressions, 21
security_tests, 26 fuzz/proptest cases; all passed locally).The highest residual risks are operational and governance-centric: single-key factory control, pause trapping limit escrow, absence of a third-party audit, and off-chain quote/indexer trust boundaries. Several historical gap items (LCD error leakage, router dust sweep, indexer cursor-on-error, tx pagination,
parseFloatonmin_received) are fixed and regression-tested in the current tree.Launch recommendation: acceptable for capped-TVL / pool-first rollout with documented controls; not a substitute for external audit + multisig governance + completed launch checklist items (#391, #407, #408, #410).
Areas analyzed
smartcontracts/contracts/{factory,pair,router,fee-discount,hooks/*}smartcontracts/packages/dex-commonmax_spread, oracle, hook settlement, blacklist typessmartcontracts/tests/src/{lib.rs,adversarial_token.rs,limit_order_tests.rs,blacklist_tests.rs,migration_tests.rs}indexer/src/indexer/{poller,block_indexer,parser,pair_discovery}.rsindexer/src/api/*,indexer/tests/security.rsfrontend-dapp/src/{pages,services,utils,hooks}.gitlab-ci.yml,scripts/ci/gitleaks-*,smartcontracts/.cargo/audit.tomldocs/{security-model,contracts-security-audit,exploit-replay-matrix}.mdfrontend-dapp/package.json,smartcontracts/Cargo.tomlcosmesAdditional areas beyond the issue text: trading blacklist, batch limit placement/cancel, permissionless book clean, wrap-mapper/treasury harness, reorg webhook, CG/CMC aggregator compliance routes.
Methodology and test execution
docs/contracts-security-audit.md,docs/security-model.md,docs/exploit-replay-matrix.md,gaps/GAP_1780200149.md).cd smartcontracts && cargo test --libcargo test audit_invariantcargo test security_testscargo test fuzz_testscd indexer && cargo test --libsecurity.rsintegrationindexer/.env); not run in this VMindexer/tests/security.rsnpm audit --audit-level=highon frontend (12 advisories, 2 high).cargo auditnot installed locally; CI enforces via.gitlab-ci.ymlcargo-audit-smartcontracts/cargo-audit-indexer.Findings
H-01 — Governance centralization (no on-chain timelock)
smartcontracts/contracts/factory/src/contract.rs(ensure_governance);docs/security-model.mdgovernanceaddress can change fees, register reverting hooks, pause pairs, redirect treasury, mutate CW20 whitelist, push discount registry, and manage trading blacklists — atomically and without delay.SetPairFee,RegisterHook,UpdateConfig— all succeed from governance only (audit_invariant_tests::p8_*).H-02 — Emergency pause blocks all maker escrow exits
smartcontracts/contracts/pair/src/contract.rs—assert_not_pausedonReceive,CancelLimitOrder,ClaimExpiredLimitOrder,CleanLimitBook(invariant L6)limit_order_tests::pause_blocks_swap_and_place_cancel_refunds_escrow;claim_expired_limit_order_blocked_while_pair_paused_then_succeeds_after_unpause.H-03 — No third-party security audit
docs/security-model.md;docs/contracts-security-audit.md§ Third-party auditM-01 — Fee-discount cache TTL (300s) allows bounded discount leakage
smartcontracts/contracts/pair/src/discount_cache.rs; invariant P9(effective_fee_bps, discount)forDISCOUNT_CACHE_TTL_SECONDS(300s). Trader who drops below tier CL8Y balance may retain cached discount until expiry.docs/contracts-security-audit.mdP9;fee_discount_coverage_tests.M-02 — Fee-discount registry outage charges full fee silently
smartcontracts/contracts/pair/src/discount_cache.rs—Err(_) => (fee_bps, None); invariant P5GetDiscountquery fails (paused registry, migration, LCD error), swaps succeed at full pair fee with no on-chain revert.audit_invariant_tests::swap_uses_full_fee_when_discount_registry_query_fails; indexerGET /api/v1/health/fee-discount.M-03 — Registered reverting hook is a liveness kill-switch
smartcontracts/contracts/pair/src/contract.rs(post-swapadd_messagesto hooks); invariant H1Errrolls back the entire swap atomically.audit_invariant_tests::swap_fails_atomically_when_allowlisted_hook_reverts.M-04 — Indexer
/healthis shallow (no DB/LCD readiness)indexer/src/api/mod.rs:368-370GET /healthreturns{"status":"ok"}without probing Postgres or LCD.curl /healthon any running instance — always 200./readywith DB ping + LCD probe; keep/healthlightweight or document K8s probe split.M-05 — Public Swagger UI / OpenAPI on indexer
indexer/src/api/mod.rs:499—SwaggerUi::new("/swagger-ui")merged into production routerindexer/tests/security.rs::swagger_ui_available,openapi_spec_available.RUN_MODE=prod; rate limits already mitigate abuse (#363).M-06 — Production CSP allows
script-src 'unsafe-inline'frontend-dapp/viteCsp.ts:50'unsafe-inline'for scripts (Vite constraint).index.htmlCSP in production build.connect-src(already env-scoped).M-07 — npm supply chain: 2 high-severity advisories
frontend-dapp/package.json— transitivews@7.x(GHSA-96hv-2xvq-fx4p);@goblinhunt/cosmes→@dao-dao/cosmiframechainnpm audit --audit-level=highreports 12 vulns (2 high).wsDoS affects dev tooling paths; wallet stack deps increase patch drift risk.cd frontend-dapp && npm audit --audit-level=highnpm auditgate on high+ for production builds.M-08 — Expired limit-order head clog can exhaust scan budget
smartcontracts/contracts/pair/src/orderbook.rs—MAX_SCAN_STEPS(500),MAX_EXPIRED_PARKS_PER_SWAP(15); invariant L5, L17CleanLimitBookor integrators passbook_start_hintpast prefix (#289). Not a direct escrow drain — wrong-side hints blocked (#272).limit_order_tests::hybrid_walk_scan_steps_cap_bounds_expired_prefix_and_spills_to_pool;hybrid_wrong_side_book_start_hint_no_cross_escrow_drain.M-09 — Off-chain route quotes are advisory (trust boundary)
indexer/src/api/route_solver.rs;frontend-dapp/src/hooks/useSubmitAlignedSimQuote.ts;docs/security-model.mdroute/solveand LCD simulations can diverge from execution snapshot; malicious/compromised indexer could suggest suboptimal or phished routes.min_return/max_spread; funds still protected when those fields set correctly.SimulateSwapOperationsafter env compromise (thought experiment); mitigated byassertSubmitQuotePayRawAligned+ BigIntminReceived.L-01 — Router inner hops omit per-hop deadline
smartcontracts/contracts/router/src/contract.rs:268,429—deadline: Noneon innerTerraSwapdeadline: None.L-02 — Fee-on-transfer CW20 desync if whitelist policy violated
adversarial_token::fee_on_transfer_creates_reserve_imbalancedocs/runbooks/cw20-whitelist-policy.md; runscripts/verify-cw20-code-ids.shbeforeAddWhitelistedCodeId.L-03 — Display-layer float formatting for very large balances
frontend-dapp/src/utils/formatAmount.ts:129(parseFloatin abbrev path)rawAmountMath.ts); display abbreviations may round for amounts > 2⁵³.Controls verified (no regression)
audit_invariant_tests::p1_k_non_decreasing_after_swap; fuzz testsaudit_invariant_tests::p7_*,p8_*commission_treasury_unchanged_after_*minimum_receive; router hop balance deltarouter_coverage_tests;adversarial_token::router_*_dustbook_start_hintcannot cross-drain escrowhybrid_wrong_side_book_start_hint_no_cross_escrow_drainblacklist_testsindexer/tests/security.rs(code review; CI)max_maker_fillshard cap 100indexer/src/hybrid_limits.rsindexer/src/indexer/poller.rs:192-204;indexer/tests/indexer_ingestion_hardening.rsindexer/src/lcd/mod.rs:257+;lcd::tests::get_block_txs_multi_pagemin_receivedrawAmountMath.ts;SwapPage.test.tsxMAX_SAFE_INTEGER gatescripts/verify-no-ibc-hooks-in-contracts.sh.gitlab-ci.ymlgitleaksjobIssue #424 checklist (contracts focus)
orderbook.rs+lib.rsfuzz modulesfuzz_tests,additional_fuzz_tests,wrap_fuzz_tests,orderbook::proptest_limitsadversarial_token.rs,limit_order_tests,blacklist_tests,reentrancy_testsUnauthorizedregressions across factory/pair/router/hookssecurity_tests::test_repeated_small_swaps_no_rounding_profit; book escrow checked mathceil_div; min_return enforced (C4); no execute>sim overpay tests failedfee_math_property_tests, fuzz conservation testsOpen follow-ups (from exploit-replay / launch track)
Conclusion
The codebase demonstrates above-average security engineering for a CosmWasm DEX: explicit invariant matrix, adversarial test harness, indexer hardening, and frontend submit-alignment guards. No Critical exploitable flaw was found in non-governance attack paths during this audit. Residual High items are governance/process and pause mobility, not missing access checks on swaps or escrow.
Full report path:
audits/INTERNAL_COMPOSER_1782692388.mdRan an independent pass on top of the Composer audit. Three things: re-ran every number the report cites, confirmed each cited test + source line actually backs its finding, then went looking for what the audit didn't cover.
Reproduced — all green
cargo test --lib388 + 21,audit_invariant7,security_tests21,fuzz_tests26. Indexercargo test --lib152. Zero failures across the board, matches the report exactly.indexer/tests/security.rs("requires Postgres, not run in this VM"). It IS runnable against the isolateddex_indexer_testDB (separate from the live one, flock-guarded) — ran it, 28/28 green, live indexer untouched. So H6/H7 (sanitized-502, LCD-heavy rate limit, no-internal-leak) have executed evidence now, not just code review.npm audit: 12 vulns / 2 high, matches. One pin-down: the two highs arews(GHSA-96hv-2xvq-fx4p) andundici— the second high is undici, not the cosmes/cosmiframe chain (that chain is moderate/low). Both clear on a non-breakingnpm audit fix; uuid needs--force.cargo-auditnot installed locally; CI jobscargo-audit-smartcontracts/cargo-audit-indexerpresent.Evidence checks — 14 tests + 14 source lines, all confirmed
dex-common/src/pair.rs(the cited files import them);/healthis 368-369 not 368-370; P10 is the twocommission_treasury_unchanged_after_*tests.No-drain conclusion — concur, after reading the math myself
ceil_div(k, new_input_reserve)atpair contract.rs:1090→ trader gross rounds DOWN (pool-favorable), and the swap independently reverts ifnew_k < k(1093-1107). Two layers on the only pool-output path; the limit-book legs never write reserves.minimum_receive— no internal accumulator to inflate.ORDERS/EXPIRED_LIMIT_CLAIMSwith a double-park guard, so cancel and claim-expired are mutually exclusive — no double-claim / over-withdraw.What the audit didn't cover
All gated (governance/owner) with working access checks + tests in tree, so none is a new unauthenticated drain — but these are fund-relevant surfaces the report never names, and the first few are worth explicit analysis before an external audit / launch sign-off:
TRUSTED_ROUTERS+ the caller-suppliedtraderfield — the anti-spoof hinge of the whole discount system. The pair forwardstraderinto the registry without itself checking the CW20 sender is a router; integrity rests entirely on the governance-set (no timelock) trusted-router allowlist. A wrong/compromised trusted router can attribute any tier to anyone (fee leakage). Not mentioned at all.unwrap_output→ external wrap-mapper/treasury contract — that contract custodies/unwraps user output and its code isn't in this repo (only types + a mock). The unwrap-leg trust boundary is unaudited.UpdateLimitOrderPrice— owners reprice a resting order with no re-charged placement fee and no re-check of the clean-config dust thresholds. Enables fee-free queue-jumping / book-head manipulation. Whole entry point unanalyzed.UpdateConfig/UpdateAllowedPairson burn / lp-burn / tax) — set tax recipient + percentage, burn knobs, LP target. A mis-set value silently re-routes value on every swap on an allowed pair. The audit only covered the caller-allowlist gate, not these.RUN_MODE=prod) isn't in the code.mod.rs:499merges SwaggerUi unconditionally, and/api-docs/openapi.jsonis public on the running build right now. Reads as more-mitigated than it is.Sweepstays callable while paused (verified Sweep provably excludes escrow + reserves, so the availability-only rating holds — just a narrative gap);/readystill 404 in the running build (M-04 fix unimplemented); the #277 bounded-fanout setters (SetLpAdminAll/Batchetc.), permissionlessIncreaseObservationCardinality, and the LP mint/burn path are uncited or thin.Layer
Everything above is contract source + unit/integration tests + the live indexer API. The two frontend findings (M-06 CSP
unsafe-inline, L-03formatAmountabbrev) I only checked at source — a browser pass is separate.Bottom line
The audit holds up on everything it covers — numbers reproduce, cited evidence is real, and the no-drain / no-overpay conclusion survives an independent read of the math. It's just not complete: the trusted-router/
traderhinge and the external wrap-mapper are genuine fund-relevant trust boundaries it skips, on top of the reprice path and the hook admin setters. None is a new unauthenticated drain, but I'd want those closed (and the swagger gate actually landed) before this stands as the pre-external-audit baseline.@PlasticDigits — flagging those four gaps + the swagger gate for your call. Happy to dig into any of them.
mentioned in issue #337
Followed up on the four coverage gaps I flagged above — deep-dived each and tried to actually break them (cw-multi-test PoCs where it mattered). Bottom line: none is a no-privilege exploit, which backs up the no-drain read. Per-gap result, action items first.
Hook admin setters — needs a trust-level confirmation (low, but real)
One correction to my earlier note first: the percentages ARE bounded. tax/burn/lp
*_percentage_bpsall clamp> 10000→InvalidBpsat both instantiate AND update_config in all three hooks, so there's no >100% / over-levy path — my "mis-set percentage" worry was off.What IS real: each hook's
config.adminis a free-form address taken fromInstantiateMsg.admin, NOT derived from factory governance. Registering a hook onto a pair is governance-only, but once registered the hook's params (recipient, bps, burn/lp targets) are controlled by that independent admin key with no further governance check. So the admin can point the taxrecipientat itself and set bps to 10000 — siphoning up to 100% of the ask-token output of every swap on a registered pair (takers with a nonzeromin_returnare protected by the slippage revert; only amin_return=0taker gets silently zeroed).No unprivileged path — it needs the hook admin key. But if that key is weaker than governance, the blast radius is wide. Action: confirm on-chain each live hook's
config.admin == factory governance / the multisig(queryGetConfig). If it's a separate ops key, that's a real exposure — worth binding the hook admin to governance in code, or a tighter max-bps clamp so even the admin can't take the full output.Router unwrap / wrap-mapper — minimum_receive doesn't cross the unwrap boundary (low correctness)
minimum_receiveis asserted on the pre-unwrap wrapped-CW20 balance delta (router contract.rs:356-363), BEFORE the unwrapSendto the mapper is built — there's no post-unwrap re-assertion on what the user actually receives. So an honest mapper charging an unwrap fee, or returning the wrong denom, would silently shortchange the user below their declared floor.Not an unprivileged exploit: the mapper is governance-set (
SetWrapMapper, no per-call override on the hook), and a reverting mapper rolls the whole tx back atomically. But the slippage guarantee should be re-asserted on the final received denom/amount (cleanest as areply_on_successbranch rather than the current top-leveladd_message). And the live mapper's unwrap-directionfee_bpsneeds to be confirmed 0 (or accounted for) — its contract isn't in this repo, so it needs to be pinned + audited regardless.Limit reprice (UpdateLimitOrderPrice) — escrow-safe, one fairness quirk (no funds)
Escrow is conserved exactly:
relink_limit_order_priceonly changesprice(the rate), neverorder.remainingorPENDING_ESCROW_*; fill always clampscost <= order.remaining, so a maker can't be paid out more of the escrowed token than they hold, regardless of repricing. No dust / min-remaining bypass (reprice changes price, not remaining), no unbounded DoS (gas per move + existing match-walk caps).One by-design quirk worth documenting: reprice keeps the order's original id, so it retains global arrival-time priority — an old low-id order can move to the front of a new price level for free, leapfrogging makers who arrived earlier at that level with higher ids. Fairness/UX, no fund movement. If you want exchange-style lose-priority-on-modify, assign a fresh id on reprice; otherwise it's a deliberate semantics choice. Either way worth a doc line + a
Repriceop in theprop_escrow_dllfuzz to cover escrow/DLL conservation across reprice explicitly.Fee-discount trusted-router / trader spoof — verified safe (no action)
Proved with a PoC: a normal caller doing
cw20.Send(pair, Swap{trader: WHALE})gets ZERO discount. The CW20 contract pins the real sender, the pair forwards THAT (not itself) assender, and the registry's sender-fallback (fee-discount contract.rs:463-474) ignores the spoofedtraderunless the forwarded sender is inTRUSTED_ROUTERS. Correctly defended; matches the existingtest_query_discount_untrusted_router_falls_back_to_sender.Net
The four gaps reduce to governance/owner-trust + coverage, not new unauthenticated drains — consistent with the audit's conclusion. The two worth acting on before external audit / launch are the hook-admin key trust level and the wrap-mapper slippage boundary; the reprice quirk is docs + fuzz; the trader spoof is clean.
@PlasticDigits — two asks: confirm the live hook
config.adminkeys == governance/multisig (not a separate ops key), and confirm the live wrap-mapper's unwrapfee_bpsis 0 (+ pin/audit that contract, since it's out-of-repo). Happy to write the min_receive-across-unwrap re-assertion and the reprice fuzz extension if you want them.config.admin keys should be governance/multisig: YES
unwrap: See https://gitlab.com/PlasticDigits/ust1-window, specifically under smartcontracts-terraclassic/contracts/cmm-native-swap. Likely needs to be brought in as depedency and as a prerequisitive for cl8y dex deployment @Brouie
On your two callouts @PlasticDigits — verified the current on-chain state for both.
8c56f4b8deploy, both the config governance field AND the CosmWasm migrate-admin (code-upgrade authority):So the rotation is only half done, two gaps for launch:
This is the deploy/rotation step, not a QA fix — I'll re-verify all five answer to the multisig once it lands.
While here, two of the audit's recommended fixes are still NOT in the tree on
8c56f4b8, confirmed live:Net: the contract/indexer audit body is covered (Composer report + my independent repro pass, all suites green incl the security.rs the report skipped). The genuinely-open residuals are all dev/deploy actions, not QA-closeable here: the admin-key + migrate-admin rotation, the cmm-native-wrap import, and the M-04/M-05 fixes. Frontend SEC-E findings are tracked under #425-430 (working those next).
Verification — #424 Contracts internal security audit
Verifier: Cloud Agent (release verify)
Date (UTC): 2026-06-30
Baseline:
main@ workspace HEAD (clean tree)Summary
Re-ran the full contracts security audit verification matrix from the Composer audit (#424 comment) and Brouie's independent repro pass. All automated contract and indexer security suites pass. The in-repo audit artifact (
docs/contracts-security-audit.md) and invariant matrix are present and doc-drift checks pass. No code or docs changes were required.Verdict: PASS — internal contracts security audit is complete and regression-tested on current
main.Acceptance criteria → results
cd smartcontracts && cargo test --lib→ 388 + 21 passedcargo test audit_invariant→ 7 passedcargo test security_tests→ 21 passedcargo test fuzz_tests→ 26 passedcargo test migration_tests→ 4 passedcargo test blacklist_tests→ 8 passedcargo test adversarial_token→ 8 passedmake test-contracts→ exit 0cd indexer && cargo test --lib→ 158 passedsecurity.rsintegration (skipped in original audit VM)cargo test --test security -- --test-threads=1→ 35 passed (Postgres viamake setup-indexer-postgres)make check-exploit-replay-matrix-docsmake check-fee-discount-tier-docsdocs/contracts-security-audit.mdpresent; rows P1–C14, L1–L17, B1, W1, etc.test_repeated_small_swaps_no_rounding_profit, k props)npm audit --audit-level=high→ 2 high (wsvia cosmes fork); tracked in original audit; not a contracts blockerResidual risks (documented follow-ups — not QA failures)
These were flagged in the issue thread as dev/deploy actions, outside the scope of closing the in-repo audit:
minimum_receiveasserted pre-unwrap;cmm-native-wrapfrom ust1-window recommended as deployment prerequisite (PlasticDigits confirmed)./readyprobe —health()returns static{"status":"ok"}; no DB+LCD readiness route yet.SwaggerUimerged unconditionally inindexer/src/api/mod.rs:499; noRUN_MODEgate in current tree.Follow-up ideas
cmm-native-wrapas router unwrap dependency and add end-to-endmin_native_outregression across the wrap boundary.Repriceop toprop_escrow_dllfuzz (Brouie suggestion) and document limit reprice priority semantics.Closing as verified — no MR opened (working tree clean).