Harden CosmWasm hooks and document CW20/hook ops policy (#376)

PlasticDigits commented

2026-06-13 07:56:20 +00:00

(Migrated from gitlab.com)

Parent

Remediation bundle for #376 — Full security report.

Covers approved findings: H-01, H-02, H-03, I-02.

Current codebase

Fee-on-transfer risk (H-01): Pair swap/LP paths in smartcontracts/contracts/pair/src/contract.rs credit reserves from declared CW20 amount, not balance deltas. Factory whitelists CW20 code IDs in smartcontracts/contracts/factory/src/contract.rs. Invariant P2 and adversarial test fee_on_transfer_creates_reserve_imbalance in smartcontracts/tests/src/adversarial_token.rs document the failure mode. docs/contracts-security-audit.md and docs/security-model.md mention whitelist trust but lack an ops prohibition on fee-on-transfer templates.
Hook swap blocking (H-02): Post-swap hooks in pair/src/contract.rs (~L1174–1224) dispatch as plain WasmMsg::Execute; hook Err atomically reverts the swap (invariant H1). docs/runbooks/launch-checklist.md says "audited hooks only" but there is no dedicated hook registration playbook.
LP burn hook spoofing (H-03): smartcontracts/contracts/hooks/lp-burn-hook/src/contract.rs gates callers via assert_allowed_pair (info.sender ∈ ALLOWED_PAIRS) but trusts the pair field in AfterSwap for burn math. Adversarial test lp_burn_hook_accepts_spoofed_pair_when_spoofer_allowlisted proves inflated burns when a non-pair spoofer is allowlisted.
Hook treasury subsidy (I-02): tax-hook, burn-hook, and lp-burn-hook compute fees from return_asset.amount but transfer/burn only up to pre-funded hook balances (tax-hook/src/contract.rs, burn-hook/src/contract.rs, lp-burn-hook/src/contract.rs). Users receive full swap output; treasury subsidizes shortfalls.

Why needed

Governance misconfiguration or hook admin mistakes can cause reserve desync, trading DoS, or treasury drain. Approved remediation is documentation + verification for H-01/H-02, on-chain hardening for H-03, and a charging model change for fee hooks (I-02).

Constraints / guardrails

H-01: Do not implement on-chain balance-delta reconciliation — document prohibition and verify planned code IDs only.
H-02: Hooks may intentionally block swaps (AML, incident response). Do not add SubMsg::reply_on_error unless product explicitly changes policy.
H-03: Restrict ALLOWED_PAIRS to verified pair contracts; require pair == info.sender and query pair state before burning.
I-02: Fee hooks must charge from swap input/output flow, not subsidize from hook treasury. LP-burn treasury funding may remain but must not use spoofable amounts (coordinate with H-03).

Relevant files

Area	Paths
Pair / factory	`smartcontracts/contracts/pair/src/contract.rs`, `smartcontracts/contracts/factory/src/contract.rs`
Hooks	`smartcontracts/contracts/hooks/lp-burn-hook/src/contract.rs`, `tax-hook/src/contract.rs`, `burn-hook/src/contract.rs`, `smartcontracts/contracts/hooks/README.md`
Tests	`smartcontracts/tests/src/adversarial_token.rs`
Docs	`docs/contracts-security-audit.md`, `docs/security-model.md`, `docs/runbooks/launch-checklist.md`

Recommended direction

H-01: Add ops runbook section forbidding fee-on-transfer CW20 code IDs. Add launch-checklist item to verify GDEX and TerraPort production code IDs are standard (non fee-on-transfer) via LCD CodeInfo before whitelist.
H-02: Create docs/runbooks/hook-registration.md (or expand docs/security-model.md) covering: mandatory audit before registration, wasm review checklist, allowlist hygiene, and when blocking hooks are acceptable.
H-03: In execute_after_swap, require pair == info.sender, query pair config/state (e.g. LP token, reserves) to validate caller is the configured target_pair, reject spoofed output_amount paths. Add regression test: allowlisted spoofer with mismatched pair must fail.
I-02: Redesign tax/burn hooks to deduct from swap flow (e.g. pair forwards tax portion to hook/recipient during swap settlement) rather than spending pre-funded balances. Document migration path for existing treasury-funded deployments.

Acceptance criteria

Runbook explicitly forbids fee-on-transfer CW20 templates; GDEX/TerraPort verification script or checklist exists.
Hook registration playbook documents audit requirement and intentional swap-blocking policy.
LP burn hook rejects calls where pair != info.sender or pair state does not match config; adversarial regression test passes.
Tax/burn hooks charge from swap I/O; no silent treasury subsidy for normal fee collection.
cargo test adversarial and hook unit tests pass.

Test plan

cd smartcontracts && cargo test adversarial
cd smartcontracts && cargo test -p cl8y-dex-lp-burn-hook
cd smartcontracts && cargo test -p cl8y-dex-tax-hook -p cl8y-dex-burn-hook (or equivalent hook crate names)
make test-contracts (full contract suite)
Manual: review new/updated docs in docs/runbooks/ and docs/security-model.md

Attack / abuse test plan

Re-run fee_on_transfer_creates_reserve_imbalance — must still demonstrate risk when adversarial code ID whitelisted (documents why prohibition matters).
Re-run lp_burn_hook_accepts_spoofed_pair_when_spoofer_allowlisted — must fail after H-03 fix.
Re-run swap_fails_atomically_when_allowlisted_hook_reverts — must still pass (H-02 documents, does not change).
Add test: tax hook with zero treasury balance still collects correct fee from swap output after I-02.

Verification criteria

All new tests green in CI.
Docs merged and referenced from docs/security-model.md or launch checklist.
No regression in existing pair swap, hook dispatch, or router multi-hop adversarial tests.

## Parent Remediation bundle for [#376 — Full security report](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/issues/376). Covers approved findings: **H-01**, **H-02**, **H-03**, **I-02**. ## Current codebase - **Fee-on-transfer risk (H-01):** Pair swap/LP paths in `smartcontracts/contracts/pair/src/contract.rs` credit reserves from declared CW20 `amount`, not balance deltas. Factory whitelists CW20 code IDs in `smartcontracts/contracts/factory/src/contract.rs`. Invariant P2 and adversarial test `fee_on_transfer_creates_reserve_imbalance` in `smartcontracts/tests/src/adversarial_token.rs` document the failure mode. `docs/contracts-security-audit.md` and `docs/security-model.md` mention whitelist trust but lack an ops prohibition on fee-on-transfer templates. - **Hook swap blocking (H-02):** Post-swap hooks in `pair/src/contract.rs` (~L1174–1224) dispatch as plain `WasmMsg::Execute`; hook `Err` atomically reverts the swap (invariant H1). `docs/runbooks/launch-checklist.md` says "audited hooks only" but there is no dedicated hook registration playbook. - **LP burn hook spoofing (H-03):** `smartcontracts/contracts/hooks/lp-burn-hook/src/contract.rs` gates callers via `assert_allowed_pair` (`info.sender ∈ ALLOWED_PAIRS`) but trusts the `pair` field in `AfterSwap` for burn math. Adversarial test `lp_burn_hook_accepts_spoofed_pair_when_spoofer_allowlisted` proves inflated burns when a non-pair spoofer is allowlisted. - **Hook treasury subsidy (I-02):** `tax-hook`, `burn-hook`, and `lp-burn-hook` compute fees from `return_asset.amount` but transfer/burn only up to pre-funded hook balances (`tax-hook/src/contract.rs`, `burn-hook/src/contract.rs`, `lp-burn-hook/src/contract.rs`). Users receive full swap output; treasury subsidizes shortfalls. ## Why needed Governance misconfiguration or hook admin mistakes can cause reserve desync, trading DoS, or treasury drain. Approved remediation is documentation + verification for H-01/H-02, on-chain hardening for H-03, and a charging model change for fee hooks (I-02). ## Constraints / guardrails - **H-01:** Do **not** implement on-chain balance-delta reconciliation — document prohibition and verify planned code IDs only. - **H-02:** Hooks **may** intentionally block swaps (AML, incident response). Do **not** add `SubMsg::reply_on_error` unless product explicitly changes policy. - **H-03:** Restrict `ALLOWED_PAIRS` to verified pair contracts; require `pair == info.sender` **and** query pair state before burning. - **I-02:** Fee hooks must charge from swap input/output flow, not subsidize from hook treasury. LP-burn treasury funding may remain but must not use spoofable amounts (coordinate with H-03). ## Relevant files | Area | Paths | |------|-------| | Pair / factory | `smartcontracts/contracts/pair/src/contract.rs`, `smartcontracts/contracts/factory/src/contract.rs` | | Hooks | `smartcontracts/contracts/hooks/lp-burn-hook/src/contract.rs`, `tax-hook/src/contract.rs`, `burn-hook/src/contract.rs`, `smartcontracts/contracts/hooks/README.md` | | Tests | `smartcontracts/tests/src/adversarial_token.rs` | | Docs | `docs/contracts-security-audit.md`, `docs/security-model.md`, `docs/runbooks/launch-checklist.md` | ## Recommended direction 1. **H-01:** Add ops runbook section forbidding fee-on-transfer CW20 code IDs. Add launch-checklist item to verify GDEX and TerraPort production code IDs are standard (non fee-on-transfer) via LCD `CodeInfo` before whitelist. 2. **H-02:** Create `docs/runbooks/hook-registration.md` (or expand `docs/security-model.md`) covering: mandatory audit before registration, wasm review checklist, allowlist hygiene, and when blocking hooks are acceptable. 3. **H-03:** In `execute_after_swap`, require `pair == info.sender`, query pair config/state (e.g. LP token, reserves) to validate caller is the configured `target_pair`, reject spoofed `output_amount` paths. Add regression test: allowlisted spoofer with mismatched `pair` must fail. 4. **I-02:** Redesign tax/burn hooks to deduct from swap flow (e.g. pair forwards tax portion to hook/recipient during swap settlement) rather than spending pre-funded balances. Document migration path for existing treasury-funded deployments. ## Acceptance criteria - [ ] Runbook explicitly forbids fee-on-transfer CW20 templates; GDEX/TerraPort verification script or checklist exists. - [ ] Hook registration playbook documents audit requirement and intentional swap-blocking policy. - [ ] LP burn hook rejects calls where `pair != info.sender` or pair state does not match config; adversarial regression test passes. - [ ] Tax/burn hooks charge from swap I/O; no silent treasury subsidy for normal fee collection. - [ ] `cargo test adversarial` and hook unit tests pass. ## Test plan - `cd smartcontracts && cargo test adversarial` - `cd smartcontracts && cargo test -p cl8y-dex-lp-burn-hook` - `cd smartcontracts && cargo test -p cl8y-dex-tax-hook -p cl8y-dex-burn-hook` (or equivalent hook crate names) - `make test-contracts` (full contract suite) - Manual: review new/updated docs in `docs/runbooks/` and `docs/security-model.md` ## Attack / abuse test plan - Re-run `fee_on_transfer_creates_reserve_imbalance` — must still demonstrate risk when adversarial code ID whitelisted (documents why prohibition matters). - Re-run `lp_burn_hook_accepts_spoofed_pair_when_spoofer_allowlisted` — **must fail** after H-03 fix. - Re-run `swap_fails_atomically_when_allowlisted_hook_reverts` — must still pass (H-02 documents, does not change). - Add test: tax hook with zero treasury balance still collects correct fee from swap output after I-02. ## Verification criteria - All new tests green in CI. - Docs merged and referenced from `docs/security-model.md` or launch checklist. - No regression in existing pair swap, hook dispatch, or router multi-hop adversarial tests.

PlasticDigits commented

2026-06-13 07:56:46 +00:00

(Migrated from gitlab.com)

mentioned in issue #376

PlasticDigits commented

2026-06-13 09:42:51 +00:00

(Migrated from gitlab.com)

mentioned in commit be82f63e28

mentioned in commit be82f63e28c7dfed807e0ea7e40db23312319f00

PlasticDigits commented

2026-06-13 09:43:04 +00:00

(Migrated from gitlab.com)

mentioned in merge request !901

ghost1 commented

2026-06-13 09:46:07 +00:00

(Migrated from gitlab.com)

mentioned in commit d2dcb7a905

mentioned in commit d2dcb7a905d2948bc748b54f99c93a15ea317d03

PlasticDigits commented

2026-06-13 10:03:09 +00:00

(Migrated from gitlab.com)

mentioned in commit 29c47c132a

mentioned in commit 29c47c132a1a64a56ab51317719a8c09d06bd6fd

PlasticDigits commented

2026-06-13 10:34:52 +00:00

(Migrated from gitlab.com)

mentioned in commit 908618d2a7

mentioned in commit 908618d2a7c16a210a596fb816bafc8445172c7e

PlasticDigits commented

2026-06-13 13:58:49 +00:00

(Migrated from gitlab.com)

mentioned in commit a688b6e5a8

mentioned in commit a688b6e5a8d5abe4667191623ce77e44074c600b

PlasticDigits commented

2026-06-13 13:59:28 +00:00

(Migrated from gitlab.com)

mentioned in merge request !905

PlasticDigits commented

2026-06-13 14:15:59 +00:00

(Migrated from gitlab.com)

mentioned in commit 5557301a09

mentioned in commit 5557301a0997575bae25f084f2ae7ed6dec183fd

PlasticDigits commented

2026-06-14 02:54:27 +00:00

(Migrated from gitlab.com)

mentioned in commit 0ca7e9865a

mentioned in commit 0ca7e9865a4deaad1758133f60ee8e3ad6865b9c

PlasticDigits commented

2026-06-14 03:03:34 +00:00

(Migrated from gitlab.com)

mentioned in commit fb7dad99ab

mentioned in commit fb7dad99ab88e7989185698cd5b4060a7c1ab428

PlasticDigits commented

2026-06-14 03:03:44 +00:00

(Migrated from gitlab.com)

mentioned in merge request !907

PlasticDigits commented

2026-06-14 05:42:08 +00:00

(Migrated from gitlab.com)

mentioned in commit 5154e7bed7

mentioned in commit 5154e7bed725c0b5ddcc6d4715212a0f07fb236b

PlasticDigits commented

2026-06-14 06:53:26 +00:00

(Migrated from gitlab.com)

mentioned in commit a7f2691ee7

mentioned in commit a7f2691ee7b1ff4e1a4462c2dcd9e1c25b42c082

PlasticDigits commented

2026-06-14 06:56:16 +00:00

(Migrated from gitlab.com)

mentioned in commit dd654b11f5

mentioned in commit dd654b11f5c8209b4807562c2e34a2c9436e3ccd

PlasticDigits commented

2026-06-14 11:56:43 +00:00

(Migrated from gitlab.com)

Verification complete — all acceptance criteria PASS

Verified on main @ 30cfd4be (merged via verify/issue-377-hook-hardening).

Acceptance criteria

Criterion	Result	How verified
Runbook forbids fee-on-transfer CW20; GDEX/TerraPort checklist/script	PASS	`docs/runbooks/cw20-whitelist-policy.md` § Prohibited templates; `scripts/verify-cw20-code-ids.sh` (SKIP without env — expected); cross-linked from `docs/security-model.md` and `docs/runbooks/launch-checklist.md`
Hook registration playbook (audit + intentional swap-blocking)	PASS	`docs/runbooks/hook-registration.md` — audit checklist, allowlist hygiene, blocking-hook policy; referenced from security model + launch checklist
LP burn hook H-03 hardening + adversarial regression	PASS	`assert_pair_caller` requires `pair == info.sender` + pair `liquidity_token` match in `lp-burn-hook/src/contract.rs`; `cargo test adversarial` → `lp_burn_hook_rejects_spoofed_pair_when_spoofer_allowlisted` ok
Tax/burn hooks charge from swap I/O (I-02)	PASS	Pair settlement via `dex-common::hook_settlement`; hooks README documents flow; `tax_hook_collects_from_swap_flow_with_zero_treasury_balance` ok
`cargo test adversarial` + hook tests + full suite	PASS	`cargo test adversarial` (9/9); `cargo test -p cl8y-dex-lp-burn-hook`; `cargo test -p cl8y-dex-tax-hook -p cl8y-dex-burn-hook`; `make test-contracts` (377/377)

Attack / abuse test plan

Test	Result
`fee_on_transfer_creates_reserve_imbalance`	PASS (still demonstrates H-01 risk)
`lp_burn_hook_rejects_spoofed_pair_when_spoofer_allowlisted`	PASS (rejects spoof after H-03)
`swap_fails_atomically_when_allowlisted_hook_reverts`	PASS (H-02 unchanged)
`tax_hook_collects_from_swap_flow_with_zero_treasury_balance`	PASS (I-02)

Manual doc review

docs/runbooks/cw20-whitelist-policy.md, docs/runbooks/hook-registration.md, docs/security-model.md, smartcontracts/contracts/hooks/README.md, .cursor/skills/AGENTS_HOOK_CW20_OPS.md — consistent cross-links and invariant mapping.

No repo changes required during this verify pass.

## Verification complete — all acceptance criteria **PASS** Verified on `main` @ `30cfd4be` (merged via `verify/issue-377-hook-hardening`). ### Acceptance criteria | Criterion | Result | How verified | |-----------|--------|--------------| | Runbook forbids fee-on-transfer CW20; GDEX/TerraPort checklist/script | **PASS** | [`docs/runbooks/cw20-whitelist-policy.md`](docs/runbooks/cw20-whitelist-policy.md) § Prohibited templates; [`scripts/verify-cw20-code-ids.sh`](scripts/verify-cw20-code-ids.sh) (SKIP without env — expected); cross-linked from [`docs/security-model.md`](docs/security-model.md) and [`docs/runbooks/launch-checklist.md`](docs/runbooks/launch-checklist.md) | | Hook registration playbook (audit + intentional swap-blocking) | **PASS** | [`docs/runbooks/hook-registration.md`](docs/runbooks/hook-registration.md) — audit checklist, allowlist hygiene, blocking-hook policy; referenced from security model + launch checklist | | LP burn hook H-03 hardening + adversarial regression | **PASS** | `assert_pair_caller` requires `pair == info.sender` + pair `liquidity_token` match in `lp-burn-hook/src/contract.rs`; `cargo test adversarial` → `lp_burn_hook_rejects_spoofed_pair_when_spoofer_allowlisted` **ok** | | Tax/burn hooks charge from swap I/O (I-02) | **PASS** | Pair settlement via `dex-common::hook_settlement`; hooks README documents flow; `tax_hook_collects_from_swap_flow_with_zero_treasury_balance` **ok** | | `cargo test adversarial` + hook tests + full suite | **PASS** | `cargo test adversarial` (9/9); `cargo test -p cl8y-dex-lp-burn-hook`; `cargo test -p cl8y-dex-tax-hook -p cl8y-dex-burn-hook`; `make test-contracts` (377/377) | ### Attack / abuse test plan | Test | Result | |------|--------| | `fee_on_transfer_creates_reserve_imbalance` | **PASS** (still demonstrates H-01 risk) | | `lp_burn_hook_rejects_spoofed_pair_when_spoofer_allowlisted` | **PASS** (rejects spoof after H-03) | | `swap_fails_atomically_when_allowlisted_hook_reverts` | **PASS** (H-02 unchanged) | | `tax_hook_collects_from_swap_flow_with_zero_treasury_balance` | **PASS** (I-02) | ### Manual doc review - [`docs/runbooks/cw20-whitelist-policy.md`](docs/runbooks/cw20-whitelist-policy.md), [`docs/runbooks/hook-registration.md`](docs/runbooks/hook-registration.md), [`docs/security-model.md`](docs/security-model.md), [`smartcontracts/contracts/hooks/README.md`](smartcontracts/contracts/hooks/README.md), [`.cursor/skills/AGENTS_HOOK_CW20_OPS.md`](skills/AGENTS_HOOK_CW20_OPS.md) — consistent cross-links and invariant mapping. No repo changes required during this verify pass.

PlasticDigits (Migrated from gitlab.com) closed this issue

2026-06-14 11:56:44 +00:00

Brouie commented

2026-06-28 22:41:06 +00:00

(Migrated from gitlab.com)

mentioned in issue #337