Skip to content

twap-monitor: no local cross-check that a ConditionalOrderCreated log or poll response ever happened on-chain #673

Description

@lgahdl

Summary

Nothing downstream of a chain-log delivery in modules/twap-monitor/src/keeper.rs independently verifies that the RPC actually relayed something real. The engine only filters incoming logs by address + topic-0 before they reach the module; nothing checks log inclusion against the chain itself. A malicious/compromised RPC provider can hand the module a fully fabricated ConditionalOrderCreated log for an owner who never registered anything, and the module accepts it unconditionally.

Affected code

  • modules/twap-monitor/src/keeper.rsdecode_conditional_order_created(), persist_watch(), TwapSource::poll() / poll_one(), decode_return()
  • crates/composable-cow/src/run.rssubmit_ready()

Finding

decode_conditional_order_created() only checks the log's topic-0 (already guaranteed by the engine's address+topic-0 filter) — there is no check that the log corresponds to a real chain inclusion. The same is true on the poll side: poll_one()/decode_return() blindly ABI-decode whatever the eth_call response contains. Since a compromised RPC controls both the log delivery and the eth_call response, it can drive the entire pipeline — index a watch, report it as poll-ready, and trigger a real submission attempt — purely from fabricated data, with nothing in the module ever cross-checking the RPC's honesty.

Confirmed with a test (fabricated_create_log_drives_a_real_submit_attempt_with_no_local_cross_check, run against a local clone, 26/26 tests pass including the new one, no regressions):

  1. Feed on_chain_logs a ConditionalOrderCreated log for an invented (owner, ConditionalOrderParams) pair — the watch is persisted unconditionally.
  2. Feed the corresponding eth_call a fabricated "ready" response (an arbitrary GPv2OrderData + a signature that verifies nothing).
  3. Dispatch on_block — the keeper submits the fabricated intent to the venue. venue.submit_count() == 1.

Why this likely isn't catastrophic in practice

The submission is only an attempt: it should still go through the real CoW orderbook's own independent verification, which validates the ERC-1271 signature on-chain via its own infrastructure — not the module's (possibly compromised) RPC. So the actual financial risk is probably contained outside shepherd. But that's a assumption about external infrastructure this repo doesn't control, and it isn't documented anywhere as a deliberate design decision — right now this reads like an oversight rather than an accepted trust boundary.

Suggested fix

Full verification (light-client-style inclusion proofs) is out of proportion for a keeper module. Two more proportionate options:

  1. Document the trust boundary explicitly in the code: the module trusts its configured RPC completely for both log delivery and poll responses; the actual safety net against a compromised RPC is the venue's own independent on-chain signature verification, not anything in this module.
  2. Add an operational signal: track the venue's Denied/refusal rate and alert on a spike. A compromised RPC feeding fabricated data would likely produce a burst of rejected submissions, which is a practical (if indirect) way to notice the scenario even without cryptographic verification.

Found via

Internal red-team exercise (malicious/compromised RPC provider persona, COW-1215 in the original epic — deferred in 2026-07-28 pending an expected absorption of twap-monitor's TWAP-specific logic into a generic composable-cow poller; re-checked in 2026-08-17, that absorption still hasn't happened, so the test was run against current code).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions