pair/router: default-deny blacklist guard on factory query error (#456) #993

2026-06-30T22:19:27Z

PlasticDigits commented

2026-06-30 22:19:27 +00:00

(Migrated from gitlab.com)

Summary

Addresses GitLab #456 (SEC-I03 F-02): pair and router blacklist guards no longer fail open when the factory BlacklistCheck query returns an error.

Per maintainer feedback on #456, pre-1.5.0 factory backwards-compatibility is removed (not yet deployed on mainnet). Any factory BlacklistCheck query error now returns ContractError::BlacklistGuardUnavailable — an observable, fail-closed signal instead of silently disabling blacklist enforcement.

Changes:

pair/src/blacklist_guard.rs and router/src/blacklist_guard.rs: error arm returns BlacklistGuardUnavailable directly (no version gate, no Ok(None) / Ok(()) pass-through).
Removed dex_common::blacklist::blacklist_query_error_blocks and cw2 version probe (only ran on error path in !982; no longer needed).
Integration test factory_blacklist_query_error_blocks_swap simulates a stale factory pointer.
Docs/skills updated: security-model.md, contracts-security-audit.md (B1), AGENTS_BLACKLIST_DECISION.md.

Acceptance checklist

Criterion	Verification	Result
Error arm on factory query updated to block (no silent pass)	Code review: `blacklist_guard.rs` (pair + router)	PASS
Test: factory query error causes guard to block	`cd smartcontracts && cargo test -p cl8y-dex-tests factory_blacklist_query_error_blocks_swap`	PASS
Full blacklist integration suite unchanged (happy path)	`cd smartcontracts && cargo test -p cl8y-dex-tests blacklist_tests -- --test-threads=1` → 9/9	PASS
Docs updated (pre-1.5.0 compat removed)	`docs/security-model.md`, `docs/contracts-security-audit.md`, `skills/AGENTS_BLACKLIST_DECISION.md`	PASS
`make test-contracts` full workspace	`make test-contracts` → 388/389 pass; 1 pre-existing unrelated failure (`pair_coverage_tests::test_empty_pool_provide_rejects_asset_decimals_above_bootstrap_cap` on `main`)	SKIP (pre-existing)

Third-party verification

# New fail-closed guard test
cd smartcontracts && cargo test -p cl8y-dex-tests factory_blacklist_query_error_blocks_swap

# Full blacklist suite (9 tests)
cd smartcontracts && cargo test -p cl8y-dex-tests blacklist_tests -- --test-threads=1

# Confirm error arm has no fail-open (grep)
rg 'Ok\(None\)|fail.?open|pre-1\.5' smartcontracts/contracts/pair/src/blacklist_guard.rs smartcontracts/contracts/router/src/blacklist_guard.rs
# Expected: no matches

# Doc cross-links
rg 'SEC-I03 F02|#456|BlacklistGuardUnavailable|fail-closed' docs/security-model.md docs/contracts-security-audit.md skills/AGENTS_BLACKLIST_DECISION.md

## Summary Addresses GitLab #456 (SEC-I03 F-02): pair and router blacklist guards no longer fail open when the factory `BlacklistCheck` query returns an error. Per maintainer feedback on #456, **pre-1.5.0 factory backwards-compatibility is removed** (not yet deployed on mainnet). Any factory `BlacklistCheck` query error now returns `ContractError::BlacklistGuardUnavailable` — an observable, fail-closed signal instead of silently disabling blacklist enforcement. Changes: - `pair/src/blacklist_guard.rs` and `router/src/blacklist_guard.rs`: error arm returns `BlacklistGuardUnavailable` directly (no version gate, no `Ok(None)` / `Ok(())` pass-through). - Removed `dex_common::blacklist::blacklist_query_error_blocks` and cw2 version probe (only ran on error path in !982; no longer needed). - Integration test `factory_blacklist_query_error_blocks_swap` simulates a stale factory pointer. - Docs/skills updated: `security-model.md`, `contracts-security-audit.md` (B1), `AGENTS_BLACKLIST_DECISION.md`. ## Acceptance checklist | Criterion | Verification | Result | |-----------|--------------|--------| | Error arm on factory query updated to block (no silent pass) | Code review: `blacklist_guard.rs` (pair + router) | **PASS** | | Test: factory query error causes guard to block | `cd smartcontracts && cargo test -p cl8y-dex-tests factory_blacklist_query_error_blocks_swap` | **PASS** | | Full blacklist integration suite unchanged (happy path) | `cd smartcontracts && cargo test -p cl8y-dex-tests blacklist_tests -- --test-threads=1` → 9/9 | **PASS** | | Docs updated (pre-1.5.0 compat removed) | `docs/security-model.md`, `docs/contracts-security-audit.md`, `skills/AGENTS_BLACKLIST_DECISION.md` | **PASS** | | `make test-contracts` full workspace | `make test-contracts` → 388/389 pass; 1 pre-existing unrelated failure (`pair_coverage_tests::test_empty_pool_provide_rejects_asset_decimals_above_bootstrap_cap` on `main`) | **SKIP** (pre-existing) | ## Third-party verification ```bash # New fail-closed guard test cd smartcontracts && cargo test -p cl8y-dex-tests factory_blacklist_query_error_blocks_swap # Full blacklist suite (9 tests) cd smartcontracts && cargo test -p cl8y-dex-tests blacklist_tests -- --test-threads=1 # Confirm error arm has no fail-open (grep) rg 'Ok\(None\)|fail.?open|pre-1\.5' smartcontracts/contracts/pair/src/blacklist_guard.rs smartcontracts/contracts/router/src/blacklist_guard.rs # Expected: no matches # Doc cross-links rg 'SEC-I03 F02|#456|BlacklistGuardUnavailable|fail-closed' docs/security-model.md docs/contracts-security-audit.md skills/AGENTS_BLACKLIST_DECISION.md ```

PlasticDigits commented

2026-06-30 22:21:27 +00:00

(Migrated from gitlab.com)

Security review — MR !993

Commit reviewed: f5085087abc5c3ad374b11184723a6ad5577cca2

Scope: Pair/router blacklist_guard.rs default-deny on factory BlacklistCheck query errors; removal of dex_common::blacklist::blacklist_query_error_blocks cw2 version probe; integration test factory_blacklist_query_error_blocks_swap; docs/skills updates for SEC-I03 F02 (#456).

Outcome: FINDINGS: 0 medium+

Summary: This diff closes SEC-I03 F-02 (silent fail-open when BlacklistCheck errors). Both guards now map any factory query Err to BlacklistGuardUnavailable, removing the pre-1.5.0 / unreadable-factory pass-through that could disable blacklist enforcement after a stale factory pointer.

Attack-path review (no exploitable medium+ issues):

Vector	Result
Blacklist bypass via query error / stale factory	Mitigated — error arm is fail-closed in pair (`probe_factory_blacklist`) and router (`assert_router_swap_not_blacklisted`).
Attacker-controlled factory pointer	Not reachable — `pair_info.factory` / router `FACTORY` are set at deploy/migrate by factory governance, not user execute input.
Invalid addresses in `BlacklistCheck` payload	Not attacker-driven — wallets/trader fields are `addr_validate`d; token list is pair assets + validated CW20 sender; router tokens/pairs come from resolved swap ops.
Availability / trading halt on query error	Intentional fail-closed — requires misconfiguration or factory outage; not a permission-boundary bypass.
Injection / authz / SSRF / XSS / secrets	N/A — on-chain CosmWasm query path only; no new external I/O or off-chain surfaces.

Tests run: cargo test -p cl8y-dex-tests factory_blacklist_query_error_blocks_swap — pass.

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

Inline findings: none (no medium+ issues).

## Security review — MR !993 **Commit reviewed:** `f5085087abc5c3ad374b11184723a6ad5577cca2` **Scope:** Pair/router `blacklist_guard.rs` default-deny on factory `BlacklistCheck` query errors; removal of `dex_common::blacklist::blacklist_query_error_blocks` cw2 version probe; integration test `factory_blacklist_query_error_blocks_swap`; docs/skills updates for SEC-I03 F02 (#456). **Outcome:** `FINDINGS: 0` medium+ **Summary:** This diff closes SEC-I03 F-02 (silent fail-open when `BlacklistCheck` errors). Both guards now map any factory query `Err` to `BlacklistGuardUnavailable`, removing the pre-1.5.0 / unreadable-factory pass-through that could disable blacklist enforcement after a stale factory pointer. **Attack-path review (no exploitable medium+ issues):** | Vector | Result | |--------|--------| | Blacklist bypass via query error / stale factory | **Mitigated** — error arm is fail-closed in pair (`probe_factory_blacklist`) and router (`assert_router_swap_not_blacklisted`). | | Attacker-controlled factory pointer | **Not reachable** — `pair_info.factory` / router `FACTORY` are set at deploy/migrate by factory governance, not user execute input. | | Invalid addresses in `BlacklistCheck` payload | **Not attacker-driven** — wallets/trader fields are `addr_validate`d; token list is pair assets + validated CW20 sender; router tokens/pairs come from resolved swap ops. | | Availability / trading halt on query error | **Intentional fail-closed** — requires misconfiguration or factory outage; not a permission-boundary bypass. | | Injection / authz / SSRF / XSS / secrets | **N/A** — on-chain CosmWasm query path only; no new external I/O or off-chain surfaces. | **Tests run:** `cargo test -p cl8y-dex-tests factory_blacklist_query_error_blocks_swap` — pass. **Prior security threads:** none on this MR; nothing to re-report. **Inline findings:** none (no medium+ issues).

PlasticDigits commented

2026-07-01 00:06:46 +00:00

(Migrated from gitlab.com)

mentioned in commit 11c4fb4788

mentioned in commit 11c4fb4788a79e9dd86f95485a21072c262f6442