Skip to content

[ANCHOR-1215]: Anchor Platform SEP-12 IDOR via unvalidated customer id allows cross-customer KYC disclosure and payout destination overwrite#1946

Merged
amandagonsalves merged 102 commits into
developfrom
fix/idor-via-sep12
Jun 5, 2026
Merged

[ANCHOR-1215]: Anchor Platform SEP-12 IDOR via unvalidated customer id allows cross-customer KYC disclosure and payout destination overwrite#1946
amandagonsalves merged 102 commits into
developfrom
fix/idor-via-sep12

Conversation

@amandagonsalves

@amandagonsalves amandagonsalves commented Jun 1, 2026

Copy link
Copy Markdown
Collaborator

Description

Before this change, `Sep12Service.validateGetOrPutRequest` enforced authorization on `request.account` and `request.memo` only. When a caller supplied `request.id` without `account`, the account check short-circuited entirely, allowing any valid SEP-10 session to `GET` or `PUT` any customer record by its `id` — regardless of who created it. The `id` was forwarded unmodified to the business-server callback, so any conforming business server would act on it.

The fix introduces a `sep12_customer_ownership` table. Every successful `PUT /sep12/customer` writes the caller's identity (`stellar_account`, `memo`) as the owner of the returned `customer_id`. Subsequent `GET` or `PUT` requests that supply `id` are checked against this table: if the stored owner does not match the token, the request is rejected with 403 before the business server is called. No business server contract changes are required.

Changes

  • `Sep12CustomerOwnership` / `Sep12CustomerOwnershipStore`: new core interfaces defining the ownership record model and its storage contract.
  • `JdbcSep12CustomerOwnership` / `JdbcSep12CustomerOwnershipRepo` / `JdbcSep12CustomerOwnershipStore`: platform JDBC implementation backed by the new `sep12_customer_ownership` table.
  • `V29__add_sep12_customer_ownership.sql`: Flyway migration creating the table (`customer_id PK`, `stellar_account NOT NULL`, `memo`).
  • `Sep12Service.putCustomer`: after every successful callback response, saves `(customer_id, token_account, token_memo)` to the ownership store, overwriting any prior record on update.
  • `Sep12Service.validateGetOrPutRequest`: when `id` is present and `transactionId` is absent, looks up the ownership record. If a record exists and the token's identity does not match, returns 403 before reaching the business server. If no record exists (customer predates the fix or SEP-31 first-time access), the request is allowed through with the caller's account injected, preserving backward compatibility.
  • `SepBeans` / `DataBeans`: wire `Sep12CustomerOwnershipStore` into the Spring context.
  • `Sep12ServiceTest`: mocked `Sep12CustomerOwnershipStore`; updated existing id-path tests; added IDOR unit tests covering mismatched account rejection, mismatched memo rejection, no-record allow-through, ownership save on PUT (plain and muxed), transactionId path bypass, and forward-account-to-callback behaviour.
  • `Sep12Tests`: added `test cross-account id access is rejected` integration test — registers a victim customer (ownership row written), attempts raw HTTP `GET` and `PUT` against that `id` from a different SEP-10 session, asserts 403 on both, and verifies the victim's record is unchanged.

Acceptance Criteria

  • `GET /sep12/customer?id=` with a SEP-10 token that does not own that customer returns 403.
  • `PUT /sep12/customer` with `{"id":"", ...}` from a different SEP-10 session returns 403.
  • The same `GET` and `PUT` succeed when issued by the token that owns the customer record.
  • No ownership record (first access or pre-existing customer) allows the request through with the caller's account injected.
  • The `transactionId` path is unaffected.
  • All existing `Sep12ServiceTest` tests pass.

Context

HackerOne #3735379

Testing

  • Unit: `./gradlew :core:test --tests "org.stellar.anchor.sep12.Sep12ServiceTest"`
  • Integration: `./gradlew runEssentialTests` — covers `test cross-account id access is rejected` in `Sep12Tests`

Documentation

N/A

Known limitations

Customers created before this migration has run have no ownership record. On their next `PUT /customer`, the record is written and all subsequent id-based requests are fully enforced. Until then, id-based access for those customers falls back to the caller's account being injected, relying on the business server's own id+account consistency check.

* add unique constraint to `transaction_id` on `exchange_quote` table

* ensure single-use integrity for exchange quotes
* add ability to bind a sep38 quote to a transaction id
* add `bindquotetotransaction` method to bind a quote to a transaction

* add validation in `getquote` to prevent fetching a quote already linked to a transaction
* add logic to bind a sep38 quote to a sep31 transaction

