[ANCHOR-1248]: Legacy SEP-31 customer ids are attacker-claimable, reopening the KYC IDOR for pre-upgrade customers

[amandagonsalves]

Description

The ANCHOR-1237 fix (commit 063e435, #3827576) introduced sep31_customer_id_owner to stop a caller from naming another customer's SEP-12 id as sender_id/receiver_id. It relies on claim-on-first-reference to cover customers that already existed when an operator deploys the fix: V29__sep31_customer_id_owner.sql creates the table empty, and verifyOrClaim hands an id with no owner row to whoever references it first. That only protects a pre-existing id if its true owner happens to reference it again after upgrading - and a customer that's simply read, updated, or named as receiver by its own anchor never re-claims its id, since only creating a brand-new SEP-31 customer claims one (Sep12Service.putCustomer's isNewSep31Customer gate). So every customer id that existed before an operator deploys ANCHOR-1237, and isn't re-referenced by its owner first, is claimable by an attacker who knows the id - reopening the exact read/overwrite chain #3827576 fixed, for the entire pre-upgrade customer catalogue.

V29 has never shipped in a tagged release (its only commit is on develop; the latest release, 4.5.0, doesn't contain it), so no deployment has it applied. This implements a SQL backfill added directly to V29, plus a small companion Java migration for the one case plain SQL can't handle.

The one part SQL can't do alone: creator is stored as a JSON blob in a VARCHAR column, and for muxed-account (M...) callers its memo field is always null — the sub-account id is embedded only in the M-address's StrKey encoding. A pure-SQL backfill would leave these rows at creator_memo = NULL, and since Sep31Service.postTransaction computes a muxed caller's memo as their decoded sub-id (never null), the legitimate owner referencing their own legacy id again would fail the ownership check and get locked out. V30 decodes it using the same MuxedAccount class WebAuthJwt already relies on.

Separately, and independent of the backfill, ClientFinder.getClientName resolves by client domain/signing key only, never consulting the account memo. Both ownership call sites nulled the memo whenever a client name resolved, collapsing distinct sub-users authenticated under one registered client's signing key (different memos or muxed ids) to the same ownership identity. Fixed by keeping the memo always, regardless of whether a client name resolves.

Changes

• V29__sep31_customer_id_owner.sql: adds a backfill insert after the CREATE TABLE, seeding sep31_customer_id_owner from sep31_transaction history. For each customer id ever named as sender_id/receiver_id, DISTINCT ON (customer_id) ordered by started_at, id picks the earliest referencing transaction, and its recorded client_name (if attributed) or raw creator account/memo becomes the owner. Rows with no recoverable identity (creator null and no client_name) are excluded rather than violating the creator_account NOT NULL constraint.
• V30__backfill_muxed_sep31_customer_id_owner.java (new): a BaseJavaMigration that re-derives the same "earliest transaction per customer id" set, filters to muxed-account creators, decodes the sub-id via org.stellar.sdk.MuxedAccount, and updates creator_memo only where it's still NULL - so it can never overwrite a distinct owner.
• Sep31Service.postTransaction: ownerMemo is now always webAuthJwt.getOwnerMemo(), not nulled when a client name resolves.
• Sep12Service.putCustomer: same change to the verifyOrClaim call in the isNewSep31Customer branch - memo is always passed through, no longer nulled when a client name resolves.
• Sep31ServiceTest, Sep12ServiceTest: new tests asserting memo stays distinct per sub-user (one plain-memo, one muxed) even when both resolve to the same client name.

Acceptance Criteria

• A customer id that existed before this fix, was referenced in a transaction created by its true owner, and has not been referenced since, cannot be claimed by a different caller after upgrading — sep31_customer_id_owner already has the correct owner row before any post-upgrade request arrives.
• The above holds for owners identified by plain account+memo, by registered client name, and by muxed account, without locking out the legitimate owner on their next reference.
• Re-running the migration (or booting against an already-migrated database) is a no-op — verified manually (see Testing).
• Two sub-users authenticated under the same registered client's signing key with distinct memos/muxed ids no longer collapse to one ownership identity.
• A customer id with no transaction history and no client_name is skipped by the backfill, not inserted with a null account.

Context

HackerOne #3852332

Testing

• Unit: ./gradlew :core:test --tests "org.stellar.anchor.sep12.Sep12ServiceTest"
• Unit: ./gradlew :core:test --tests "org.stellar.anchor.sep31.Sep31ServiceTest"
• Manual: seeded a throwaway local Postgres 16 with sep31_transaction rows covering a base-account owner referenced by two different transactions (confirming the earliest wins, not the latest), a client-name-resolved owner with a non-null memo, a muxed-account owner, and a transaction with no creator recorded (confirming it's skipped). Ran V29's backfill and V30's decode-and-repair against it directly: all three recoverable identities backfilled correctly, the orphan was skipped, and re-running both migrations against the already-migrated state was a no-op.

Documentation

N/A

Known limitations

A SEP-12 customer created via putCustomer before this fix shipped but never named on any SEP-31 transaction has no recoverable owner anywhere in the platform database - there's no signal linking it to a caller identity, so it remains claimable by whoever references it first after this migration runs. Closing this would require an operator-facing reclaim workflow (e.g., letting an owner explicitly establish ownership of an id they can otherwise prove is theirs), which is out of scope here.

Also deliberately not addressed: if an attacker already claimed a legacy id in the wild before an operator applies this fix, the backfill enshrines that prior claim as legitimate (it can't distinguish a malicious first-reference from a genuine one). Operators who suspect prior exploitation should audit sep31_transaction history for a legacy id's first referencing caller before applying this migration, rather than relying on it as a retroactive remedy.

[Copilot]

Pull request overview

Fixes a regression in SEP-31 customer-id ownership enforcement for pre-upgrade customers by backfilling sep31_customer_id_owner from historical sep31_transaction records, including correct handling for muxed-account (M...) callers, and by ensuring account memos are not dropped when a client name resolves.

Changes:

• Backfills sep31_customer_id_owner in V29 from earliest historical references in sep31_transaction, skipping rows without a recoverable identity.
• Adds a V30 Java migration to decode muxed-account sub-ids and repair creator_memo where it would otherwise remain NULL.
• Ensures ownership checks in SEP-31 and SEP-12 always include the caller memo (including muxed sub-id), plus adds/updates unit tests around memo distinctness.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
platform/src/main/resources/db/migration/V29__sep31_customer_id_owner.sql	Adds a backfill insert to seed ownership rows from earliest sender/receiver references.
platform/src/main/java/db/migration/V30__backfill_muxed_sep31_customer_id_owner.java	Repairs muxed-account ownership rows by decoding and filling creator_memo from M-addresses.
core/src/main/java/org/stellar/anchor/sep31/Sep31Service.java	Passes owner memo even when client_name resolves to keep sub-user ownership distinct.
core/src/main/java/org/stellar/anchor/sep12/Sep12Service.java	Same memo-preserving change for the verifyOrClaim call on new SEP-31 customer creation.
core/src/test/kotlin/org/stellar/anchor/sep31/Sep31ServiceTest.kt	Adds coverage ensuring memo remains distinct across sub-users under the same client name.
core/src/test/kotlin/org/stellar/anchor/sep12/Sep12ServiceTest.kt	Adds coverage for memo preservation when client name resolves (needs muxed-case extension).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.