Security review: canceler RPC hardening findings (mirrors ust1-window sweep)

Brouie commented

2026-04-22 04:35:25 +00:00

(Migrated from gitlab.com)

@PlasticDigits parallel security check on canceler following the ust1-window 40-category sweep. Per your guidance, filing as single summary issue rather than parent + sub-issues since findings are all RPC-related and medium-severity.

Current launch-blocking status: none — you confirmed cl8y xchain activation is fine at this severity level given current asset / rate-limit scope.

Findings

EVM-01: Sequential RPC fallback, not multi-provider cross-check — Medium

canceler/src/evm_client.rs uses multichain_rs::run_with_evm_rpc_url_fallback which tries primary, falls through to fallbacks on error. First responsive RPC wins. No consensus check across providers.

Recommendation: mirror the ust1-window fix — query first 2 URLs in parallel, require 0.01% tolerance agreement, use 3rd URL as tiebreaker. Pattern already proven in oracle-service 8c9fafb.

EVM-02: Polls up to `latest` with no confirmation depth — Medium

canceler/src/watcher.rs:781 polls current_block (latest) as the poll end, without subtracting confirmation depth. Reorgs could cause the canceler to act on approvals that get rolled back.

Canceler has a reactive 'Chain reset detected — resetting to lookback window' handler (line 772) that clears caches after-the-fact, but no proactive confirmation-depth subtraction.

Recommendation: subtract configurable EVM_CONFIRMATION_BLOCKS (default ~15 on BSC, ~20 on opBNB) from the poll end. Operator already has finality_blocks field — canceler should follow same pattern.

EVM-03: No canonical bridge address allowlist — Low-Med

EVM_BRIDGE_ADDRESS accepted from env as any hex string. Wrong bridge address (typo, wrong-environment copy) would be silently used.

Lower severity than ust1-window EVM-03 because bridge-address changes are governance events (hard to slip past) and mismatch would surface quickly via failed cancels. Still worth a canonical-address startup check for production environments.

EVM-08: HTTPS validation warns but doesn't block — Low-Med

canceler/src/config.rs::validate_rpc_url accepts http:// with a tracing::warn!("...use https:// in production") rather than erroring. Operator also uses this via multichain_rs::validate_rpc_url.

Easy to miss a warn log; a single misconfigured http:// URL would be MITM-able for the canceler's EVM reads.

Recommendation: add DEV_ALLOW_HTTP=1 env flag (default off) gating loopback-only http allowance. Matches ust1-window fix pattern (0f5a3e4 era).

EVM-10: `/health` returns unconditional `OK`, no staleness alert — Medium

canceler/src/server.rs:

/health returns HealthResponse { status: "healthy", ... } regardless of actual staleness
/readyz checks 'has polled ≥1 block ever' — flips to OK once and stays OK forever
/metrics exposes last_evm_block, last_terra_height (good) but no built-in threshold

External Prometheus with alerting rules could monitor this, but the service-local health endpoints don't detect 'stuck canceler' states.

Recommendation: track last_successful_cancel_ts + last_successful_poll_ts; readiness/health return non-OK if silence exceeds configurable threshold (default ~4-8h depending on expected cancel frequency). Pattern: ust1-window liveness.rs in 31a8b3a.

EVM-14: No `eth_chainId` verification — Low-Med

canceler reads EVM_CHAIN_ID from env (u64) but never calls eth_chainId on the configured RPC to verify. Wrong-chain RPC (opBNB pointed at BSC account, mainnet pointed at testnet, etc) would be silently used.

Recommendation: startup verify_all_bsc_rpc_urls equivalent — call eth_chainId on every configured URL, error if mismatch against EVM_CHAIN_ID. Pattern: ust1-window bsc.rs in 673ac74.

EVM-18: ✅ PASS (no finding)

Config, EvmConfig, TerraConfig, SolanaConfig, DatabaseConfig all have manual fmt::Debug impls that redact secrets (evm_private_key, terra_mnemonic, private_key, database URL). Explicit note: "NOTE: Debug is manually implemented to redact sensitive fields — Do NOT re-add #[derive(Debug)]."

