#36 U2F login is broken

Around 20 minutes ago, I added 3 different U2F keys to my tracker account. (same username: thisis_myname).

When I tried logging out and logging back in with the first two U2F keys, I got stuck at this prompt:

This account is protected by a Universal Two Factor token. To continue logging in, please insert your U2F token and press it if necessary.

The LED on my key blinks slowly which confirms that the site is requesting authentication. However, pressing either of the two keys does nothing. The blinking stops but the page doesn’t change.

When I tried logging in with the third key, I get this bizarre error:

You entered an invalid password. You have 2 attempts remaining.

This seems… wrong. Doesn’t the site check my password before asking me to press the U2F key?

I’m guessing there are 2 bugs here:

1. Only my 3rd key got added to the account. Could someone manually check the db and confirm?
2. Even though my 3rd key works, something else is triggering the “invalid password” error.

(Tagging @spaghetti since he committed u2f.php)

Ok I see why the second bug is happening: I HTML-injected myself because my password contains a " and $_POST['password'] is not HTML-escaped in u2f.php

By itself, this isn’t a notable security risk because it only allows users to HTML-inject themselves. It just blocks people like me that use password managers :(

I don’t know PHP, but I’ll see if I can send a PR and fix it.

To find similar bugs, I’d recommend adding a Selenium test that runs on each page containing forms. The test fills in the form with random data and verifies that the HTML returned isn’t malformed. (Do we have continuous integration for this repo?)

If PHP has something similar to Django’s AutoEscaping or a way to prevent non-escaped strings from being implicitly converted to escaped strings using the type system, I’d highly recommend that you guys enable it. There could be other forms on this site that allow one user to HTML-inject a page that’s visible to a different user and therefore compromise that user.

Thanks for the PR. Merged.