* update validation to ensure sep38 quotes are used only once
* add sql update query to bind a quote to a transaction in `JdbcSep38QuoteRepo`

* add public method to bind a quote to a transaction in `JdbcSep38QuoteStore`
* add transactional annotation to sep24 withdraw and deposit methods

* add rollback rules for anchor, malformed url, uri syntax, and runtime exceptions

* add logic to bind quotes to transactions for both withdraw and deposit flows
* add `@transactional` annotation to `depositExchange` and `withdrawExchange` methods

* add logic to bind quotes to sep-6 transactions for deposit and withdraw exchange requests
* add `exchangeamountscalculator` dependency

* refactor quote binding logic

* update `sep31service` to delegate quote binding
* add `exchangeamountscalculator` dependency to `sep31service` to handle quote binding.
* add `sep38QuoteStore.bindToTransaction` mock behavior

* update sep24 service tests to account for quote binding during withdraw and deposit flows
* add `exchangeamountscalculator` to `sep31service` constructor

* update `sep31service` bean creation to include `exchangeamountscalculator`
* add tests for quote binding in `exchangeamountscalculator`

* add tests to verify rejection of already bound quotes

* add tests for successful and failed quote binding attempts
* add tests to sep24 service for quote reuse rejection during withdraw and deposit

* add tests to sep6 service for quote reuse rejection during deposit exchange and withdraw exchange
* add a new test case for sep38 quotes

* test that a sep38 quote can only be used once

* verify error when reusing a quote across sep-31, sep-6, and sep-24 flows
* refactor how exchangeamountscalculator is provided to sep31service

* update exchangeamountscalculator instantiation within the service method
* add a test for concurrent sep31 post transaction calls

* verify that only one transaction succeeds with the same quote id

* verify the correct error message for failed concurrent transactions
* add new tests for the `jdbcsep38quotestore`

* verify the `bindtotransaction` method's behavior

* ensure correct handling of concurrent quote binding attempts
* add test for `calculateFromQuote` to reject quote used by a cancelled transaction

* add test for `bindQuoteToTransaction` to reject quote used by a cancelled transaction
* update `@transactional` annotation import from spring to jakarta
* create a dedicated bean for `ExchangeAmountsCalculator`

* update `sep6Service` and `sep24Service` to use the new `ExchangeAmountsCalculator` bean

* add conditional creation for `ExchangeAmountsCalculator` bean
* refactor the transactional annotation import to jakarta

* update the transactional api from spring to jakarta
* reorder `jakarta.transaction.transactional` import
…ar/anchor-platform into fix/enforce-sep38-quote-single-use
* delete local variable for exchange amounts calculator

* add exchange amounts calculator directly in sep31 service constructor
### Description
This PR solves #1934 by adding `bindToTransaction` to `Sep38QuoteStore`
interface and `bindQuoteToTransaction` to `ExchangeAmountsCalculator`,
`Sep6Service` and `Sep24Service`. It throws if a quote is already bound
to a transaction.

