Skip to content

fix(backend)!: derive OnRamper signed wallet addresses from caller principal#13167

Draft
AntonioVentilii wants to merge 2 commits into
feat/onramper-address-encodersfrom
feat/onramper-derive-server-side
Draft

fix(backend)!: derive OnRamper signed wallet addresses from caller principal#13167
AntonioVentilii wants to merge 2 commits into
feat/onramper-address-encodersfrom
feat/onramper-derive-server-side

Conversation

@AntonioVentilii

Copy link
Copy Markdown
Collaborator

Motivation

sign_onramper_widget_url was an open signing oracle: it HMAC-signed the wallets / network_wallets / wallet_address_tags parameters exactly as supplied by any authenticated caller, without checking the addresses belonged to msg_caller(). An attacker could get the backend to sign their own address and phish a victim into funding it via a valid, OISY-branded OnRamper URL — the HMAC is OnRamper's sole URL-integrity check, so a valid signature reads as OISY authorization.

This closes the oracle by deriving the caller's own receiving addresses server-side and signing only those. A caller can now only ever obtain a signature bound to their own wallet.

Spec: docs/ai/spec-driven-development/specs/2026-06-22-fix-onramper-signing-oracle.md (#13164). Stacked on #13166 (the address encoders).

Changes

  • Endpoint is now argument-less and async. sign_onramper_widget_url takes no request; it derives the caller's BTC/ETH/ICP/SOL receiving addresses from msg_caller() and signs them as networkWallets. The caller_is_not_anonymous guard and per-caller rate limiter are unchanged.
  • Server-side derivation via management-canister public-key reads (signer/service.rs): ETH (secp256k1 schema 1 + keccak/EIP-55) and SOL (Ed25519 schnorr + base58) reuse the same canister_id = cfs_canister_id pattern as the existing BTC derivation; ICP is local principal2account. These are pubkey reads — no chain-fusion-signer call, no allow_signing/allowance flow, no signing fee. A chain is omitted if its derivation fails (ICP always succeeds), and AddressDerivationFailed is returned only if nothing derives.
  • Breaking Candid change: removed wallets / network_wallets / wallet_address_tags from the request (type removed); added the AddressDerivationFailed error variant; backend.did + src/declarations/backend/ regenerated via npm run generate.
  • Frontend no longer derives or sends wallet addresses: buildOnramperLink and the canister/api layer call the endpoint with the identity only; OnramperWidget.svelte drops the address-store wiring; the now-dead mapOnramperNetworkWallets util and Onramper*Wallet* types are removed.
  • docs/ai/PRODUCT.md gains a Buy (OnRamper) section with the explicit negative guarantee.

Tests

  • Backend unit (encoders, in feat(backend): add ETH/SOL public-key-to-address encoders #13166) + signer::service cover the pubkey→address encoding against known vectors.
  • Integration (tests/it/onramper.rs): anonymous rejection, SecretNotConfigured, rate limit, deterministic signing of the caller's own derived addresses, and a positive anti-oracle proof — two distinct callers get different signed content (a caller can only sign their own addresses).
  • Frontend: onramper.utils.spec.ts rewritten for the argument-less call; affected buy specs pass.
  • Gates: cargo fmt / lint.rust.sh / lint.did.sh, npm run check (clean except a pre-existing unrelated sol-instructions-system.utils.ts error not in this diff), scoped ESLint, and the affected vitest specs all pass. The heavy pocketIC integration suite runs in CI.

Parity note: byte-for-byte parity between the backend-derived addresses and the wallet's own addresses must be confirmed on staging before the buy flow is un-gated — there is no signer wasm in the test harness to assert it automatically (by design — production never calls the signer).

BREAKING CHANGE: sign_onramper_widget_url no longer accepts wallets / network_wallets / wallet_address_tags and takes no arguments; it derives and signs the caller's own addresses. Clients must stop sending wallet parameters (frontend binding regenerated in this PR).

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