Security (EVM-01): oracle-service uses sequential RPC fallback, not multi-provider cross-check

Brouie commented

2026-04-21 08:12:45 +00:00

(Migrated from gitlab.com)

@PlasticDigits finding from security review on #5.

Severity

Medium. Single point of trust in the rate pipeline.

Description

oracle-service/src/evm_rpc.rs run_with_evm_rpc_url_fallback iterates BSC_RPC_URLS sequentially and returns the first successful result. It does not cross-check the rate against other providers before accepting it.

for (i, url) in urls.iter().enumerate() {
    match op(url.clone()).await {
        Ok(v) => return Ok(v),  // first success wins
        Err(e) => last_err = Some(e),
    }
}

Combined with bsc.rs read_exchange_rate_stored, which takes a single rpc_url, this means: whatever the first-responding RPC returns is what gets proposed on-chain.

Why this matters

BSC_RPC_URLS is user-configured. If one URL is compromised (or an attacker controls its DNS), and it appears first in the list, the attacker can forge exchangeRateStored and the service will dutifully broadcast that forged rate to Terra. On-chain caps (4h throttle, 2% daily, monotonic) bound the blast radius per tick, but over many ticks an attacker can steadily push the rate in a harmful direction within caps.

A single responsive-and-wrong RPC is enough — the other URLs are only consulted if the first one errors, not when it returns an anomalous value.

Recommendation

Read from at least 2 providers on every tick (e.g. first 2 in the list)
Require exact match (or tight tolerance) between them
On mismatch, escalate to a third, or abort and alert
Consider weighted median if using ≥3 providers

Review parent: #5
Pairs with EVM-02 (reorg), EVM-03 (Venus address allowlist), EVM-14 (chainId)

@PlasticDigits finding from security review on #5. ## Severity Medium. Single point of trust in the rate pipeline. ## Description `oracle-service/src/evm_rpc.rs` `run_with_evm_rpc_url_fallback` iterates `BSC_RPC_URLS` sequentially and returns the first successful result. It does not cross-check the rate against other providers before accepting it. ```rust for (i, url) in urls.iter().enumerate() { match op(url.clone()).await { Ok(v) => return Ok(v), // first success wins Err(e) => last_err = Some(e), } } ``` Combined with `bsc.rs` `read_exchange_rate_stored`, which takes a single `rpc_url`, this means: whatever the first-responding RPC returns is what gets proposed on-chain. ## Why this matters `BSC_RPC_URLS` is user-configured. If one URL is compromised (or an attacker controls its DNS), and it appears first in the list, the attacker can forge `exchangeRateStored` and the service will dutifully broadcast that forged rate to Terra. On-chain caps (4h throttle, 2% daily, monotonic) bound the blast radius per tick, but over many ticks an attacker can steadily push the rate in a harmful direction within caps. A single responsive-and-wrong RPC is enough — the other URLs are only consulted if the first one errors, not when it returns an anomalous value. ## Recommendation 1. Read from at least 2 providers on every tick (e.g. first 2 in the list) 2. Require exact match (or tight tolerance) between them 3. On mismatch, escalate to a third, or abort and alert 4. Consider weighted median if using ≥3 providers ## Related - Review parent: #5 - Pairs with EVM-02 (reorg), EVM-03 (Venus address allowlist), EVM-14 (chainId)

Brouie commented

2026-04-21 08:17:07 +00:00

(Migrated from gitlab.com)

mentioned in issue #5

PlasticDigits commented

2026-04-21 13:42:50 +00:00

(Migrated from gitlab.com)

mentioned in commit 8c9fafbe69

mentioned in commit 8c9fafbe696f8bd99a93f3db680003a1e8d01871

PlasticDigits commented

2026-04-21 13:43:06 +00:00

(Migrated from gitlab.com)

Fix implemented (merged to `main`)

Behavior

BSC_RPC_URLS must now list at least two comma-separated endpoints (validated in config::Config::from_env).
Each tick calls run_with_evm_rpc_rate_consensus: the first two URLs are queried in parallel via tokio::join!.
A rate is accepted only when two successful responses agree within 0.01% relative: |a-b| / max(a,b) ≤ 1/10 000 (integer check: (hi-lo) * 10_000 ≤ hi).
If the first two disagree beyond that tolerance and a third URL is configured, a third read is used as a tie-break (must match one of the first two within tolerance).
If the first two both error and only the third succeeds, the tick fails: two agreeing successes are still required.

