You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
fix(anchor-sdk)!: validate SEP-10 challenges before handing them to a signer (#999)
`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.
Co-authored-by: Salmatcre8 <118213044+Salmatcre8@users.noreply.github.com>
0 commit comments