fix(base-account): parse the SIWE nonce correctly and bind verification to the app domain - #1794
Open
Dusk1e wants to merge 1 commit into
Open
fix(base-account): parse the SIWE nonce correctly and bind verification to the app domain#1794Dusk1e wants to merge 1 commit into
Dusk1e wants to merge 1 commit into
Conversation
Collaborator
🟡 Heimdall Review Status
|
…on to the app domain
The "Authenticate users" guide ships a server example that cannot
authenticate anyone, and a verification step that accepts signatures
minted for other sites.
Nonce extraction used `/at (\w{32})$/`. In an EIP-4361 message the nonce
sits on a `Nonce: <value>` line with `Issued At:` after it, so the regex
never matches and `/auth/verify` answers 400 "Invalid or reused nonce"
for every valid login. Two other pages in this repo already read the
nonce correctly, so the guide was the odd one out. Switching to viem's
`parseSiweMessage` removes the regex entirely.
Verification called `client.verifyMessage`, which only checks that the
signature matches the message. It does not look at the `domain` field,
so a signature a user produced on another site verifies against this
endpoint too. EIP-4361 requires the relying party to check `domain`.
`verifySiweMessage` checks domain, nonce and expiry, and still routes
through `verifyHash`, so ERC-6492 and ERC-1271 signatures from
undeployed Base Accounts keep working.
Verified against a local node with real signatures: a legitimate login
returns 200, a replayed signature 400, a signature minted for another
domain 401, and an unissued nonce 400. Before the change the first case
returned 400 and the third returned 200.
Fixes base#1502
Dusk1e
force-pushed
the
fix/siwe-domain-binding-and-nonce-parsing
branch
from
August 10, 2026 23:56
c5f7105 to
32bddba
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
Authenticate usersguide ships an Express example that rejects every valid login, and a verification step that accepts signatures a user produced on someone else's site. Both are in code a developer is meant to copy into a production auth endpoint.1. The nonce is never extracted, so
/auth/verifyalways returns 400docs/base-account/guides/authenticate-users.mdx:207reads the nonce with:In an EIP-4361 message the nonce is on its own
Nonce: <value>line andIssued At:comes after it, so nothing sits at the end of the string for$to anchor to. The match is alwaysnull, the guard below it fires, and the endpoint answers400 Invalid or reused noncefor a completely valid login.Using the message format this repo documents at
docs/base-account/reference/core/capabilities/signInWithEthereum.mdx:128:Two other pages in this repo already read the nonce correctly —
signInWithEthereum.mdx:216andframework-integrations/privy/authentication.mdx:227— so this guide was the only one reading it incorrectly. This PR uses viem'sparseSiweMessagerather than another regex.2. Verification is not bound to your domain
Both snippets verified with:
verifyMessageonly answers "does this signature match this message". It never looks at thedomainfield, so a SIWE signature a user was asked to produce onevil.comverifies against your endpoint as well. EIP-4361 requires the relying party to checkdomainagainst its own host (issue #1502).Swapped to
verifySiweMessage, which validatesdomain,nonceand expiry before checking the signature. It still routes throughverifyHashinternally, so ERC-6492 and ERC-1271 signatures from not-yet-deployed Base Accounts keep verifying exactly as before — the guide's own note at line 62 stays accurate.Verification
I ran the proposed server code unmodified against a local node, signing real messages with viem:
400 Invalid or reused nonce200 { ok: true }400400 Invalid or reused nonceevil.com200 { ok: true }401 Invalid signature400400 Invalid or reused nonceRow 1 is the functional bug, row 3 is the security one. Rows 2 and 4 confirm the existing replay protection still behaves.
Relationship to the existing PRs on this file
Five PRs already touch this guide, all opened in May and all still unreviewed. For whoever triages this, here is what each actually changes, checked against viem 2.55.13:
const { isValid } = await verifySiweMessage(...). That function returns aboolean, soisValidisundefinedand the endpoint would answer 401 for every login.nonces.delete(nonce)to after verification, so a nonce survives a failed attempt and two concurrent requests can both pass thehas()check.parseSiweMessage(...).domaincomparison; keepsverifyMessage.noncetoverifySiweMessage, but thatnoncestill comes from the broken regex.Math.random()nonces), no overlap.#1542 covers the same two defects this PR does and is the older submission. The differences here are that the nonce stays an atomic check-and-consume (
nonces.deleteas the guard, as in the current guide), the domain constant does not default tolocalhost:3000, and the behaviour is backed by the run above. If you would rather take #1542, this can be closed — the important thing is that one of them lands, because the guide is currently broken for every reader who copies it.Notes
node scripts/lint-mdx.js docs/base-account/guides/authenticate-users.mdxreports the same 9 errors and 1 warning before and after this change — no new findings introduced.