Enforce limit order price band to block dust-ask match DoS (#467) #1010

2026-07-07T02:21:41Z

PlasticDigits commented

2026-07-07 02:21:41 +00:00

(Migrated from gitlab.com)

Summary

Fixes GitLab #467: a dust-priced limit ask at Decimal::raw(1) (1e-18) could sort to the book head and make checked_mul_floor overflow on 1/price during hybrid match, reverting any crossing swap with a large enough token1 leg.

Add MIN_LIMIT_PRICE (1e-9) and MAX_LIMIT_PRICE (1e9) in dex-common::limit_placement with validate_limit_order_price.
Enforce the band on batch placement, ladder expansion, and UpdateLimitOrderPrice.
Belt-and-suspenders: match_bids / match_asks / simulate_match_* skip legacy out-of-band resting rows on reciprocal math overflow instead of aborting the whole swap.
Document invariant L19 and add make verify-issue-467.

Acceptance checklist

Criterion	Verification	Result
Dust ask at `raw(1)` rejected at placement	`cargo test -p cl8y-dex-tests place_limit_order_dust_price_rejected`	PASS
Ladder/batch cannot expand out-of-band rungs	`cargo test -p dex-common expand_ladder_rejects_out_of_band`	PASS
Valid crossing swap still fills after dust blocked	`cargo test -p cl8y-dex-tests dust_ask_brick_attack_prevented_valid_ask_still_fills`	PASS
Legacy dust head ask skipped (no whole-swap revert)	`cargo test -p cl8y-dex-pair match_asks_skips_legacy_dust_price_without_reverting`	PASS
Sub-unity in-band prices still work (#470)	`cargo test -p cl8y-dex-tests match_asks_skips_zero_cost_fill_sub_unity_price`	PASS
Docs + agent skill cross-linked	`make verify-issue-467` (docs/skill grep steps)	PASS
Full regression script	`make verify-issue-467`	PASS

Verification for third parties

git fetch origin fix/467-limit-price-band
git checkout fix/467-limit-price-band
make verify-issue-467

Optional broader sweep:

cd smartcontracts && cargo test -p cl8y-dex-tests limit_order_tests::
cd smartcontracts && cargo test -p cl8y-dex-pair orderbook::

Parent security umbrella: #381
Complements zero-cost fill skip: #470 (L18)

## Summary Fixes GitLab #467: a dust-priced limit ask at `Decimal::raw(1)` (1e-18) could sort to the book head and make `checked_mul_floor` overflow on `1/price` during hybrid match, reverting **any** crossing swap with a large enough token1 leg. - Add **`MIN_LIMIT_PRICE` (1e-9)** and **`MAX_LIMIT_PRICE` (1e9)** in `dex-common::limit_placement` with `validate_limit_order_price`. - Enforce the band on batch placement, ladder expansion, and `UpdateLimitOrderPrice`. - Belt-and-suspenders: `match_bids` / `match_asks` / `simulate_match_*` **skip** legacy out-of-band resting rows on reciprocal math overflow instead of aborting the whole swap. - Document invariant **L19** and add `make verify-issue-467`. ## Acceptance checklist | Criterion | Verification | Result | |-----------|--------------|--------| | Dust ask at `raw(1)` rejected at placement | `cargo test -p cl8y-dex-tests place_limit_order_dust_price_rejected` | PASS | | Ladder/batch cannot expand out-of-band rungs | `cargo test -p dex-common expand_ladder_rejects_out_of_band` | PASS | | Valid crossing swap still fills after dust blocked | `cargo test -p cl8y-dex-tests dust_ask_brick_attack_prevented_valid_ask_still_fills` | PASS | | Legacy dust head ask skipped (no whole-swap revert) | `cargo test -p cl8y-dex-pair match_asks_skips_legacy_dust_price_without_reverting` | PASS | | Sub-unity in-band prices still work (#470) | `cargo test -p cl8y-dex-tests match_asks_skips_zero_cost_fill_sub_unity_price` | PASS | | Docs + agent skill cross-linked | `make verify-issue-467` (docs/skill grep steps) | PASS | | Full regression script | `make verify-issue-467` | PASS | ## Verification for third parties ```bash git fetch origin fix/467-limit-price-band git checkout fix/467-limit-price-band make verify-issue-467 ``` Optional broader sweep: ```bash cd smartcontracts && cargo test -p cl8y-dex-tests limit_order_tests:: cd smartcontracts && cargo test -p cl8y-dex-pair orderbook:: ``` ## Related - Parent security umbrella: #381 - Complements zero-cost fill skip: #470 (L18)

ghost1 commented

2026-07-07 02:21:45 +00:00

(Migrated from gitlab.com)

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the [Cursor dashboard](https://www.cursor.com/dashboard/bugbot) to get automatic reviews on future PRs.

PlasticDigits commented

2026-07-07 02:24:09 +00:00

(Migrated from gitlab.com)

Security review — MR !1010

Commit reviewed: e5cb6f3f1c89b79808e79056993eafb0b860b5c6
Scope: Limit price band enforcement (MIN_LIMIT_PRICE / MAX_LIMIT_PRICE, validate_limit_order_price), placement/ladder/price-update gates, and match-time overflow skip in match_* / simulate_match_* (GitLab #467 / invariant L19). Docs, QA script, and integration tests only otherwise.

Method: Diff review; traced attacker-controlled price through PlaceLimitOrderBatch, ladder expansion, UpdateLimitOrderPrice, and hybrid book matching; checked auth boundaries, escrow accounting on skip paths, and overflow thresholds at band edges.

Outcome: FINDINGS: 0 medium+

Inline threads: none

Summary

This MR closes the documented dust-ask match DoS (extreme price at book head causing checked_mul_floor / 1/price overflow and whole-swap revert). Controls are appropriately layered:

Placement gate — validate_limit_order_price rejects out-of-band prices on batch items, ladder expansion (start/end and each rung), and UpdateLimitOrderPrice (owner-gated, unchanged).
Match belt-and-suspenders — try_price_inverse / try_mul_floor failures skip the resting row (continue) without debiting maker escrow or crediting the taker; walk advances via next_ptr.
Band math — at MIN_LIMIT_PRICE (1e-9), 1/price overflow requires taker notionals far above practical Uint128 swap sizes (~3.4e29 raw units), so in-band placement does not recreate the pre-fix brick vector at realistic sizes.

No authn/authz bypass, injection surface, secret leakage, or fund-extraction path identified in the added/modified code. Legacy out-of-band rows are intentionally skipped rather than matched; that is griefing mitigation, not a new theft vector. Indexer mirror parity for overflow-skip is out of this diff (quote accuracy only; on-chain execute remains authoritative).

Prior security-review threads: none on this MR; nothing to re-report.

Security review: no medium+ findings on this diff.

## Security review — MR !1010 **Commit reviewed:** `e5cb6f3f1c89b79808e79056993eafb0b860b5c6` **Scope:** Limit price band enforcement (`MIN_LIMIT_PRICE` / `MAX_LIMIT_PRICE`, `validate_limit_order_price`), placement/ladder/price-update gates, and match-time overflow skip in `match_*` / `simulate_match_*` (GitLab #467 / invariant L19). Docs, QA script, and integration tests only otherwise. **Method:** Diff review; traced attacker-controlled `price` through `PlaceLimitOrderBatch`, ladder expansion, `UpdateLimitOrderPrice`, and hybrid book matching; checked auth boundaries, escrow accounting on skip paths, and overflow thresholds at band edges. **Outcome:** `FINDINGS: 0` medium+ **Inline threads:** none ### Summary This MR closes the documented dust-ask match DoS (extreme `price` at book head causing `checked_mul_floor` / `1/price` overflow and whole-swap revert). Controls are appropriately layered: 1. **Placement gate** — `validate_limit_order_price` rejects out-of-band prices on batch items, ladder expansion (start/end and each rung), and `UpdateLimitOrderPrice` (owner-gated, unchanged). 2. **Match belt-and-suspenders** — `try_price_inverse` / `try_mul_floor` failures skip the resting row (`continue`) without debiting maker escrow or crediting the taker; walk advances via `next_ptr`. 3. **Band math** — at `MIN_LIMIT_PRICE` (1e-9), `1/price` overflow requires taker notionals far above practical `Uint128` swap sizes (~3.4e29 raw units), so in-band placement does not recreate the pre-fix brick vector at realistic sizes. No authn/authz bypass, injection surface, secret leakage, or fund-extraction path identified in the added/modified code. Legacy out-of-band rows are intentionally skipped rather than matched; that is griefing mitigation, not a new theft vector. Indexer mirror parity for overflow-skip is out of this diff (quote accuracy only; on-chain execute remains authoritative). **Prior security-review threads:** none on this MR; nothing to re-report. Security review: no medium+ findings on this diff.

PlasticDigits commented

2026-07-07 02:39:19 +00:00

(Migrated from gitlab.com)

added 5 commits

e5cb6f3f...529b34e1 - 4 commits from branch main
23a36d0f - Merge origin/main into fix/467-limit-price-band

Compare with previous version

added 5 commits <ul><li>e5cb6f3f...529b34e1 - 4 commits from branch <code>main</code></li><li>23a36d0f - Merge origin/main into fix/467-limit-price-band</li></ul> [Compare with previous version](/PlasticDigits/cl8y-dex-terraclassic/-/merge_requests/292/diffs?diff_id=1889046441&start_sha=e5cb6f3f1c89b79808e79056993eafb0b860b5c6)

PlasticDigits commented

2026-07-07 02:39:21 +00:00

(Migrated from gitlab.com)

Resolved merge conflicts with main:

docs/contracts-security-audit.md: kept L19 (blacklisted maker skip, #468 from main) and renumbered limit price band to L20 (#467).
orderbook.rs: combined module docs for both invariants; added missing blacklist_gate: None arg in limit_price_band_tests::match_asks_skips_legacy_dust_price_without_reverting.
Updated L20 cross-references in limit-orders.md, AGENTS_BOOK_MATCH_HINT_SECURITY.md, and verify-issue-467.sh.

Sanity: make verify-issue-467 — all 7 steps pass.

Resolved merge conflicts with `main`: - **`docs/contracts-security-audit.md`**: kept **L19** (blacklisted maker skip, #468 from main) and renumbered limit price band to **L20** (#467). - **`orderbook.rs`**: combined module docs for both invariants; added missing `blacklist_gate: None` arg in `limit_price_band_tests::match_asks_skips_legacy_dust_price_without_reverting`. - Updated L20 cross-references in `limit-orders.md`, `AGENTS_BOOK_MATCH_HINT_SECURITY.md`, and `verify-issue-467.sh`. Sanity: `make verify-issue-467` — all 7 steps pass.

ghost1 commented

2026-07-07 02:39:22 +00:00

(Migrated from gitlab.com)

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the [Cursor dashboard](https://www.cursor.com/dashboard/bugbot) to get automatic reviews on future PRs.

PlasticDigits commented

2026-07-07 02:41:48 +00:00

(Migrated from gitlab.com)

Security review — MR !1010

Commit reviewed: 23a36d0f4f04aa5fe4f6268c5857e25900bbc7e6
Scope: Post-merge-conflict re-review of limit price band enforcement (MIN_LIMIT_PRICE / MAX_LIMIT_PRICE, validate_limit_order_price), placement/ladder/price-update gates, and match-time overflow skip in match_* / simulate_match_* (GitLab #467 / invariant L20). Includes merge with main (L19 blacklist maker skip renumber, blacklist_gate test fix).

Method: Full MR diff review; traced attacker-controlled price through PlaceLimitOrderBatch, ladder expansion (expand_limit_ladder + per-rung validation), UpdateLimitOrderPrice, and relink_limit_order_price; verified skip paths do not debit maker escrow or credit takers; checked band-edge overflow thresholds vs realistic Uint128 swap notionals.

Outcome: FINDINGS: 0 medium+

Inline threads: none

Summary

This MR closes the documented dust-ask match DoS (extreme resting price at book head causing checked_mul_floor / 1/price overflow and whole-swap revert). Controls remain appropriately layered:

Placement gate — validate_limit_order_price rejects out-of-band prices on batch items, ladder expansion (start/end and each interpolated rung), UpdateLimitOrderPrice, and relink_limit_order_price. Owner/auth checks on price update unchanged.
Match belt-and-suspenders — try_price_inverse / try_mul_floor failures skip the resting row (continue via next_ptr) without makers_used++, escrow debits, or taker credits.
Band math — at MIN_LIMIT_PRICE (1e-9), 1/price mul_floor overflow requires taker notionals far above practical Uint128 swap sizes (~3.4e29 raw units), so in-band placement does not recreate the pre-fix brick vector at realistic sizes.

Merge-conflict resolution with main (L19/L20 renumber, blacklist_gate: None in unit test) does not weaken auth or introduce new attack surface.

No authn/authz bypass, injection surface, secret leakage, or fund-extraction path identified in the added/modified code. Legacy out-of-band rows are intentionally skipped rather than matched; that is griefing mitigation, not a new theft vector. Indexer mirror parity for overflow-skip is out of this diff (quote accuracy only; on-chain execute remains authoritative).

Prior security-review thread (e5cb6f3): conclusions still hold on this SHA; no new medium+ issues from the merge.

Security review: no medium+ findings on this diff.

## Security review — MR !1010 **Commit reviewed:** `23a36d0f4f04aa5fe4f6268c5857e25900bbc7e6` **Scope:** Post-merge-conflict re-review of limit price band enforcement (`MIN_LIMIT_PRICE` / `MAX_LIMIT_PRICE`, `validate_limit_order_price`), placement/ladder/price-update gates, and match-time overflow skip in `match_*` / `simulate_match_*` (GitLab #467 / invariant L20). Includes merge with `main` (L19 blacklist maker skip renumber, `blacklist_gate` test fix). **Method:** Full MR diff review; traced attacker-controlled `price` through `PlaceLimitOrderBatch`, ladder expansion (`expand_limit_ladder` + per-rung validation), `UpdateLimitOrderPrice`, and `relink_limit_order_price`; verified skip paths do not debit maker escrow or credit takers; checked band-edge overflow thresholds vs realistic `Uint128` swap notionals. **Outcome:** `FINDINGS: 0` medium+ **Inline threads:** none ### Summary This MR closes the documented dust-ask match DoS (extreme resting `price` at book head causing `checked_mul_floor` / `1/price` overflow and whole-swap revert). Controls remain appropriately layered: 1. **Placement gate** — `validate_limit_order_price` rejects out-of-band prices on batch items, ladder expansion (start/end and each interpolated rung), `UpdateLimitOrderPrice`, and `relink_limit_order_price`. Owner/auth checks on price update unchanged. 2. **Match belt-and-suspenders** — `try_price_inverse` / `try_mul_floor` failures skip the resting row (`continue` via `next_ptr`) without `makers_used++`, escrow debits, or taker credits. 3. **Band math** — at `MIN_LIMIT_PRICE` (1e-9), `1/price` mul_floor overflow requires taker notionals far above practical `Uint128` swap sizes (~3.4e29 raw units), so in-band placement does not recreate the pre-fix brick vector at realistic sizes. Merge-conflict resolution with `main` (L19/L20 renumber, `blacklist_gate: None` in unit test) does not weaken auth or introduce new attack surface. No authn/authz bypass, injection surface, secret leakage, or fund-extraction path identified in the added/modified code. Legacy out-of-band rows are intentionally skipped rather than matched; that is griefing mitigation, not a new theft vector. Indexer mirror parity for overflow-skip is out of this diff (quote accuracy only; on-chain execute remains authoritative). **Prior security-review thread** (`e5cb6f3`): conclusions still hold on this SHA; no new medium+ issues from the merge. Security review: no medium+ findings on this diff.

PlasticDigits commented

2026-07-07 02:49:16 +00:00

(Migrated from gitlab.com)

mentioned in commit 86cf616168

mentioned in commit 86cf6161689efb3b8b1557c1bd52d1bfd93080ec