feat(security): C-1 — verify InstantWithdrawCw20 wire format vs ustr-cmm treasury schema #21

New issue

Closed

opened 2026-08-08 04:40:16 +00:00 by PlasticDigits · 9 comments

PlasticDigits commented 2026-08-08 04:40:16 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

Summary

Close C-1 from audits/INTERNAL_KIMIK3_1786162831.md: make ust1-window's InstantWithdrawCw20 wire format a cross-repo verified contract against final ustr-cmm treasury schema, not a self-referential local stub.

Audit ref: C-1 / TEST-1 / SC-7
Depends on: PlasticDigits2/ustr-cmm#6 (final msg schema frozen)
Related: #20 (withdraw path that introduced this client)

---

Current codebase

Component	Path	Behavior today
Treasury client	contracts/ust1-window/src/treasury.rs	Local TreasuryExecuteMsg::InstantWithdrawCw20 { recipient, token, amount } via cw_serde; only schema check is serialize-against-self
Multitest stub	contracts/ust1-window/src/multitest.rs	Stub accepts InstantWithdraw → CW20 Transfer; written to match window client
Integration stub	smartcontracts-terraclassic/tests/src/stub_treasury.rs	Same local enum; no deny_unknown_fields; forwards-compatible by design
Cached ustr-cmm	git dep (see Cargo.lock)	Checkout historically lacked InstantWithdrawCw20 until ustr-cmm#6 merges
Live window	code 11550 / terra1zxwp…	Redeem path depends on this wire format after migrate + treasury SetCw20Spender

Problem: The only schema authority in this repo is the window's own type + a permissive stub. Any final treasury field rename, casing change, or enum layout difference bricks every mainnet withdraw (fail-closed — no theft — but redemption dead until emergency migrate).

---

Why this is needed