Code

oracle-service/src/evm_rpc.rs: consensus + unit tests for the tolerance helper.
oracle-service/src/main.rs: uses the new consensus path instead of first-success fallback.
scripts/deploy_local.py: example env shows two Binance dataseed URLs.

Commit on main: merge of fix/issue-7-evm-rpc-consensus (pushed to origin/main).

Residual risk / follow-ups: still no reorg handling (EVM-02) or weighted median for ≥3 providers; can extend later if desired.

## Fix implemented (merged to `main`) **Behavior** - `BSC_RPC_URLS` must now list **at least two** comma-separated endpoints (validated in `config::Config::from_env`). - Each tick calls `run_with_evm_rpc_rate_consensus`: the **first two** URLs are queried **in parallel** via `tokio::join!`. - A rate is accepted only when **two successful** responses agree within **0.01% relative**: `|a-b| / max(a,b) ≤ 1/10 000` (integer check: `(hi-lo) * 10_000 ≤ hi`). - If the first two **disagree** beyond that tolerance and a **third** URL is configured, a third read is used as a **tie-break** (must match one of the first two within tolerance). - If the first two **both error** and only the third succeeds, the tick **fails**: two agreeing successes are still required. **Code** - `oracle-service/src/evm_rpc.rs`: consensus + unit tests for the tolerance helper. - `oracle-service/src/main.rs`: uses the new consensus path instead of first-success fallback. - `scripts/deploy_local.py`: example env shows two Binance dataseed URLs. **Commit** on `main`: merge of `fix/issue-7-evm-rpc-consensus` (pushed to `origin/main`). Residual risk / follow-ups: still no reorg handling (EVM-02) or weighted median for ≥3 providers; can extend later if desired.

Brouie commented

2026-04-22 00:23:31 +00:00

(Migrated from gitlab.com)

@PlasticDigits verified fix on 8c9fafb.

Code review:

parse_comma_separated_rpc_urls + startup check: ≥2 URLs now required, startup errors if fewer
run_with_evm_rpc_rate_consensus: queries first two endpoints in parallel, accepts rate only when they agree within 0.01% relative (|a−b|/max ≤ 1e-4)
Third URL consulted as tiebreaker on disagreement
Symmetric agreement check (rates_agree_within_point_zero_one_percent)
4 new unit tests: identical_rates_agree, zero_pair, within_point_zero_one_percent, symmetric

Tolerance choice of 0.01% is a good balance — tight enough to catch malicious forgery, loose enough for cross-provider fixed-point rounding noise.

Workspace test count confirms: 40/40 pass (includes new oracle-service unit tests).

@PlasticDigits verified fix on 8c9fafb. Code review: - `parse_comma_separated_rpc_urls` + startup check: ≥2 URLs now required, startup errors if fewer - `run_with_evm_rpc_rate_consensus`: queries first two endpoints in parallel, accepts rate only when they agree within 0.01% relative (|a−b|/max ≤ 1e-4) - Third URL consulted as tiebreaker on disagreement - Symmetric agreement check (`rates_agree_within_point_zero_one_percent`) - 4 new unit tests: `identical_rates_agree`, `zero_pair`, `within_point_zero_one_percent`, `symmetric` Tolerance choice of 0.01% is a good balance — tight enough to catch malicious forgery, loose enough for cross-provider fixed-point rounding noise. Workspace test count confirms: 40/40 pass (includes new oracle-service unit tests).

Brouie (Migrated from gitlab.com) closed this issue

2026-04-22 00:23:46 +00:00

PlasticDigits commented

2026-07-30 00:20:31 +00:00

(Migrated from gitlab.com)

mentioned in issue #19

Rows
Columns

Security (EVM-01): oracle-service uses sequential RPC fallback, not multi-provider cross-check #7

Severity

Description

Why this matters

Recommendation

Related

Fix implemented (merged to main)

Fix implemented (merged to `main`)