Already covered by canceler security review C1. Clean.

Cross-references

ust1-window sweep: PlasticDigits/ust1-window#5 (all 9 findings closed as of today)
Fix patterns: commits b3b8937, 8c9fafb, 0f5a3e4, c42ca85, 31a8b3a, 673ac74, 7cd0c45, 7bcc285 in ust1-window

Operator counterpart

Operator has a very similar profile — same findings mostly apply except EVM-02 (operator already has finality_blocks, defaults to 1 which is weak but addresses the mechanic). Filing as a separate issue for component-level tracking.

@PlasticDigits parallel security check on canceler following the ust1-window 40-category sweep. Per your guidance, filing as single summary issue rather than parent + sub-issues since findings are all RPC-related and medium-severity. Current launch-blocking status: **none** — you confirmed cl8y xchain activation is fine at this severity level given current asset / rate-limit scope. ## Findings ### EVM-01: Sequential RPC fallback, not multi-provider cross-check — Medium `canceler/src/evm_client.rs` uses `multichain_rs::run_with_evm_rpc_url_fallback` which tries primary, falls through to fallbacks on error. First responsive RPC wins. No consensus check across providers. **Recommendation:** mirror the ust1-window fix — query first 2 URLs in parallel, require 0.01% tolerance agreement, use 3rd URL as tiebreaker. Pattern already proven in `oracle-service` 8c9fafb. ### EVM-02: Polls up to `latest` with no confirmation depth — Medium `canceler/src/watcher.rs:781` polls `current_block` (latest) as the poll end, without subtracting confirmation depth. Reorgs could cause the canceler to act on approvals that get rolled back. Canceler has a **reactive** 'Chain reset detected — resetting to lookback window' handler (line 772) that clears caches after-the-fact, but no proactive confirmation-depth subtraction. **Recommendation:** subtract configurable `EVM_CONFIRMATION_BLOCKS` (default ~15 on BSC, ~20 on opBNB) from the poll end. Operator already has `finality_blocks` field — canceler should follow same pattern. ### EVM-03: No canonical bridge address allowlist — Low-Med `EVM_BRIDGE_ADDRESS` accepted from env as any hex string. Wrong bridge address (typo, wrong-environment copy) would be silently used. Lower severity than ust1-window EVM-03 because bridge-address changes are governance events (hard to slip past) and mismatch would surface quickly via failed cancels. Still worth a canonical-address startup check for production environments. ### EVM-08: HTTPS validation warns but doesn't block — Low-Med `canceler/src/config.rs::validate_rpc_url` accepts `http://` with a `tracing::warn!("...use https:// in production")` rather than erroring. Operator also uses this via `multichain_rs::validate_rpc_url`. Easy to miss a warn log; a single misconfigured http:// URL would be MITM-able for the canceler's EVM reads. **Recommendation:** add `DEV_ALLOW_HTTP=1` env flag (default off) gating loopback-only http allowance. Matches ust1-window fix pattern (0f5a3e4 era). ### EVM-10: `/health` returns unconditional `OK`, no staleness alert — Medium `canceler/src/server.rs`: - `/health` returns `HealthResponse { status: "healthy", ... }` regardless of actual staleness - `/readyz` checks 'has polled ≥1 block ever' — flips to OK once and stays OK forever - `/metrics` exposes `last_evm_block`, `last_terra_height` (good) but no built-in threshold External Prometheus with alerting rules could monitor this, but the service-local health endpoints don't detect 'stuck canceler' states. **Recommendation:** track `last_successful_cancel_ts` + `last_successful_poll_ts`; readiness/health return non-OK if silence exceeds configurable threshold (default ~4-8h depending on expected cancel frequency). Pattern: ust1-window `liveness.rs` in 31a8b3a. ### EVM-14: No `eth_chainId` verification — Low-Med `canceler` reads `EVM_CHAIN_ID` from env (u64) but never calls `eth_chainId` on the configured RPC to verify. Wrong-chain RPC (opBNB pointed at BSC account, mainnet pointed at testnet, etc) would be silently used. **Recommendation:** startup `verify_all_bsc_rpc_urls` equivalent — call `eth_chainId` on every configured URL, error if mismatch against `EVM_CHAIN_ID`. Pattern: ust1-window `bsc.rs` in 673ac74. ### EVM-18: ✅ PASS (no finding) `Config`, `EvmConfig`, `TerraConfig`, `SolanaConfig`, `DatabaseConfig` all have **manual `fmt::Debug` impls** that redact secrets (`evm_private_key`, `terra_mnemonic`, `private_key`, database URL). Explicit note: "NOTE: Debug is manually implemented to redact sensitive fields — Do NOT re-add `#[derive(Debug)]`." Already covered by canceler security review C1. Clean. ## Cross-references - ust1-window sweep: `PlasticDigits/ust1-window#5` (all 9 findings closed as of today) - Fix patterns: commits `b3b8937`, `8c9fafb`, `0f5a3e4`, `c42ca85`, `31a8b3a`, `673ac74`, `7cd0c45`, `7bcc285` in ust1-window ## Operator counterpart Operator has a very similar profile — same findings mostly apply except EVM-02 (operator already has `finality_blocks`, defaults to 1 which is weak but addresses the mechanic). Filing as a separate issue for component-level tracking.

Brouie commented

2026-04-22 04:36:13 +00:00

(Migrated from gitlab.com)

mentioned in issue #115

PlasticDigits commented

2026-04-22 06:29:00 +00:00

(Migrated from gitlab.com)

mentioned in commit 8efb987c25

mentioned in commit 8efb987c2597a0eb44fad9d48a3bd92dc3dfce16

PlasticDigits commented

2026-04-22 06:29:44 +00:00

(Migrated from gitlab.com)

Implemented on main (commit 8efb987, branch fix/gl-114-115-rpc-hardening merged).

EVM-01 / quorum: multichain-rs now resolves eth_blockNumber across up to 3 endpoints with 0.01% tolerance and a configurable quorum (EVM_RPC_AGREEMENT_QUORUM, default min(2, url_count)). With multiple URLs, EVM_RPC_AGREEMENT_QUORUM=1 is rejected unless EVM_RPC_SINGLE_ENDPOINT_READS=1 pins reads to the first URL (covers single-reliable-RPC chains like opBNB while blocking silent failover to a different truth). Canceler uses the same policy for enumeration, events, getThisChainId, and can_cancel; cancel tx submission tries the consensus endpoint first then fallbacks.

EVM-02: EVM_CONFIRMATION_BLOCKS (chain-aware default: BSC 15, opBNB 20, else 12) subtracted from the EVM head for approval polling.

EVM-03: Optional EVM_CANONICAL_BRIDGE_ADDRESSES allowlist.

EVM-08: Shared validate_rpc_url — remote http:// blocked unless DEV_ALLOW_HTTP=1 or loopback host.

EVM-10: /health JSON + status 503 when idle exceeds CANCELER_HEALTH_MAX_IDLE_SECS (default 8h) after CANCELER_HEALTH_STARTUP_GRACE_SECS (default 5m); /readyz considers staleness. Activity timestamp updated each successful poll_approvals cycle.

EVM-14: Startup verify_evm_jsonrpc_chain_ids on all primary + multi-EVM RPC URLs.

@Brouie please verify in your environment (especially multi-RPC quorum vs single-URL + EVM_RPC_SINGLE_ENDPOINT_READS).

