dabit3 / sign-in-with-ethereum-authentication-flow

Example implementation of how to implement Sign In with Ethereum
https://sign-in-with-ethereum-dabit3.vercel.app/
85 stars 8 forks source link

Nonce - replay attack? #1

Open carletex opened 2 years ago

carletex commented 2 years ago

Hi @dabit3

It doesn't seem possible to comment on your mirror.xyz article so I'm using GH issues for it.

Trying to understand the need for a Nonce when dealing with signatures, I got to the conclusion that the main reason to use a Nonce is to prevent replay attacks.

Looking at your code I see that you are generating a new nonce when requesting the signature message (auth function), but not when verifying the signature (verify function). Wouldn't be this an attack vector for a replay attack? If I'm able to sniff your verify request I could send the same payload to the server and get a valid authentication.

However, if you generate a new Nonce on every verify function call, the signature will become obsolete immediately, preventing someone reusing the same signature.

I might be missing something. What do you think?

P.S. Thanks for all the good quality content that you put out there. Really useful for experienced web2 devs.

dabit3 commented 2 years ago

Hey, thank you for flagging this! Let me dive into this code this week and report back, this might be something I need to improve / fix!

carletex commented 2 years ago

Great!

It could simply be to generate a new Nonce for the user after every verify call:

export default function verify(req, res) {
  let authenticated = false
  const {address, signature} = req.query
  const user = users[address]
  const decodedAddress = ethers.utils.verifyMessage(user.nonce.toString(), signature)

  // New Nonce to prevent replay attacks
  const nonce = Math.floor(Math.random() * 10000000)
  user.nonce = nonce
  users[address] = user

  if(address.toLowerCase() === decodedAddress.toLowerCase()) authenticated = true
  res.status(200).json({authenticated})
}

Optionally we could just return the existing user (with the Nonce generated in verify) for existing users in auth:

export default function auth(req, res) {
  const {address} = req.query
  let user = users[address]
  if (!user) {
    user = {
      address,
      nonce: Math.floor(Math.random() * 10000000)
    }
    users[address] = user
  } 
  res.status(200).json(user)  
}

What do you think?