pair/router: version-gate the blacklist-guard fail-open (#456) #982

Merged

Brouie merged 1 commit from qa/456-blacklist-failopen-gate into main

2026-06-30 21:57:09 +00:00

Brouie commented

2026-06-30 19:04:24 +00:00

(Migrated from gitlab.com)

New dex_common::blacklist::blacklist_query_error_blocks() reads the factory's
cw2 version from raw storage (contract_info) and decides:

factory reachable AND version >= 1.5.0 -> BlacklistCheck must work; an error is
anomalous -> BLOCK (new ContractError::BlacklistGuardUnavailable, an observable
signal instead of a silent pass)
factory reachable AND version < 1.5.0 -> documented pre-1.5.0 compat -> fail open
version unreadable (no contract / not cw2 / unit-test double) -> preserve the
legacy fail-open. Residual (orphaned/typo'd factory address -> no contract ->
fails open) is documented in the helper; near-theoretical on-chain since
contracts aren't deletable. Flip the None arm to block for a stricter posture.

The version probe runs only on the error path, so the normal flow pays no extra
gas.

Tests: 5 new unit tests on the decision fn (current/newer block, pre-1.5 + both
unreadable cases fail open); blacklist integration suite 10/0 unchanged (real
factory, happy path); full workspace 460/0.

Both blacklist guards returned Ok(None)/Ok(()) on ANY factory BlacklistCheck query error, to stay compatible with pre-1.5.0 factories. That silently disabled all blacklist enforcement if the stored factory pointer ever errored on the check (SEC-I03 F02). New dex_common::blacklist::blacklist_query_error_blocks() reads the factory's cw2 version from raw storage (contract_info) and decides: - factory reachable AND version >= 1.5.0 -> BlacklistCheck must work; an error is anomalous -> BLOCK (new ContractError::BlacklistGuardUnavailable, an observable signal instead of a silent pass) - factory reachable AND version < 1.5.0 -> documented pre-1.5.0 compat -> fail open - version unreadable (no contract / not cw2 / unit-test double) -> preserve the legacy fail-open. Residual (orphaned/typo'd factory address -> no contract -> fails open) is documented in the helper; near-theoretical on-chain since contracts aren't deletable. Flip the None arm to block for a stricter posture. The version probe runs only on the error path, so the normal flow pays no extra gas. Tests: 5 new unit tests on the decision fn (current/newer block, pre-1.5 + both unreadable cases fail open); blacklist integration suite 10/0 unchanged (real factory, happy path); full workspace 460/0.

Brouie commented

2026-06-30 19:04:24 +00:00

(Migrated from gitlab.com)

assigned to @Brouie

PlasticDigits commented

2026-06-30 19:07:30 +00:00

(Migrated from gitlab.com)

Security review — MR !982

Commit reviewed: 7cc0afb51d5b06efa8466ac62edec1168304d783
Scope: Version-gated fail-open for pair/router factory BlacklistCheck query errors (dex_common::blacklist::blacklist_query_error_blocks, pair/router blacklist_guard.rs, new BlacklistGuardUnavailable error variants, unit tests).

Method: Diff review; traced attacker-controlled inputs (swap sender/trader, token/pair addresses in guard probes) through probe_factory_blacklist / assert_router_swap_not_blacklisted into factory query_blacklist_check; verified version probe (query_wasm_raw → contract_info cw2 JSON) is only reachable on the query-error path and is not user-writable cross-contract.

Outcome: FINDINGS: 0 medium+

Inline threads: None (no medium+ findings).

Notes (non-finding)

This change tightens SEC-I03 F02 posture: reachable factory cw2 >= 1.5.0 + BlacklistCheck error now blocks (BlacklistGuardUnavailable) instead of silently disabling enforcement.
Documented residual fail-open when the stored factory address has no readable cw2 version (no contract / empty contract_info) remains intentional legacy compatibility; pair factory is set at factory-driven instantiation, not swap-time user input, so no plausible unprivileged bypass path was identified on this diff.
Pre-1.5.0 version-gated fail-open is documented compatibility, not a regression introduced here.

Security review: no medium+ findings on this diff.

## Security review — MR !982 **Commit reviewed:** `7cc0afb51d5b06efa8466ac62edec1168304d783` **Scope:** Version-gated fail-open for pair/router factory `BlacklistCheck` query errors (`dex_common::blacklist::blacklist_query_error_blocks`, pair/router `blacklist_guard.rs`, new `BlacklistGuardUnavailable` error variants, unit tests). **Method:** Diff review; traced attacker-controlled inputs (swap sender/trader, token/pair addresses in guard probes) through `probe_factory_blacklist` / `assert_router_swap_not_blacklisted` into factory `query_blacklist_check`; verified version probe (`query_wasm_raw` → `contract_info` cw2 JSON) is only reachable on the query-error path and is not user-writable cross-contract. **Outcome:** `FINDINGS: 0` medium+ **Inline threads:** None (no medium+ findings). ### Notes (non-finding) - This change **tightens** SEC-I03 F02 posture: reachable factory cw2 `>= 1.5.0` + `BlacklistCheck` error now **blocks** (`BlacklistGuardUnavailable`) instead of silently disabling enforcement. - Documented residual fail-open when the stored factory address has no readable cw2 version (no contract / empty `contract_info`) remains intentional legacy compatibility; pair `factory` is set at factory-driven instantiation, not swap-time user input, so no plausible unprivileged bypass path was identified on this diff. - Pre-1.5.0 version-gated fail-open is documented compatibility, not a regression introduced here. Security review: **no medium+ findings** on this diff.

Brouie commented

2026-06-30 19:08:32 +00:00

(Migrated from gitlab.com)

mentioned in issue #456