Implemented on `main` (commit 8efb987, branch `fix/gl-114-115-rpc-hardening` merged). **EVM-01 / quorum:** `multichain-rs` now resolves `eth_blockNumber` across up to 3 endpoints with 0.01% tolerance and a configurable quorum (`EVM_RPC_AGREEMENT_QUORUM`, default `min(2, url_count)`). With multiple URLs, `EVM_RPC_AGREEMENT_QUORUM=1` is rejected unless `EVM_RPC_SINGLE_ENDPOINT_READS=1` pins reads to the first URL (covers single-reliable-RPC chains like opBNB while blocking silent failover to a different truth). Canceler uses the same policy for enumeration, events, `getThisChainId`, and `can_cancel`; cancel tx submission tries the consensus endpoint first then fallbacks. **EVM-02:** `EVM_CONFIRMATION_BLOCKS` (chain-aware default: BSC 15, opBNB 20, else 12) subtracted from the EVM head for approval polling. **EVM-03:** Optional `EVM_CANONICAL_BRIDGE_ADDRESSES` allowlist. **EVM-08:** Shared `validate_rpc_url` — remote `http://` blocked unless `DEV_ALLOW_HTTP=1` or loopback host. **EVM-10:** `/health` JSON + status 503 when idle exceeds `CANCELER_HEALTH_MAX_IDLE_SECS` (default 8h) after `CANCELER_HEALTH_STARTUP_GRACE_SECS` (default 5m); `/readyz` considers staleness. Activity timestamp updated each successful `poll_approvals` cycle. **EVM-14:** Startup `verify_evm_jsonrpc_chain_ids` on all primary + multi-EVM RPC URLs. @Brouie please verify in your environment (especially multi-RPC quorum vs single-URL + `EVM_RPC_SINGLE_ENDPOINT_READS`).

Brouie commented

2026-04-23 01:16:03 +00:00

(Migrated from gitlab.com)

@PlasticDigits verified on 8efb987.

Build + test baseline on branch:

multichain-rs: 166 tests passing
canceler: 34 tests passing (29 unit + 5 server, 11 ignored/integration)
0 failures

Findings:

EVM-01 RPC quorum: EvmRpcReadPolicy + evm_consensus_latest_block parallel quorum with 0.01% tolerance. Default min(2, url_count), explicit error on quorum=1 with multiple URLs. Unit tests in rpc_fallback.rs cover the policy.
EVM-02 confirmation depth: EVM_CONFIRMATION_BLOCKS first-class, chain-aware defaults (BSC=15, opBNB=20, default=12), saturating_sub in watcher.rs at both event-scan sites.
EVM-03 bridge allowlist: check_bridge_allowlist() at startup, case-insensitive, opt-in via EVM_CANONICAL_BRIDGE_ADDRESSES. Validates primary + all multi-EVM bridge addresses.
EVM-08 HTTPS gate: validate_rpc_url (multichain-rs) rejects remote http without DEV_ALLOW_HTTP=1, allows loopback, rejects file:// and ftp://. Full unit coverage.
EVM-14 chainId verify: verify_evm_jsonrpc_chain_ids at startup, fails fast with index + expected/actual.

Minor cross-service note (not blocker): canceler defaults opBNB confirmation=20, operator defaults opBNB finality=12. Both safe, just asymmetric — flagging for your awareness.

Closing as verified.

@PlasticDigits verified on `8efb987`. Build + test baseline on branch: - multichain-rs: 166 tests passing - canceler: 34 tests passing (29 unit + 5 server, 11 ignored/integration) - 0 failures Findings: - **EVM-01 RPC quorum**: `EvmRpcReadPolicy` + `evm_consensus_latest_block` parallel quorum with 0.01% tolerance. Default `min(2, url_count)`, explicit error on quorum=1 with multiple URLs. Unit tests in rpc_fallback.rs cover the policy. - **EVM-02 confirmation depth**: `EVM_CONFIRMATION_BLOCKS` first-class, chain-aware defaults (BSC=15, opBNB=20, default=12), `saturating_sub` in watcher.rs at both event-scan sites. - **EVM-03 bridge allowlist**: `check_bridge_allowlist()` at startup, case-insensitive, opt-in via `EVM_CANONICAL_BRIDGE_ADDRESSES`. Validates primary + all multi-EVM bridge addresses. - **EVM-08 HTTPS gate**: `validate_rpc_url` (multichain-rs) rejects remote http without `DEV_ALLOW_HTTP=1`, allows loopback, rejects file:// and ftp://. Full unit coverage. - **EVM-14 chainId verify**: `verify_evm_jsonrpc_chain_ids` at startup, fails fast with index + expected/actual. Minor cross-service note (not blocker): canceler defaults opBNB confirmation=20, operator defaults opBNB finality=12. Both safe, just asymmetric — flagging for your awareness. Closing as verified.

Brouie (Migrated from gitlab.com) closed this issue

2026-04-23 01:17:07 +00:00