[ANCHOR-1156] Fix SEP-10 memo validation in SEP24 GET /transaction#1891
[ANCHOR-1156] Fix SEP-10 memo validation in SEP24 GET /transaction#1891
GET /transaction#1891Conversation
GET /transaction
There was a problem hiding this comment.
Pull request overview
This PR fixes a security vulnerability in SEP-24's GET /transaction endpoint where memo validation was broken, potentially allowing unauthorized cross-memo transaction access in custodial account setups. The fix aligns SEP-24 authorization logic with the already-correct SEP-6 implementation.
Changes:
- Fixed broken memo validation logic that was comparing account field against concatenated "account:memo" string
- Changed to use null-safe
Objects.equals()for both account and memo comparison - Added comprehensive test coverage for memo validation scenarios including matching, mismatching, and null cases
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| core/src/main/java/org/stellar/anchor/sep24/Sep24Service.java | Fixed authorization logic to properly validate both account and memo separately using Objects.equals() for null safety |
| core/src/test/kotlin/org/stellar/anchor/sep24/Sep24ServiceTest.kt | Added three new test cases covering memo mismatch, memo match, and null account scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @Test | ||
| fun `test find transaction with null web auth account`() { | ||
| val testTxn = createTestTransaction("deposit") | ||
| testTxn.webAuthAccount = null | ||
| every { txnStore.findByTransactionId(any()) } returns testTxn | ||
|
|
||
| val gtr = GetTransactionRequest(TEST_TRANSACTION_ID_0, null, null, "en-US") | ||
| assertThrows<SepNotFoundException> { sep24Service.findTransaction(createTestWebAuthJwt(), gtr) } | ||
| } |
There was a problem hiding this comment.
Consider adding test cases for two additional edge cases to ensure complete memo validation coverage: (1) A token without a memo attempting to access a transaction with a memo, and (2) A token with a memo attempting to access a transaction without a memo. Both scenarios should throw SepNotFoundException to prevent unauthorized cross-memo access.
Description
This fixes SEP-24
GET /transactionauthorization logic to correctly enforce both account and memo matching.Context
In custodial account setups, the previous logic could allow cross memo transaction fetching if the transaction UUID is known.
Testing
./gradlew testDocumentation
N/A
Known limitations
N/A