security: fix non-functional and replayable Ethereum signature check in external-delegate-token-master

[NikkiAung]

Summary

tokens/external-delegate-token-master gates a Solana token transfer on an Ethereum ECDSA
signature via transfer_tokens. It has two independent bugs, and — the pattern is the
same as #690 — they mask each other, so fixing only the obvious one would make the other
immediately exploitable. Both survived because transfer_tokens, the one instruction this
example exists to demonstrate, had zero test coverage; the only tested transfer path was
authority_transfer, which does no Ethereum check at all.

Bug 1 — the address derivation is cryptographically wrong (100% false-negative)

recovered_address.copy_from_slice(&keccak256(&pubkey_bytes[1..])[12..]);

&pubkey_bytes[1..] assumes to_bytes() returns a 0x04-prefixed 65-byte SEC1 key. It
doesn't. Checked directly against the pinned crate's source
(solana-secp256k1-recover = "2.0.0"):

pub const SECP256K1_PUBLIC_KEY_LENGTH: usize = 64;
pub struct Secp256k1Pubkey(pub [u8; SECP256K1_PUBLIC_KEY_LENGTH]);
pub fn to_bytes(self) -> [u8; 64] { self.0 }

It's the bare 64-byte X || Y, no prefix — matching Solana's own canonical
construct_eth_pubkey cookbook recipe, which hashes the full slice with no skip. So [1..]
here hashes 63 bytes starting one byte into the X-coordinate. No genuine Ethereum signature
could ever pass this check — transfer_tokens was dead code for legitimate use. Verified
empirically: a standalone script that registers a real, correctly-derived Ethereum address and
signs with the matching key still gets rejected with InvalidSignature against the unpatched
code.

Bug 2 — the signature authorizes nothing (unbound, replayable)

message: [u8; 32] was a free-form, caller-supplied value with zero cryptographic tie to
amount, recipient_token_account, user_account, or any nonce. The check only proved "this
key signed some 32 bytes at some point," never "this key authorized this transfer." Fixing
only Bug 1 — the natural, obvious typo to spot — would have produced a working signature
check that's fully replayable: a captured (message, signature) pair could be resubmitted for
any amount, to any recipient, indefinitely.

Fix

Both bugs fixed together in transfer_tokens:

• &pubkey_bytes[1..] → &pubkey_bytes[..].
• The signed digest is now derived on-chain, not supplied by the caller, binding every value
  that determines where funds move plus a replay guard:

  keccak256(domain_separator || program_id || user_account || user_token_account
            || recipient_token_account || amount || nonce)
  

  user_token_account is bound explicitly (not just the recipient) since nothing constrains it
  to a canonical ATA. Added UserAccount.nonce: u64, incremented with checked_add after a
  successful transfer (this repo's overflow-checks = true release profile panics rather than
  wraps on overflow, matching the convention security: fix swapped transfer amounts + broken invariant check in token-swap #690 established).

authority_transfer is untouched — it's gated by a real has_one = authority Ed25519 signer
check and is a legitimate, separate, lower-friction path for the account's Solana-side owner.

Test changes

Added the transfer_tokens coverage that never existed, using @noble/curves (secp256k1
signing with recovery bit) and @noble/hashes (keccak256) as devDependencies — both already
resolved elsewhere in this repo's dependency tree:

• Happy path: valid signature, balances move, nonce advances.
• Replay: the exact signature from the happy path, resubmitted after the nonce advanced → rejected.
• Parameter tampering: a signature valid for one amount, submitted with a different amount → rejected.
• Wrong signer: a correctly-bound digest signed by an unregistered key → rejected.

Note this changes transfer_tokens's instruction args (drops message) and UserAccount's
account layout (+8 bytes for nonce) — breaking for anything already deployed against the old
layout, expected for an educational example fix.

Verification

• anchor build, cargo check -p external-delegate-token-master, cargo fmt -p external-delegate-token-master, pnpm exec tsc --noEmit, prettier --check all pass.
• Full pnpm test suite: 7/7 passing.
• Captured the fix as a patch, reverted only lib.rs, rebuilt, and confirmed Bug 1 reproduces
  against the unpatched code via a standalone script (real key, real signature, still rejected);
  reapplied the patch and confirmed the full suite passes green.

[greptile-apps]

Greptile Summary

This PR repairs Ethereum signature verification and makes transfer authorizations parameter-bound and single-use.

• Hashes the full recovered secp256k1 public key when deriving the Ethereum address.
• Derives the signed digest on-chain from the program, accounts, amount, and nonce.
• Adds nonce-backed replay protection and tests valid transfers, replay attempts, parameter tampering, and incorrect signers.
• Adds Noble cryptography packages for test-side signing and hashing.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
tokens/external-delegate-token-master/anchor/programs/external-delegate-token-master/src/lib.rs	Corrects Ethereum address recovery and introduces an account-bound, amount-bound, nonce-protected transfer digest.
tokens/external-delegate-token-master/anchor/tests/litesvm.test.ts	Adds coverage for successful signed transfers, replay rejection, amount tampering, and wrong Ethereum signers.
tokens/external-delegate-token-master/anchor/package.json	Adds cryptographic test dependencies used to construct Ethereum addresses, digests, and recoverable signatures.
tokens/external-delegate-token-master/anchor/pnpm-lock.yaml	Locks the newly declared Noble test dependencies and their resolved versions.

_{Reviews (2): Last reviewed commit: "style: match root prettier config in ext..." | Re-trigger Greptile}

[NikkiAung]

Hey @dev-jodee — this is ready for review whenever you get a chance. CI is green and Greptile's automated pass found no blocking issues.