Permissionless CreatePair one-per-block gate is grief-able #276

Closed
opened 2026-06-03 07:11:54 +00:00 by Brouie · 41 comments
Brouie commented 2026-06-03 07:11:54 +00:00 (Migrated from gitlab.com)

Severity: Informational
Reachability: Permissionless, but self-limiting — the attacker pays create gas and must deploy a fresh whitelisted token every block to sustain it.
Affected: execute_create_pair one-per-block gate (smartcontracts/contracts/factory/src/contract.rs).

Summary

CreatePair is permissionless and has a global PAIR_CREATION_BLOCK gate — one pair creation per block across the whole factory. So an attacker who lands a CreatePair first in a block makes every legitimate CreatePair in that block fail with OnePairCreationPerBlock, and can repeat it to keep stalling pair creation.

It's mitigated: both tokens must use whitelisted code ids and PairAlreadyExists blocks dupes, so to grief every block the attacker has to deploy a fresh whitelisted CW20 (or a new unique combo) and pay create gas each block, while honest users just retry next block. No fund or state risk — filing it as informational so the global gate is a conscious choice, not an accident.

Current codebase

  • execute_create_pair: if PAIR_CREATION_BLOCK.may_load()?.is_some_and(|h| h == height) { return OnePairCreationPerBlock } then PAIR_CREATION_BLOCK.save(height). _info is unused — no caller restriction.
  • If the gate exists to bound per-block instantiate load, make it per-sender instead of global, or add a tiny creation fee so sustained griefing has a real cost. If it exists for another reason, document it.

Acceptance criteria

  • A single sender cannot indefinitely block other senders from creating pairs at zero marginal cost.
**Severity:** Informational **Reachability:** Permissionless, but self-limiting — the attacker pays create gas and must deploy a fresh whitelisted token every block to sustain it. **Affected:** `execute_create_pair` one-per-block gate (`smartcontracts/contracts/factory/src/contract.rs`). ## Summary `CreatePair` is permissionless and has a global `PAIR_CREATION_BLOCK` gate — one pair creation per block across the whole factory. So an attacker who lands a `CreatePair` first in a block makes every legitimate `CreatePair` in that block fail with `OnePairCreationPerBlock`, and can repeat it to keep stalling pair creation. It's mitigated: both tokens must use whitelisted code ids and `PairAlreadyExists` blocks dupes, so to grief every block the attacker has to deploy a fresh whitelisted CW20 (or a new unique combo) and pay create gas each block, while honest users just retry next block. No fund or state risk — filing it as informational so the global gate is a conscious choice, not an accident. ## Current codebase - `execute_create_pair`: `if PAIR_CREATION_BLOCK.may_load()?.is_some_and(|h| h == height) { return OnePairCreationPerBlock }` then `PAIR_CREATION_BLOCK.save(height)`. `_info` is unused — no caller restriction. ## Recommended direction - If the gate exists to bound per-block instantiate load, make it per-sender instead of global, or add a tiny creation fee so sustained griefing has a real cost. If it exists for another reason, document it. ## Acceptance criteria - [ ] A single sender cannot indefinitely block other senders from creating pairs at zero marginal cost.
PlasticDigits commented 2026-06-03 10:36:31 +00:00 (Migrated from gitlab.com)

Add a 100 LUNC fee to pair creation that is transferred to treasury to mitigate this attack. It should be governance settable so if an attack is observed, it can be updated to increase the cost.

Add a 100 LUNC fee to pair creation that is transferred to treasury to mitigate this attack. It should be governance settable so if an attack is observed, it can be updated to increase the cost.
Brouie commented 2026-06-04 06:28:51 +00:00 (Migrated from gitlab.com)

Implementation plan (your 100 LUNC creation-fee direction). Medium, factory-only + small frontend.

  • Add pair_creation_fee_uluna to Config with #[serde(default = ...)] = 100_000_000 uluna so stored configs migrate cleanly (same pattern as default_limit_batch_max_rungs), plus to InstantiateMsg + ConfigResponse (dex-common/factory.rs). New SetPairCreationFee gov setter — copy ensure_governance + execute_set_pair_fee exactly.
  • In execute_create_pair: rename _info→info, validate attached funds ≥ fee with must_pay (cw-utils, already a dep), BankMsg::Send the fee to config.treasury, new InsufficientFee error. Bump CONTRACT_VERSION 1.3→1.4 + the standard migrate.
  • Files: dex-common/factory.rs, factory state.rs/error.rs/contract.rs, tests/lib.rs (instantiate literals gain the field), frontend factory.ts (attach the uluna coins on createPair) + types.