1. Cross-repo wire drift is invisible to unit tests that round-trip the window's own type.
2. Stub deliberately dropped strictness so unknown fields pass — the opposite of production CosmWasm decode behavior.
3. Mainnet Phase 5 redeem (parent #19) must not discover schema mismatch at first user withdraw.
4. Bootstrap still requires treasury gov SetCw20Spender; schema conformance is a necessary but not sufficient gate before announcing redeem.

---

Constraints / guardrails

1. Do not loosen production decode; restore stub strictness to match real treasury (deny_unknown_fields / equivalent).
2. Pin the treasury schema source to a specific git rev of ustr-cmm (or published JSON schema artifact), not a floating branch tip in CI without lock.
3. Keep window client a minimal subset of treasury ExecuteMsg (only InstantWithdrawCw20) — do not import the full treasury crate into the window wasm unless size/compile cost is acceptable.
4. Integration suite must exercise the real cmm-treasury wasm when available; stub remains for fast unit tests only and must stay byte-compatible.
5. No change to deposit path, fee math, limits, pause, or min_vfdusd_out in this issue.
6. Post-migrate ops: one live withdraw probe before public announcement (runbook, not code).

---

Relevant files

• contracts/ust1-window/src/treasury.rs — client msg + builder
• contracts/ust1-window/src/multitest.rs — in-crate stub
• smartcontracts-terraclassic/tests/src/stub_treasury.rs — integration stub
• smartcontracts-terraclassic/tests/src/integration_tests.rs — prefer real treasury wasm path
• Cargo.toml / Cargo.lock — ustr-cmm pin if shared types used
• docs/DEPLOYMENT.md — post-migrate withdraw probe checklist
• CI (.gitlab-ci.yml and/or .github/workflows/ci.yml) — cross-repo schema job

---

Recommended direction

1. From final ustr-cmm#6 rev, export/generate treasury JSON schema (or compile treasury msg types) and add a CI/unit test that window::TreasuryExecuteMsg::InstantWithdrawCw20 serializes byte-for-byte to the treasury's expected variant.
2. Restore deny_unknown_fields (or matching strictness) on stubs so misspelled/extra fields fail like production.
3. Add an integration path that stores/loads the real treasury wasm artifact built from that rev (skip or mark ignore if artifact unavailable offline; required in CI when network/cache allows).
4. Document SetCw20Spender + live probe ordering in DEPLOYMENT.md.

---

Acceptance criteria

• Automated test fails if window InstantWithdrawCw20 JSON diverges from pinned ustr-cmm treasury schema.
• Stubs reject unknown/renamed fields consistently with real treasury decode.
• At least one integration test path runs against real treasury wasm (or CI-gated equivalent), not only the local stub.
• Deployment docs include: schema pin rev, SetCw20Spender, live withdraw probe before announce.
• Existing deposit/withdraw unit tests still pass; no intentional behavior change to swap math.

---

Test plan (all paths)

1. Happy: window InstantWithdrawCw20 JSON == treasury schema for representative recipient/token/amount values (incl. max Uint128).
2. Schema mismatch (negative): rename field / wrong casing / wrong type in a fixture → conformance test fails.
3. Stub strictness: send unknown field to stub → reject; send valid msg → Transfer emitted.
4. Real wasm: instantiate real treasury + register window as spender → withdraw succeeds; unregistered spender → atomic revert (burn+withdraw fail together).
5. Deposit regression: deposit still forwards vFDUSD; unchanged.
6. CI: job clones/pins ustr-cmm rev and runs conformance (document skip conditions).

---

Test plan (attack / hack / abuse vectors)

1. Supply-chain schema bait: attacker proposes MR that “updates” local stub to a wrong shape without updating conformance pin → CI must fail.
2. Forward-compat trap: stub accepts unknown fields that production rejects → ensure stub no longer hides this (deny unknown).
3. Partial field omission: missing token/amount/recipient → both stub and real treasury reject; window never builds incomplete msgs.
4. Wrong recipient injection: client builder must still bind recipient to cw20 Send sender only (existing INV); conformance must not weaken that.
5. Spender not registered: withdraw reverts atomically; no partial burn without payout (existing atomicity tests remain green).

---

Verification criteria

• Conformance test green against pinned ustr-cmm rev used for mainnet treasury code.
• cargo test (window + integration) green with strict stubs.
• Operator runbook lists probe tx; schema pin recorded in docs or CI config.
• Audit C-1 marked addressed when the above lands on the deploy branch.

## Summary Close **C-1** from [`audits/INTERNAL_KIMIK3_1786162831.md`](../audits/INTERNAL_KIMIK3_1786162831.md): make `ust1-window`'s `InstantWithdrawCw20` wire format a **cross-repo verified** contract against final `ustr-cmm` treasury schema, not a self-referential local stub. **Audit ref:** C-1 / TEST-1 / SC-7 **Depends on:** [PlasticDigits2/ustr-cmm#6](https://gitlab.com/PlasticDigits2/ustr-cmm/-/issues/6) (final msg schema frozen) **Related:** [#20](https://gitlab.com/PlasticDigits/ust1-window/-/issues/20) (withdraw path that introduced this client) --- ## Current codebase | Component | Path | Behavior today | |-----------|------|----------------| | Treasury client | `contracts/ust1-window/src/treasury.rs` | Local `TreasuryExecuteMsg::InstantWithdrawCw20 { recipient, token, amount }` via `cw_serde`; only schema check is serialize-against-self | | Multitest stub | `contracts/ust1-window/src/multitest.rs` | Stub accepts InstantWithdraw → CW20 Transfer; written to match window client | | Integration stub | `smartcontracts-terraclassic/tests/src/stub_treasury.rs` | Same local enum; **no** `deny_unknown_fields`; forwards-compatible by design | | Cached ustr-cmm | git dep (see `Cargo.lock`) | Checkout historically lacked `InstantWithdrawCw20` until ustr-cmm#6 merges | | Live window | code **11550** / `terra1zxwp…` | Redeem path depends on this wire format after migrate + treasury `SetCw20Spender` | **Problem:** The only schema authority in this repo is the window's own type + a permissive stub. Any final treasury field rename, casing change, or enum layout difference bricks every mainnet withdraw (fail-closed — no theft — but redemption dead until emergency migrate). --- ## Why this is needed 1. Cross-repo wire drift is invisible to unit tests that round-trip the window's own type. 2. Stub deliberately dropped strictness so unknown fields pass — the opposite of production CosmWasm decode behavior. 3. Mainnet Phase 5 redeem (parent [#19](https://gitlab.com/PlasticDigits/ust1-window/-/issues/19)) must not discover schema mismatch at first user withdraw. 4. Bootstrap still requires treasury gov `SetCw20Spender`; schema conformance is a necessary but not sufficient gate before announcing redeem. --- ## Constraints / guardrails 1. **Do not** loosen production decode; restore stub strictness to match real treasury (`deny_unknown_fields` / equivalent). 2. Pin the treasury schema source to a **specific git rev** of `ustr-cmm` (or published JSON schema artifact), not a floating branch tip in CI without lock. 3. Keep window client a **minimal subset** of treasury `ExecuteMsg` (only InstantWithdrawCw20) — do not import the full treasury crate into the window wasm unless size/compile cost is acceptable. 4. Integration suite must exercise the **real** `cmm-treasury` wasm when available; stub remains for fast unit tests only and must stay byte-compatible. 5. No change to deposit path, fee math, limits, pause, or `min_vfdusd_out` in this issue. 6. Post-migrate ops: one live withdraw probe before public announcement (runbook, not code). --- ## Relevant files - `contracts/ust1-window/src/treasury.rs` — client msg + builder - `contracts/ust1-window/src/multitest.rs` — in-crate stub - `smartcontracts-terraclassic/tests/src/stub_treasury.rs` — integration stub - `smartcontracts-terraclassic/tests/src/integration_tests.rs` — prefer real treasury wasm path - `Cargo.toml` / `Cargo.lock` — ustr-cmm pin if shared types used - `docs/DEPLOYMENT.md` — post-migrate withdraw probe checklist - CI (`.gitlab-ci.yml` and/or `.github/workflows/ci.yml`) — cross-repo schema job --- ## Recommended direction 1. From final `ustr-cmm#6` rev, export/generate treasury JSON schema (or compile treasury msg types) and add a CI/unit test that `window::TreasuryExecuteMsg::InstantWithdrawCw20` serializes **byte-for-byte** to the treasury's expected variant. 2. Restore `deny_unknown_fields` (or matching strictness) on stubs so misspelled/extra fields fail like production. 3. Add an integration path that stores/loads the **real** treasury wasm artifact built from that rev (skip or mark `ignore` if artifact unavailable offline; required in CI when network/cache allows). 4. Document SetCw20Spender + live probe ordering in `DEPLOYMENT.md`. --- ## Acceptance criteria - [ ] Automated test fails if window InstantWithdrawCw20 JSON diverges from pinned ustr-cmm treasury schema. - [ ] Stubs reject unknown/renamed fields consistently with real treasury decode. - [ ] At least one integration test path runs against real treasury wasm (or CI-gated equivalent), not only the local stub. - [ ] Deployment docs include: schema pin rev, SetCw20Spender, live withdraw probe before announce. - [ ] Existing deposit/withdraw unit tests still pass; no intentional behavior change to swap math. --- ## Test plan (all paths) 1. **Happy:** window InstantWithdrawCw20 JSON == treasury schema for representative recipient/token/amount values (incl. max `Uint128`). 2. **Schema mismatch (negative):** rename field / wrong casing / wrong type in a fixture → conformance test fails. 3. **Stub strictness:** send unknown field to stub → reject; send valid msg → Transfer emitted. 4. **Real wasm:** instantiate real treasury + register window as spender → withdraw succeeds; unregistered spender → atomic revert (burn+withdraw fail together). 5. **Deposit regression:** deposit still forwards vFDUSD; unchanged. 6. **CI:** job clones/pins ustr-cmm rev and runs conformance (document skip conditions). --- ## Test plan (attack / hack / abuse vectors) 1. **Supply-chain schema bait:** attacker proposes MR that “updates” local stub to a wrong shape without updating conformance pin → CI must fail. 2. **Forward-compat trap:** stub accepts unknown fields that production rejects → ensure stub no longer hides this (deny unknown). 3. **Partial field omission:** missing `token`/`amount`/`recipient` → both stub and real treasury reject; window never builds incomplete msgs. 4. **Wrong recipient injection:** client builder must still bind recipient to cw20 Send sender only (existing INV); conformance must not weaken that. 5. **Spender not registered:** withdraw reverts atomically; no partial burn without payout (existing atomicity tests remain green). --- ## Verification criteria - Conformance test green against pinned ustr-cmm rev used for mainnet treasury code. - `cargo test` (window + integration) green with strict stubs. - Operator runbook lists probe tx; schema pin recorded in docs or CI config. - Audit C-1 marked addressed when the above lands on the deploy branch.

PlasticDigits commented 2026-08-08 04:57:28 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

mentioned in commit 7021c8661d568f3c0f3c717af5f5219c09257484

mentioned in commit 7021c8661d568f3c0f3c717af5f5219c09257484

PlasticDigits commented 2026-08-08 04:57:39 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

mentioned in merge request !42

mentioned in merge request !42

PlasticDigits commented 2026-08-08 04:58:05 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

mentioned in merge request !43

mentioned in merge request !43

PlasticDigits commented 2026-08-08 05:10:50 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

MR verification (issue #21 / !43)

Verdict: FAIL→FIXING (CI) — implementation complete locally; GitLab pipeline blocked merge.

Local tests (tip 7021c86+)

• cargo test -p ust1-window --lib — 20/20 ✅
• cargo test -p ust1-integration-tests --lib — 19/19 ✅
• scripts/verify_treasury_wire_schema.sh — ✅ (golden matches ustr-cmm @ e6c4b7cf)

AC status

All five acceptance criteria appear satisfied in code and tests.

CI failure (pipeline 2742608719, job rust)

rust:1.85-bookworm provides rustc 1.85.1, but Cargo.lock requires ≥1.86 (icu_*@2.2.0) and ≥1.88 (time@0.3.47). Compilation aborted before cargo test.

Fix applied: bump GitLab CI image to rust:1.88-bookworm on the MR branch.

Ops follow-up (out of scope)

Live withdraw probe after migrate + SetCw20Spender per DEPLOYMENT.md Phase 5.

## MR verification (issue #21 / !43) **Verdict: FAIL→FIXING (CI)** — implementation complete locally; GitLab pipeline blocked merge. ### Local tests (tip `7021c86`+) - `cargo test -p ust1-window --lib` — 20/20 ✅ - `cargo test -p ust1-integration-tests --lib` — 19/19 ✅ - `scripts/verify_treasury_wire_schema.sh` — ✅ (golden matches ustr-cmm @ `e6c4b7cf`) ### AC status All five acceptance criteria appear satisfied in code and tests. ### CI failure (pipeline 2742608719, job `rust`) `rust:1.85-bookworm` provides rustc 1.85.1, but `Cargo.lock` requires ≥1.86 (`icu_*@2.2.0`) and ≥1.88 (`time@0.3.47`). Compilation aborted before `cargo test`. **Fix applied:** bump GitLab CI image to `rust:1.88-bookworm` on the MR branch. ### Ops follow-up (out of scope) Live withdraw probe after migrate + `SetCw20Spender` per DEPLOYMENT.md Phase 5.

PlasticDigits commented 2026-08-08 05:16:05 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

mentioned in commit d8fa2081b7bccc744fd2ce0a77daa18fe826b304

mentioned in commit d8fa2081b7bccc744fd2ce0a77daa18fe826b304

PlasticDigits commented 2026-08-08 05:29:25 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

mentioned in commit 6a946fb870

mentioned in commit 6a946fb870bdbea4d7515f0d4f39d7639071b6fe

PlasticDigits commented 2026-08-08 05:35:31 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

Follow-up: CI fixes on !43

After rebase onto merged !37/!40/!35/!41:

1. Bumped GitLab image rust:1.85-bookworm → rust:1.88-bookworm (Cargo.lock MSRV).
2. Applied cargo fmt so cargo fmt --check passes.
3. Fixed clippy redundant_locals in oracle-service consensus tests (let rate = rate / let url = url) that -D warnings rejects in the new rust CI job.

Local gate before push: cargo fmt --check, cargo clippy --all-targets -- -D warnings, window/integration/oracle-service tests green.

## Follow-up: CI fixes on !43 After rebase onto merged !37/!40/!35/!41: 1. Bumped GitLab image `rust:1.85-bookworm` → `rust:1.88-bookworm` (Cargo.lock MSRV). 2. Applied `cargo fmt` so `cargo fmt --check` passes. 3. Fixed clippy `redundant_locals` in `oracle-service` consensus tests (`let rate = rate` / `let url = url`) that `-D warnings` rejects in the new `rust` CI job. Local gate before push: `cargo fmt --check`, `cargo clippy --all-targets -- -D warnings`, window/integration/oracle-service tests green.

PlasticDigits commented 2026-08-08 05:48:28 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

mentioned in commit a26793150e

mentioned in commit a26793150eabd6fcaef7491721e6b75033d6a4da

PlasticDigits (Migrated from gitlab.com) closed this issue 2026-08-08 05:48:28 +00:00

PlasticDigits commented 2026-08-08 05:48:51 +00:00 (Migrated from gitlab.com)

• Copy link
• Copy Markdown

MR !43 merged

CI is green (rust:1.88-bookworm + fmt/clippy/tests + verify_treasury_wire_schema.sh) and !43 is merged into main.

Remaining ops follow-up (out of MR scope): live withdraw probe after migrate + SetCw20Spender per DEPLOYMENT.md Phase 5.

## MR !43 merged CI is green (`rust:1.88-bookworm` + fmt/clippy/tests + `verify_treasury_wire_schema.sh`) and !43 is merged into `main`. Remaining ops follow-up (out of MR scope): live withdraw probe after migrate + `SetCw20Spender` per DEPLOYMENT.md Phase 5.

Sign in to join this conversation.

No Branch/Tag specified

Branches Tags

main

issue/50

renovate/configure

No results found.

Labels

Clear labels   

enhancement

  

governance

  

QA

  

testing

No labels

enhancement

governance

QA

testing

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

code/ust1-window#21

No description provided.