Skip to content

fix(signInWithEthereum): secure SIWE verification with domain/nonce v…#1544

Open
maho0638 wants to merge 1 commit into
base:masterfrom
maho0638:patch-5
Open

fix(signInWithEthereum): secure SIWE verification with domain/nonce v…#1544
maho0638 wants to merge 1 commit into
base:masterfrom
maho0638:patch-5

Conversation

@maho0638
Copy link
Copy Markdown

Security Fix

This PR addresses the security vulnerability reported in #1502 where client.verifyMessage was used without validating the SIWE message's domain and nonce fields. This left applications vulnerable to cross-domain replay attacks.

Changes Made

Updated docs/base-account/reference/core/capabilities/signInWithEthereum.mdx:

  1. Added import { verifySiweMessage } from 'viem/siwe';
  2. Replaced 3 instances of client.verifyMessage with verifySiweMessage.
  3. Added domain: req.headers.host and nonce parameters to ensure full EIP-4361 compliance.
  4. Updated the info note at the bottom to reference verifySiweMessage.

Impact

Developers copying these examples will now have secure authentication backends by default, protecting against signature replay attacks across different domains.

Fixes part of #1502.

…alidation in all examples

Replaced insecure `client.verifyMessage` with `verifySiweMessage` in 3 backend code examples to prevent cross-domain replay attacks. 

Added explicit `domain` and `nonce` validation as required by EIP-4361. This ensures that signatures are not only cryptographically valid but also intended for the correct application and not reused.

Updates documentation to reflect the use of `verifySiweMessage` instead of `verifyMessage`.

Part of base#1502.
@cb-heimdall
Copy link
Copy Markdown
Collaborator

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

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.

2 participants