Heads up: there's no in-repo precedent for a BankMsg::Send to treasury from a contract (only in tests/frontend) — that part is new, worth careful review. Frontend ripple is additive (createPair must attach the fee; CreatePairPage should surface it) — that's a laptop follow-up. Off-chain scripts calling CreatePair with empty funds will start failing InsufficientFee (intended). Test: exact-fee succeeds + treasury balance +fee; underpay rejects; gov can raise the fee. @PlasticDigits

Implementation plan (your 100 LUNC creation-fee direction). Medium, factory-only + small frontend. - Add `pair_creation_fee_uluna` to `Config` with `#[serde(default = ...)]` = 100_000_000 uluna so stored configs migrate cleanly (same pattern as `default_limit_batch_max_rungs`), plus to InstantiateMsg + ConfigResponse (dex-common/factory.rs). New `SetPairCreationFee` gov setter — copy `ensure_governance` + `execute_set_pair_fee` exactly. - In `execute_create_pair`: rename `_info`→`info`, validate attached funds ≥ fee with `must_pay` (cw-utils, already a dep), `BankMsg::Send` the fee to `config.treasury`, new `InsufficientFee` error. Bump CONTRACT_VERSION 1.3→1.4 + the standard migrate. - Files: dex-common/factory.rs, factory state.rs/error.rs/contract.rs, tests/lib.rs (instantiate literals gain the field), frontend factory.ts (attach the uluna coins on createPair) + types. **Heads up:** there's no in-repo precedent for a `BankMsg::Send` to treasury from a contract (only in tests/frontend) — that part is new, worth careful review. Frontend ripple is additive (createPair must attach the fee; CreatePairPage should surface it) — that's a laptop follow-up. Off-chain scripts calling CreatePair with empty funds will start failing InsufficientFee (intended). Test: exact-fee succeeds + treasury balance +fee; underpay rejects; gov can raise the fee. @PlasticDigits
Brouie commented 2026-06-05 02:44:58 +00:00 (Migrated from gitlab.com)

mentioned in merge request !754

mentioned in merge request !754
Brouie commented 2026-06-05 02:45:07 +00:00 (Migrated from gitlab.com)

Shipped your "100 LUNC creation fee, governance-settable" direction — MR !754.

CreatePair is now charged a pair_creation_fee_uluna (default 100 LUNC, on Config + InstantiateMsg + ConfigResponse with #[serde(default)] so 1.3.x configs migrate to the default; CONTRACT_VERSION 1.3.0 -> 1.4.0). execute_create_pair requires the fee in uluna, BankMsg::Sends it to treasury, rejects stray denoms, and refunds any overpay; it's atomic with the pair instantiate (instantiate fails -> tx reverts -> no fee charged). New SetPairCreationFee governance setter so the cost can be raised if spam shows up.

Tests: empty attach rejected, exact fee credits treasury, gov raises the fee (visible in Config), non-gov blocked. Contract suite 416/0.

Two heads-ups: (1) there was no in-repo precedent for a contract BankMsg::Send to treasury, so the fund handling is worth a careful review pass; (2) the frontend ripple is a laptop follow-up — createPair must attach the uluna fee and the CreatePair page should surface it, and any off-chain scripts calling CreatePair with no funds will start failing InsufficientPairCreationFee (intended). @PlasticDigits

Shipped your "100 LUNC creation fee, governance-settable" direction — MR !754. `CreatePair` is now charged a `pair_creation_fee_uluna` (default 100 LUNC, on `Config` + `InstantiateMsg` + `ConfigResponse` with `#[serde(default)]` so 1.3.x configs migrate to the default; CONTRACT_VERSION 1.3.0 -> 1.4.0). `execute_create_pair` requires the fee in uluna, `BankMsg::Send`s it to treasury, rejects stray denoms, and refunds any overpay; it's atomic with the pair instantiate (instantiate fails -> tx reverts -> no fee charged). New `SetPairCreationFee` governance setter so the cost can be raised if spam shows up. Tests: empty attach rejected, exact fee credits treasury, gov raises the fee (visible in Config), non-gov blocked. Contract suite 416/0. Two heads-ups: (1) there was no in-repo precedent for a contract BankMsg::Send to treasury, so the fund handling is worth a careful review pass; (2) the frontend ripple is a laptop follow-up — createPair must attach the uluna fee and the CreatePair page should surface it, and any off-chain scripts calling CreatePair with no funds will start failing InsufficientPairCreationFee (intended). @PlasticDigits
PlasticDigits commented 2026-06-05 03:18:12 +00:00 (Migrated from gitlab.com)

mentioned in commit 244e8a2e62

mentioned in commit 244e8a2e627532b90684006c878742eca95abdc9
ghost1 commented 2026-06-05 03:52:06 +00:00 (Migrated from gitlab.com)

mentioned in commit 6ba08a4fb5

mentioned in commit 6ba08a4fb55372feec805e7524c4dd048bf1b3b3
PlasticDigits commented 2026-06-05 03:52:37 +00:00 (Migrated from gitlab.com)

mentioned in merge request !762

mentioned in merge request !762
Brouie commented 2026-06-05 03:54:53 +00:00 (Migrated from gitlab.com)

Did a self-review pass on the BankMsg fund handling (it's the one part with no in-repo precedent) and caught a stuck-funds edge before it could ship — pushed the fix onto the same MR (!754).

The gap: the whole fee block was gated on fee != 0, so if you ever DISABLE the fee (set it to 0) and someone mistakenly attaches uluna, it was neither forwarded nor refunded — it'd sit stuck in the factory. A stray denom while the fee's off had the same hole.

Tightened it: the denom check, the paid >= fee guard, and the excess refund now run regardless of the fee value; only the treasury send stays conditional on fee > 0. So with the fee disabled any attached uluna is fully refunded, and nothing can get stuck in any path. Added create_pair_refunds_uluna_when_fee_disabled (fee=0 + attach uluna -> treasury 0, factory 0, full refund to sender) to lock it in. Suite 417/0. @PlasticDigits

Did a self-review pass on the BankMsg fund handling (it's the one part with no in-repo precedent) and caught a stuck-funds edge before it could ship — pushed the fix onto the same MR (!754). The gap: the whole fee block was gated on `fee != 0`, so if you ever DISABLE the fee (set it to 0) and someone mistakenly attaches uluna, it was neither forwarded nor refunded — it'd sit stuck in the factory. A stray denom while the fee's off had the same hole. Tightened it: the denom check, the `paid >= fee` guard, and the excess refund now run regardless of the fee value; only the treasury send stays conditional on `fee > 0`. So with the fee disabled any attached uluna is fully refunded, and nothing can get stuck in any path. Added `create_pair_refunds_uluna_when_fee_disabled` (fee=0 + attach uluna -> treasury 0, factory 0, full refund to sender) to lock it in. Suite 417/0. @PlasticDigits
Brouie commented 2026-06-05 04:02:32 +00:00 (Migrated from gitlab.com)

Heads up — after the merge I did a self-review of the BankMsg fund handling and found a stuck-funds edge that's now on main, so I pushed a fix as a fresh MR (the original !754 had already merged without it).

The hole: the fee block was gated on fee != 0, so if you ever DISABLE the fee (set it 0) and someone mistakenly attaches uluna, it was neither forwarded nor refunded — stuck in the factory. Stray denom while the fee's off, same. Fixed: the denom check + paid >= fee guard + excess refund now run regardless of the fee value; only the treasury send stays conditional on fee > 0, so nothing can get stuck in any path. Added create_pair_refunds_uluna_when_fee_disabled. Worth pulling in before the next deploy. @PlasticDigits

Heads up — after the merge I did a self-review of the BankMsg fund handling and found a stuck-funds edge that's now on main, so I pushed a fix as a fresh MR (the original !754 had already merged without it). The hole: the fee block was gated on `fee != 0`, so if you ever DISABLE the fee (set it 0) and someone mistakenly attaches uluna, it was neither forwarded nor refunded — stuck in the factory. Stray denom while the fee's off, same. Fixed: the denom check + `paid >= fee` guard + excess refund now run regardless of the fee value; only the treasury send stays conditional on `fee > 0`, so nothing can get stuck in any path. Added create_pair_refunds_uluna_when_fee_disabled. Worth pulling in before the next deploy. @PlasticDigits
Brouie commented 2026-06-05 04:02:32 +00:00 (Migrated from gitlab.com)

mentioned in merge request !763

mentioned in merge request !763
PlasticDigits commented 2026-06-05 04:08:28 +00:00 (Migrated from gitlab.com)

mentioned in issue #313

mentioned in issue #313
PlasticDigits commented 2026-06-05 04:08:28 +00:00 (Migrated from gitlab.com)

marked as related to #313

marked as related to #313
PlasticDigits commented 2026-06-05 04:12:10 +00:00 (Migrated from gitlab.com)

mentioned in issue #318

mentioned in issue #318
PlasticDigits commented 2026-06-05 04:12:11 +00:00 (Migrated from gitlab.com)

marked as related to #318

marked as related to #318
PlasticDigits commented 2026-06-05 04:13:04 +00:00 (Migrated from gitlab.com)

mentioned in commit 4ab91ed936

mentioned in commit 4ab91ed9366656445edbf4a07162f67a9c9aa83b
PlasticDigits commented 2026-06-05 04:30:32 +00:00 (Migrated from gitlab.com)

mentioned in issue #303

mentioned in issue #303
PlasticDigits commented 2026-06-05 04:31:45 +00:00 (Migrated from gitlab.com)

mentioned in issue #298

mentioned in issue #298
PlasticDigits commented 2026-06-05 04:35:51 +00:00 (Migrated from gitlab.com)

mentioned in commit e48e676e90

mentioned in commit e48e676e904ad4ca8d49665d8d09f7e5624efc5a
ghost1 commented 2026-06-05 04:35:57 +00:00 (Migrated from gitlab.com)

mentioned in commit 01e35696b7

mentioned in commit 01e35696b70274ad47b003053b4b5f77e02643c1
ghost1 commented 2026-06-05 04:47:54 +00:00 (Migrated from gitlab.com)

mentioned in commit 8e8c1ef41b

mentioned in commit 8e8c1ef41b212bcb73f8550fe1adc445ad537c67
PlasticDigits commented 2026-06-05 04:48:57 +00:00 (Migrated from gitlab.com)

mentioned in issue #274

mentioned in issue #274
PlasticDigits commented 2026-06-05 05:00:05 +00:00 (Migrated from gitlab.com)

mentioned in merge request !768

mentioned in merge request !768
ghost1 commented 2026-06-05 05:24:02 +00:00 (Migrated from gitlab.com)

mentioned in commit b4432f658f

mentioned in commit b4432f658f0b8a3b83ba1057298a0d2acce4d185
PlasticDigits commented 2026-06-05 05:39:44 +00:00 (Migrated from gitlab.com)

mentioned in merge request !769

mentioned in merge request !769
PlasticDigits commented 2026-06-05 05:42:24 +00:00 (Migrated from gitlab.com)

mentioned in issue #320

mentioned in issue #320
PlasticDigits commented 2026-06-05 05:42:33 +00:00 (Migrated from gitlab.com)

marked as related to #320

marked as related to #320
PlasticDigits commented 2026-06-05 05:52:34 +00:00 (Migrated from gitlab.com)

mentioned in merge request !770

mentioned in merge request !770
PlasticDigits commented 2026-06-05 06:02:25 +00:00 (Migrated from gitlab.com)

mentioned in commit 46dd8e90d1

mentioned in commit 46dd8e90d1a507fb5c83b27eacac54faa4ceaf47
Brouie commented 2026-06-05 06:10:49 +00:00 (Migrated from gitlab.com)

mentioned in merge request !772

mentioned in merge request !772
PlasticDigits commented 2026-06-05 09:35:40 +00:00 (Migrated from gitlab.com)

Verification complete — GitLab #276

Issue: https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/work_items/276

Verified on branch main (no repo changes from this verification run).

Acceptance criteria

Item Result How verified
A single sender cannot indefinitely block other senders from creating pairs at zero marginal cost PASS CreatePair requires attached uluna ≥ config.pair_creation_fee_uluna (default 100 LUNC = 100_000_000 uluna), forwarded to treasury via BankMsg::Send. Sustained one-per-block griefing therefore costs the attacker the fee every block (plus gas and fresh whitelisted tokens). Governance can raise the cost via SetPairCreationFee. The global PAIR_CREATION_BLOCK gate remains (one create flow per block), but blocking is no longer free.

Automated checks (make verify-issue-276)

PASS  factory fee test (empty attach rejected, treasury credited, gov setter)
PASS  one-per-block gate regression
PASS  full contract suite (362/362)
PASS  docs mention #276 / pair_creation_fee_uluna
PASS  local deploy attaches pair_creation_fee on create_pair (#318)

Additional explicit test:

cd smartcontracts && cargo test create_pair_refunds_uluna_when_fee_disabled --quiet
→ 1 passed (fee=0 + attached uluna → full refund, nothing stuck in factory/treasury)

Implementation evidence (on main)

  • execute_create_pair in smartcontracts/contracts/factory/src/contract.rs: fee validation, treasury send, overpay refund (runs regardless of fee value; treasury send only when fee > 0).
  • SetPairCreationFee governance setter; CONTRACT_VERSION 1.4.0 with migrate for pair_creation_fee_uluna default.
  • Docs: docs/security-model.md, docs/contracts-terraclassic.md, docs/contracts-security-audit.md (invariant F2).
  • scripts/deploy-dex-local.sh attaches fee on every create_pair.

Notes (non-blocking)

  • Frontend createPair must attach the uluna fee and surface it on CreatePair page — tracked as a laptop follow-up in implementation comments; not part of on-chain acceptance criteria.
  • If fee is ever set to 0 via governance, callers attaching uluna by mistake are fully refunded (hardening test above).

Closing — all acceptance criteria pass; mitigation shipped on main.

## Verification complete — GitLab #276 **Issue:** https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/work_items/276 Verified on branch `main` (no repo changes from this verification run). ### Acceptance criteria | Item | Result | How verified | |------|--------|--------------| | A single sender cannot indefinitely block other senders from creating pairs at **zero marginal cost** | **PASS** | `CreatePair` requires attached **uluna** ≥ `config.pair_creation_fee_uluna` (default **100 LUNC** = `100_000_000` uluna), forwarded to treasury via `BankMsg::Send`. Sustained one-per-block griefing therefore costs the attacker the fee every block (plus gas and fresh whitelisted tokens). Governance can raise the cost via `SetPairCreationFee`. The global `PAIR_CREATION_BLOCK` gate remains (one create flow per block), but blocking is no longer free. | ### Automated checks (`make verify-issue-276`) ``` PASS factory fee test (empty attach rejected, treasury credited, gov setter) PASS one-per-block gate regression PASS full contract suite (362/362) PASS docs mention #276 / pair_creation_fee_uluna PASS local deploy attaches pair_creation_fee on create_pair (#318) ``` Additional explicit test: ``` cd smartcontracts && cargo test create_pair_refunds_uluna_when_fee_disabled --quiet → 1 passed (fee=0 + attached uluna → full refund, nothing stuck in factory/treasury) ``` ### Implementation evidence (on `main`) - `execute_create_pair` in `smartcontracts/contracts/factory/src/contract.rs`: fee validation, treasury send, overpay refund (runs regardless of fee value; treasury send only when `fee > 0`). - `SetPairCreationFee` governance setter; `CONTRACT_VERSION` **1.4.0** with migrate for `pair_creation_fee_uluna` default. - Docs: `docs/security-model.md`, `docs/contracts-terraclassic.md`, `docs/contracts-security-audit.md` (invariant **F2**). - `scripts/deploy-dex-local.sh` attaches fee on every `create_pair`. ### Notes (non-blocking) - Frontend `createPair` must attach the uluna fee and surface it on CreatePair page — tracked as a laptop follow-up in implementation comments; not part of on-chain acceptance criteria. - If fee is ever set to `0` via governance, callers attaching uluna by mistake are fully refunded (hardening test above). **Closing** — all acceptance criteria pass; mitigation shipped on `main`.
PlasticDigits (Migrated from gitlab.com) closed this issue 2026-06-05 09:35:44 +00:00
totdking commented 2026-06-05 09:48:34 +00:00 (Migrated from gitlab.com)

mentioned in issue #325

mentioned in issue #325
PlasticDigits commented 2026-06-05 09:55:26 +00:00 (Migrated from gitlab.com)

mentioned in merge request !783

mentioned in merge request !783
ghost1 commented 2026-06-05 09:58:42 +00:00 (Migrated from gitlab.com)

mentioned in commit 9ec5c0db10

mentioned in commit 9ec5c0db10b6a9844353aaea71ff75582e21a61f
PlasticDigits commented 2026-06-05 10:01:02 +00:00 (Migrated from gitlab.com)

mentioned in merge request !785

mentioned in merge request !785
ghost1 commented 2026-06-05 10:06:43 +00:00 (Migrated from gitlab.com)

mentioned in commit ce26e061f1

mentioned in commit ce26e061f1c169e549252e9514d8b6e45c6a3c82
PlasticDigits commented 2026-06-05 10:07:22 +00:00 (Migrated from gitlab.com)

mentioned in merge request !789

mentioned in merge request !789
Brouie commented 2026-06-09 02:58:22 +00:00 (Migrated from gitlab.com)

mentioned in issue #345

mentioned in issue #345
Brouie commented 2026-06-09 02:59:17 +00:00 (Migrated from gitlab.com)

mentioned in issue #337

mentioned in issue #337
PlasticDigits commented 2026-08-17 03:45:49 +00:00 (Migrated from gitlab.com)

mentioned in issue #542

mentioned in issue #542
PlasticDigits commented 2026-08-21 00:21:04 +00:00 (Migrated from gitlab.com)

mentioned in issue #586

mentioned in issue #586
PlasticDigits commented 2026-08-22 11:02:34 +00:00 (Migrated from gitlab.com)

mentioned in issue #595

mentioned in issue #595
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
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/cl8y-dex-terraclassic#276
No description provided.