fix(anchor-sdk)!: validate SEP-10 challenges before handing them to a signer - #999
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This was referenced Aug 8, 2026
… signer
`Sep10Client.authenticate()` fetched the anchor's challenge and passed the
raw XDR straight into the caller-supplied `sign` callback with no checks at
all. None of the SEP-10 mandated client-side validation ran: no verification
of the anchor's signature over the transaction, no source-account check, no
`sequence == 0`, no `<home_domain> auth` Manage Data operation check, no
operation-source check, no time bounds. `network_passphrase` was parsed by
the Zod schema and then never compared to anything.
The giveaway was in sep1.ts: `SIGNING_KEY` — the single value that can prove
a challenge came from the anchor — was parsed out of `stellar.toml` and never
read by any code path.
Impact: a hostile or compromised anchor, or an on-path attacker against a
plain-http WEB_AUTH_ENDPOINT (the constructor accepted one), could return an
ordinary transaction instead of a challenge — a payment, or a set_options
adding a signer — and have it blind-signed by the consumer's wallet, hardware
device, or KMS. The module docstring offered "this package never holds key
material" as a safety property; not holding the key does not make signing an
unchecked transaction safe, it just relocates the consequence to whoever does
hold it.
Fix: `verifyChallenge()` runs `WebAuth.readChallengeTx` — the reference
implementation of every required check — and `authenticate()` calls it BEFORE
`sign`. A `network_passphrase` that disagrees with the configured network is
rejected first, since otherwise the signature would be checked against the
wrong network's transaction hash. Non-https endpoints are refused at
construction.
`WebAuth` is re-exported from pulse-core rather than added as a direct
dependency of anchor-sdk, following the existing `StrKey` re-export at
pulse-core/src/index.ts:45. pulse-core already depends on
@stellar/stellar-sdk, so the runtime dependency tree is unchanged and the
bundle-size job is unaffected.
`Sep10Client.fromToml(toml, homeDomain)` is the recommended constructor: it
takes SIGNING_KEY and NETWORK_PASSPHRASE from the toml the caller already
fetches, so the verification key cannot be forgotten. Anchors publishing no
SIGNING_KEY are refused rather than trusted.
BREAKING CHANGE: `Sep10Client` now requires `serverAccountId`,
`networkPassphrase`, `homeDomain` and `webAuthDomain`. These cannot be
optional — an optional verification key is one callers omit, which is the
bug. Taken under the security exception in STABILITY.md ("if a covered
surface is itself the vulnerability"); anchor-sdk goes 0.1.0 -> 0.2.0 and
CHANGELOG.md carries the required `### Security` entry with migration.
A GitHub Security Advisory still needs publishing per SECURITY.md.
Tests: 14 new cases in test/sep10.test.ts built against real challenge
transactions. Each rejection case asserts `sign` was never called — that the
challenge is refused is secondary, that it never reached the signer is the
point. Verified as genuine regressions: with the `verifyChallenge` call
removed, 6 of them fail.
The three SEP-10 cases in test/sep24.test.ts moved here. They drove
authenticate() with placeholder XDR ("AAAA-challenge") and only passed
because nothing inspected it; they now use real challenges.
67 tests pass in anchor-sdk, 614 in pulse-core.
determined-001
force-pushed
the
fix/sep10-challenge-validation
branch
from
August 10, 2026 00:18
1018fab to
2900c1b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The vulnerability
Sep10Client.authenticate()fetched the anchor's challenge and passed the raw XDR straight into the caller-suppliedsigncallback with no checks at all. None of the SEP-10 mandated client-side validation ran:sequence == 0<home_domain> authManage Data operation checknetwork_passphrasewas parsed by the Zod schema and then never compared to anythingThe giveaway is in
sep1.ts:SIGNING_KEY— the single value that can prove a challenge came from the anchor — was parsed out ofstellar.tomland never read by any code path.Impact
A hostile or compromised anchor, or an on-path attacker against a plain-
httpWEB_AUTH_ENDPOINT(the constructor accepted one), could return an ordinary transaction instead of a challenge — a payment, or aset_optionsadding a signer — and have it blind-signed by the consumer's wallet, hardware device, or KMS.The module docstring offered "this package never holds key material" as a safety property. Not holding the key does not make signing an unchecked transaction safe; it relocates the consequence to whoever does hold it.
This is shipped on npm as
@orbital-stellar/anchor-sdk@0.1.0.The fix
verifyChallenge()runsWebAuth.readChallengeTx— the reference implementation of every required check — andauthenticate()calls it beforesign. Anetwork_passphrasethat disagrees with the configured network is rejected first, since otherwise the signature would be checked against the wrong network's transaction hash. Non-httpsendpoints are refused at construction.WebAuthis re-exported frompulse-corerather than added as a direct dependency, following the existingStrKeyre-export atpulse-core/src/index.ts:45.pulse-corealready depends on@stellar/stellar-sdk, so the runtime dependency tree is unchanged and thebundle-sizejob is unaffected.Sep10Client.fromToml(toml, homeDomain)is the recommended constructor — it takesSIGNING_KEYandNETWORK_PASSPHRASEfrom the toml the caller already fetches, so the verification key cannot be forgotten. Anchors publishing noSIGNING_KEYare refused rather than trusted.Sep10Clientnow requiresserverAccountId,networkPassphrase,homeDomain,webAuthDomain. These cannot be optional — an optional verification key is one callers omit, which is the bug.Taken under the security exception in
STABILITY.md("if a covered surface is itself the vulnerability").anchor-sdkgoes0.1.0→0.2.0;CHANGELOG.mdcarries the required### Securityentry with migration.Still outstanding: a GitHub Security Advisory needs publishing per
SECURITY.md.Tests
14 new cases in
test/sep10.test.ts, built against real challenge transactions. Each rejection case assertssignwas never called — that the challenge is refused is secondary; that it never reached the signer is the point.Verified as genuine regressions: with the
verifyChallengecall removed, 6 of them fail.The three SEP-10 cases in
test/sep24.test.tsmoved here. They droveauthenticate()with placeholder XDR ("AAAA-challenge") and only passed because nothing inspected it; they now use real challenges.70 tests pass in
anchor-sdk, 614 inpulse-core.