#### Changes
- [x] Validator check. Reject in
ExchangeAmountsCalculator.validateQuoteAgainstRequestInfo and
Sep31Service.preValidateQuote when quote.getTransactionId() != null.
- [x] Atomic bind. In SEP-31 / SEP-24 / SEP-6 transaction-creation
paths, mark the quote consumed via a conditional UPDATE on
exchange_quote (set transaction_id only where it's currently NULL),
inside the same DB transaction that persists the SEP transaction. Zero
rows affected → reject and roll back. This closes the TOCTOU race that a
plain read-then-write would leave open. Implement at the shared
Sep38QuoteStore` layer so cross-protocol attempts are covered.
- [x] DB backstop. Migration adding UNIQUE on
`exchange_quote.transaction_id`` for defense in depth.

#### Acceptance Criteria
- [x] Validator check, atomic bind, and UNIQUE migration all landed.
- [x] Same quote_id cannot be bound twice within SEP-31, SEP-24, or
SEP-6.
- [x] Same quote_id cannot be bound across different SEPs.
- [x] Cancelled / refunded / expired transactions do not free the quote.
- [x] Concurrent transaction-creation requests with the same quote_id
result in exactly one success and one rejection.
- [x] Existing SEP-31 / SEP-24 / SEP-6 happy paths continue to pass.

### Context
#1934 

### Testing
- [x] Validator rejects when quote.getTransactionId() != null (both
validators).
- [x] Validator still passes for unbound quote (regression).
- [x] Sep38QuoteStore atomic-bind method returns true once, false on
second call.
- [x] Integration: SEP-31 → SEP-31 reuse rejected; SEP-31 → SEP-24 reuse
rejected; SEP-31 → SEP-6 reuse rejected.
- [x] Cancellation regression: bind T1, transition to
error/cancelled/expired, attempt to bind T2 → still rejected.
- [x] Concurrency: two parallel POST /sep31/transactions with same
quote_id → exactly one 201, one 400.
- [x] Migration test: UNIQUE constraint rejects duplicate non-null
transaction_id inserts on supported DBs.
* delete account field from getcustomerresponse

* delete memo field from getcustomerresponse

* delete memo type field from getcustomerresponse

* stop populating these fields in the customer service
* refactor customer authorization logic for requests containing an id

* update customer retrieval to ensure the customer is owned by the token's account

* fix authorization check to compare the requested id with the owned customer's id
* fix: wrap customer id authorization logic in try-catch

* fix: log unexpected exceptions during id authorization

* fix: return consistent authorization error for id lookups
* update tests for customer id path authorization logic

* remove redundant account from mock prefetch responses

* add tests for id ownership mismatch and callback exception handling

* refactor how token account and memo pass to callback integration

* update request object with token account and memo after validation
* refactor sep12 customer id ownership to prevent IDOR when memos are used

* update customer lookup to pass customer id for ownership verification

* add an independent memo-based customer lookup to verify token memo ownership

* refactor memo handling in id path requests
Comment thread core/src/main/java/org/stellar/anchor/sep12/Sep12Service.java
Comment thread core/src/main/java/org/stellar/anchor/sep12/Sep12Service.java Outdated
* delete redundant customer lookup for memo validation

* update initial getcustomer request to use memo and memo type for id validation
* fix customer id prefetch assertion for sep31-receiver

* delete idor reverse lookup test
Comment thread core/src/main/java/org/stellar/anchor/sep12/Sep12Service.java Outdated
* update customer id validation logic

* ensure requested customer id matches owned customer id for authorization
* refactor customer id validation process

* update authorization error messages

* update conditions for memo validation
* refactor error handling for customer id retrieval path

* update generic exceptions to map to customer id not authorized error

* fix memo validation to skip when customer id is provided

* update exception logging for customer id validation

@JiahuiWho JiahuiWho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Don't forget to squash merge

@amandagonsalves amandagonsalves merged commit 842cf83 into develop Jun 5, 2026
14 of 16 checks passed
@amandagonsalves amandagonsalves deleted the fix/idor-via-sep12 branch June 5, 2026 17:51
amandagonsalves added a commit that referenced this pull request Jun 8, 2026
### Description

Merges `release/4.4.0` into `main` for the 4.4.0 release.

### Context

- **[ANCHOR-1215]** Fix SEP-12 IDOR via unvalidated customer `id` —
prevents cross-customer KYC disclosure and payout destination overwrite
(#1946)
- **[ANCHOR-1943]** Fix RPC observer DoS via malformed Soroban transfer
event (#1944)
- **[ANCHOR-1939]** Add per-client event isolation and ambiguous payment
routing safety (#1941, #1942)
- **[ANCHOR-1934]** Enforce single-use on SEP-38 quotes (#1935)
- **[ANCHOR-1206]** Add Gateway API support to Helm charts and carry
forward security headers (#1936, #1940)
- **[Chore]** Bump Soroban SDK from v22 to v26 (#1937)
- **[Chore]** Bump version to 4.4.0


[ANCHOR-1215]:
https://stellarorg.atlassian.net/browse/ANCHOR-1215?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
[ANCHOR-1206]:
https://stellarorg.atlassian.net/browse/ANCHOR-1206?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
amandagonsalves added a commit that referenced this pull request Jun 8, 2026
### Description

Merges main into `develop` for the 4.4.0 release.

### Context

- **[ANCHOR-1215]** Fix SEP-12 IDOR via unvalidated customer `id` —
prevents cross-customer KYC disclosure and payout destination overwrite
(#1946)
- **[ANCHOR-1943]** Fix RPC observer DoS via malformed Soroban transfer
event (#1944)
- **[ANCHOR-1939]** Add per-client event isolation and ambiguous payment
routing safety (#1941, #1942)
- **[ANCHOR-1934]** Enforce single-use on SEP-38 quotes (#1935)
- **[ANCHOR-1206]** Add Gateway API support to Helm charts and carry
forward security headers (#1936, #1940)
- **[Chore]** Bump Soroban SDK from v22 to v26 (#1937)
- **[Chore]** Bump version to 4.4.0


[ANCHOR-1215]:
https://stellarorg.atlassian.net/browse/ANCHOR-1215?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
[ANCHOR-1206]:
https://stellarorg.atlassian.net/browse/ANCHOR-1206?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
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.

3 participants