liveshowy / webauthn_components

WebauthnComponents allows Phoenix developers to quickly add passwordless authentication to LiveView applications.
MIT License
170 stars 9 forks source link

[BUG] - Value of form input for email is over-trusted #67

Open peaceful-james opened 3 months ago

peaceful-james commented 3 months ago

Description

The handle_info of :registration_successful trusts the assigned form value of the email. The email could be changed during the WebAuthn flow.

Steps To Reproduce

Steps to reproduce the behavior:

  1. Visit http://localhost:4000/sign-in
  2. Enter alice@example.com into the email input.
  3. In console, run this JS:
function changeEmail() {
  var emailInput = document.getElementById("email")
  emailInput.value = "bob@example.com";
  emailInput.dispatchEvent(new Event("input", {bubbles: true}))
}

setTimeout(changeEmail, 10000);
  1. Click "Sign Up".
  2. Wait at most 10 seconds for the email input to change (because of JS from step 3).
  3. Complete the sign up flow successfully.

Expected behavior

The difference in the form email and the email of the created passkey should cause user creation to fail.

Suggestion

Pass the email back as part of the :registration_successful payload and check that the form email matches exactly.

I do not think this is a realistic attack vector for any serious system. Email confirmation should be done after creating the passkey.

type1fool commented 3 months ago

Thank you for the bug report, @peaceful-james. I will look into this.

type1fool commented 3 months ago

I see what's happening here. The email field from the form is being read when :registration_successful is received, where it should be taking the value from the @webauthn_user assign or possibly a separate assign.

It may also be preferable to remove the email form field when the registration starts so it can't be modified. That may lead to a bit of rethinking the design of the authentication_live.ex template used by the code generator. 🤔

Good catch! I will consider breaking up the form into a multistep process, which will probably affect the way the component works as well.

type1fool commented 2 months ago

Relates to #76