Skip to content

Security review: fix critical bridge bugs, add withdrawal path, remove ethers/ring - #10

Merged
ishaanman7898 merged 1 commit into
mainfrom
security-review-fixes
Nov 14, 2026
Merged

ishaanman7898 merged 1 commit into
mainfrom
security-review-fixes

Conversation

@ishaanman7898

@ishaanman7898 ishaanman7898 commented Nov 14, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Security review of the whole repo, with fixes. Full findings, severity and status: docs/SECURITY_REVIEW.md.

Not fixed, and the reason this is still not safe for real funds: the Groth16 circuit proves only a commitment relation. It does not verify source-chain consensus or receipt inclusion, so whoever holds the proving key can attest any message (finding C1). That needs a light-client circuit / zkVM program and a multi-party setup. The contracts now hash a canonical message designed to be that circuit's public output.

Critical fixes

  • Solana hub: mint/swap amount, recipient and mint were not bound to the proof, so any staked relayer could mint unlimited tokens or drain the pool. All fields are now recomputed on-chain into a canonical payload hash.
  • Cosmos: the "verifier" accepted any 256 non-zero bytes, so anyone could drain the contract. Inbound now fails closed until a real verifier exists.
  • EVM: verified messages could make arbitrary calls from the custody contract (Poly Network pattern). Custody now moves only via releaseTokens, capped by totalLocked.
  • Staking: any mint could be staked. The ILINK mint is now pinned.
  • Legacy program: deleted contracts/solana/src, whose verifier was forgeable.

Also fixed (high/medium)

  • Replay: proofs were not bound to chain or gateway, nonces collided across sources, and the hub's global sequence could stall the bridge forever. Replay protection is now per payload hash.
  • EVM proofs: the EVM verifier used a different public-input formula than the prover, so relayer proofs could never verify.
  • Hub compute: hub field math exceeded Solana's compute limit; replaced with 4-limb Montgomery arithmetic, checked against arkworks.
  • Relayer key: the relayer ran a new random trusted setup on every restart. The proving key is now persistent and shared with export-vk.
  • Reorgs: finality was a block-depth count with no canonical re-check. It now waits for the finalized tag and re-checks the receipt.
  • Admin powers: VK and remote-gateway changes are timelocked, guardian transfer is two-step, and emergency withdrawal works only while paused.
  • Tokens and addresses: USDT and fee-on-transfer tokens are handled, recipients are bytes32, and amounts are decimal-scaled into u64.
  • Way back from Solana: added burn_for_withdrawal, relayer hub → EVM delivery via EIP-1559, and claim_swap_as_wrapped. NFT locking is disabled until a destination supports NFTs.
  • Execution: the relayer now executes attested transfers; previously nothing was ever minted.
  • Webhooks: routes now need a bearer API_TOKEN.
  • Dependencies: ethers-rs is removed from the runtime and the stale interlink-core relayer deleted, so ring 0.16 (RUSTSEC-2025-0009/0010) is gone.
  • SDK: sends real gateway transactions with the user's signer and tracks by payload hash. ChainId.Solana is now 2 (was 900, which mismatched the contracts).
  • Build: two Foundry scripts didn't compile; they do now.
  • Docs: removed claims of an audit and devnet results that weren't true.

Breaking changes

New account layouts and instructions on the hub, a new EVM ABI/event, and new Cosmos messages. Everything needs redeploying, and the VK must be regenerated with cargo run --bin export-vk.

Still open

  • C1: the circuit.
  • M11: pool economics; reserve_a is virtual.
  • Cosmos inbound: no BN254 verifier on CosmWasm 1.5.
  • Dependabot alerts on main beyond the ethers chain should be re-checked after merge.

Test plan

  • CI (Unified Protocol Verification): all 7 stages pass. Stage 5 (Cosmos wasm) also fails on main: newer rustc no longer passes --allow-undefined to the wasm linker, which cosmwasm-std 1.5 relies on. Fixed with a crate-local cargo config; Stages 6–7 now run on CI too
  • cargo test --workspace: relayer lib 445, integration 19, security 32, checklist 9, circuits 10, interlink-core 5; clippy -D warnings and fmt --check clean
  • Hub host tests (11), including a real arkworks Groth16 proof through the on-chain verifier
  • Cosmos tests (14), including a regression for the forged-proof drain
  • SDK: 31 Jest tests, tsc clean, coverage above thresholds
  • EVM: contract, tests and scripts compile with solc 0.8.28 (via_ir); the compiled gateway was run in revm with real proofs (release, tamper, replay, custody guards) and with the relayer's own prover and calldata
  • Shared canonical-hash vector agrees across Solidity, the hub, the relayer and the SDK; PDA/ATA derivation matches the Solana CLI; EIP-1559 signing matches ethers-core
  • forge test (Foundry not available in the review environment)
  • anchor test and an SBF build with on-chain CU measurement
  • Relayer against live chains (devnet + an EVM testnet)

🤖 Generated with Claude Code

@ishaanman7898
ishaanman7898 merged commit 17c2ba1 into main Nov 14, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant