Permissionless CreatePair one-per-block gate is grief-able #276
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#276
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?
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_pairone-per-block gate (smartcontracts/contracts/factory/src/contract.rs).Summary
CreatePairis permissionless and has a globalPAIR_CREATION_BLOCKgate — one pair creation per block across the whole factory. So an attacker who lands aCreatePairfirst in a block makes every legitimateCreatePairin that block fail withOnePairCreationPerBlock, and can repeat it to keep stalling pair creation.It's mitigated: both tokens must use whitelisted code ids and
PairAlreadyExistsblocks 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 }thenPAIR_CREATION_BLOCK.save(height)._infois unused — no caller restriction.Recommended direction
Acceptance criteria
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.
Implementation plan (your 100 LUNC creation-fee direction). Medium, factory-only + small frontend.
pair_creation_fee_ulunatoConfigwith#[serde(default = ...)]= 100_000_000 uluna so stored configs migrate cleanly (same pattern asdefault_limit_batch_max_rungs), plus to InstantiateMsg + ConfigResponse (dex-common/factory.rs). NewSetPairCreationFeegov setter — copyensure_governance+execute_set_pair_feeexactly.execute_create_pair: rename_info→info, validate attached funds ≥ fee withmust_pay(cw-utils, already a dep),BankMsg::Sendthe fee toconfig.treasury, newInsufficientFeeerror. Bump CONTRACT_VERSION 1.3→1.4 + the standard migrate.Heads up: there's no in-repo precedent for a
BankMsg::Sendto 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. @PlasticDigitsmentioned in merge request !754
Shipped your "100 LUNC creation fee, governance-settable" direction — MR !754.
CreatePairis now charged apair_creation_fee_uluna(default 100 LUNC, onConfig+InstantiateMsg+ConfigResponsewith#[serde(default)]so 1.3.x configs migrate to the default; CONTRACT_VERSION 1.3.0 -> 1.4.0).execute_create_pairrequires 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). NewSetPairCreationFeegovernance 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
mentioned in commit
244e8a2e62mentioned in commit
6ba08a4fb5mentioned in merge request !762
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 >= feeguard, and the excess refund now run regardless of the fee value; only the treasury send stays conditional onfee > 0. So with the fee disabled any attached uluna is fully refunded, and nothing can get stuck in any path. Addedcreate_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. @PlasticDigitsHeads 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 >= feeguard + excess refund now run regardless of the fee value; only the treasury send stays conditional onfee > 0, so nothing can get stuck in any path. Added create_pair_refunds_uluna_when_fee_disabled. Worth pulling in before the next deploy. @PlasticDigitsmentioned in merge request !763
mentioned in issue #313
marked as related to #313
mentioned in issue #318
marked as related to #318
mentioned in commit
4ab91ed936mentioned in issue #303
mentioned in issue #298
mentioned in commit
e48e676e90mentioned in commit
01e35696b7mentioned in commit
8e8c1ef41bmentioned in issue #274
mentioned in merge request !768
mentioned in commit
b4432f658fmentioned in merge request !769
mentioned in issue #320
marked as related to #320
mentioned in merge request !770
mentioned in commit
46dd8e90d1mentioned in merge request !772
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
CreatePairrequires attached uluna ≥config.pair_creation_fee_uluna(default 100 LUNC =100_000_000uluna), forwarded to treasury viaBankMsg::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 viaSetPairCreationFee. The globalPAIR_CREATION_BLOCKgate remains (one create flow per block), but blocking is no longer free.Automated checks (
make verify-issue-276)Additional explicit test:
Implementation evidence (on
main)execute_create_pairinsmartcontracts/contracts/factory/src/contract.rs: fee validation, treasury send, overpay refund (runs regardless of fee value; treasury send only whenfee > 0).SetPairCreationFeegovernance setter;CONTRACT_VERSION1.4.0 with migrate forpair_creation_fee_ulunadefault.docs/security-model.md,docs/contracts-terraclassic.md,docs/contracts-security-audit.md(invariant F2).scripts/deploy-dex-local.shattaches fee on everycreate_pair.Notes (non-blocking)
createPairmust 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.0via governance, callers attaching uluna by mistake are fully refunded (hardening test above).Closing — all acceptance criteria pass; mitigation shipped on
main.mentioned in issue #325
mentioned in merge request !783
mentioned in commit
9ec5c0db10mentioned in merge request !785
mentioned in commit
ce26e061f1mentioned in merge request !789
mentioned in issue #345
mentioned in issue #337
mentioned in issue #542
mentioned in issue #586
mentioned in issue #595