[pallets/weaver] Off-by-one <= in validate_attestation_signature rejects every honest OCW attestation

Bug Bounty
1

Component: ghost-node @ 0162da4604d9931c16d12a0cec40dd85951368c5 — pallets/weaver/src/lib.rs:642 (validate_attestation_signature).

Description

validate_unsigned for extend_warp / weave_weft calls validate_attestation_signature, which contains:

// pallets/weaver/src/lib.rs:642-646
if T::BlockNumberProvider::current_block_number() <= attestation_block {
    return Err(InvalidTransaction::BadProof);
}

The off-chain worker builds attestations stamped with the block it is processing (AttestationContext::default().with_current_block(current_block), lib.rs:776-780, where current_block is the now passed from offchain_worker(now)). When the transaction enters the pool, the pool validates it against the current best block, which is that same now. So now <= now is always true and the pool discards the transaction with BadProof.

extend_warp and weave_weft are the only two ways oracle data (external block numbers, Merkle roots, block hashes) gets on chain. Through the normal unsigned-transaction path, neither can ever enter the pool.

The existing test suite masks this: every validate_unsigned test advances the block first (set_block_number(N+1)) before validating, so the <= branch never triggers. No test validates an attestation at the block it was created, which is the production scenario.

Impact

Total loss of bridge liveness. Without extend_warp, no external block-number consensus ever starts; without weave_weft, no external Merkle root or block hash is ever committed to LoomStates. Every bridge function (weaving plus claims via pull_thread) is dead. The only workaround is a block author manually inserting the transactions past its own pool.

Reproduction

Local test poc_w1_ocw_attestation_rejected_at_submission_block in pallets/weaver/src/poc.rs:

  1. Set the mock chain block to 5.
  2. Build a BlockAttestation with with_current_block(5) (exactly what the OCW does at block 5).
  3. Sign it with an authority key, wrap it in Call::extend_warp.
  4. Call GhostWeaver::validate_unsigned(TransactionSource::External, &call) → Err(InvalidTransaction::BadProof).
  5. Control: set block to 6, repeat → Ok.

A second test proves the same rejection for weave_weft. Ran cargo test -p ghost-weaver --lib poc_w1: passed.

Severity: High

Reason: the pallet’s core bridge function is 100% non-functional through the normal path in production (a permanent denial-of-service against all bridge features). Not Critical because it enables no theft or unauthorized minting; it blocks everything instead.

Recommendation

Change <= to < at lib.rs:642 so only attestations for future blocks are rejected. Signature re-verification at dispatch keeps the security properties unchanged.

Verification status

VERIFIED. Mechanism confirmed line-by-line in source; PoC executed locally and passed.

2

Thank you for your report!

It would be good to reproduce the inability to register anything external on the development network before proceeding. As an external gatekeeper address, you can use the same contract on the Sepolia network that was used for testing: 0x7fec11e652f4d54e8cf11c4b52c360380332f2bd.

On this point: the OCW starts at the end of each block T. So how could the node push a value for that block when the work only begins once the block has ended? Could you please clarify your idea?

3

Good questions. On the timing: the OCW’s now is the block that just finished importing (block T). It builds the attestation stamped with T and hands it to the local transaction pool via SubmitTransaction while the node’s best block is still T. The pool runs validate_unsigned immediately at submission, and T::BlockNumberProvider is System in the runtimes, so current_block_number() is T, the same T the attestation is stamped with. T <= T yields BadProof, and the transaction is rejected before it ever propagates. The node never needs to push the value “during” block T; the binding constraint is pool-side validation at submission time against best block T.

So in the normal flow the rejection is deterministic, not a race: OCW submits at T, pool validates at T. (If an OCW were delayed past T+1 the check would pass, but relying on slow OCWs is not a workaround.)

On reproduction: my PoC ran against the pallet’s mock runtime (validate_unsigned at the submission block returns BadProof, and Ok once the block advances). I can reproduce the pool-level rejection end to end on a local devnet. Is there a standard devnet harness you’d like me to use, or should I stand one up from the repo’s dev setup?

On the Sepolia contract: that address is the attested data source, which is orthogonal here. The rejection happens in validate_unsigned before any attestation content is evaluated, so it bites regardless of which contract is being attested.

4

This issue is really interesting. During the initial draft, I personally ran the development chain locally with Alice, Bob, Carol, and Dave. For testing purposes, we added the actual Gatekeeper contract address to watch on the Sepolia network.

  1. The exact same check persists there - take a look.
  2. The genesis was filled from the actual RPC for the Sepolia testnet - see here.

That’s why your reported issue is unclear: the development network successfully reached the latest Merkle root hash and registered it as LoomState. So I’m questioning it - why do you think it will break? You can run development chain on your own.

5

Thanks for running this on the devnet, that is useful data.

You are right to question the “deterministic” framing in my report, and I want to correct it. My PoC submitted an attestation stamped T for validation at T (synchronous, mock runtime), which is the only timing that the <= check rejects. A production OCW does external RPC I/O (fetching Sepolia block data, reaching consensus across endpoints) before it calls submit_unsigned_transaction, so by the time the transaction reaches the pool the best block is usually T+1 or later, and T+1 <= T is false. That explains why your devnet reaches LoomState fine: the check passes by timing, not by correctness.

So the narrowed claim is this: the <= check makes bridge liveness timing-dependent instead of guaranteed. Any attestation submitted in the same block it is stamped with is rejected with BadProof, and there is no retry with a fresh stamp (the OCW only logs the submission error). On your devnet the RPC latency hides this; with a fast local RPC endpoint or cached responses, same-block submissions would start failing.

The fix recommendation stands (<= to <, rejecting only genuinely future-dated attestations), but the severity framing in my report was overstated. I will leave the severity call to you given the devnet evidence.

Happy to run the devnet myself to confirm the timing behavior if that would help.

6

But let me slightly expand on your report.

What if there are network delays between nodes? Would that create any issues?

By the way, changing <= to < won’t help if the delay between nodes is more than one block - in that case, it will still be rejected until the nodes sync.