browserpass / browserpass-legacy

Legacy Browserpass repo, development is now happening at:
https://github.com/browserpass/browserpass-extension
MIT License
1k stars 80 forks source link

No auto sign-in with Asus router UI #156

Closed ghost closed 7 years ago

ghost commented 7 years ago

General information

Latest browserpass and extension. The only page I have a problem with is my routers sign in page. It's an Asus Ac87 (running merlin firmware), it autofills fine, but leaves it for me to hit sign in. 1

chrboe commented 7 years ago

Reminds me of #115 (though that should already be fixed).

Are you able to identify a relevant snippet of the HTML source of the page (e.g. the form that's used or the entire SIGN IN dialog)? If so, it would be great if you could post it to a pastebin and link it here or something.

ghost commented 7 years ago

No problem. https://ptpb.pw/UCuV is the entire contents of http://router.asus.com/Main_Login.asp Hope it helps

chrboe commented 7 years ago

Yep, that's what I suspected.

In inject.browserify.js we check for an input element with type == "submit", however this page uses

<div class="button" onclick="login();">Sign in</div>

Which is apparently a custom CSS class to make a div look like a button. The downside, of course, is that this isn't picked up by browserpass.

Any ideas on how we could support this anyone? (without completely botching it by just hardcoding the class=button selector)

/cc @maximbaz @qbit

chrboe commented 7 years ago

Of course we could just fix this and similar cases (which no doubt exist) by manually curating a list of quirks that websites have and including it in our script. So, for instance, if the domain name matches router.asus.com, we would search for the class=button div.

maximbaz commented 7 years ago

Router sign-in pages are known to be developed without a quality in mind. Relying on a <button> (or especially on <div> with a button class) is not an option, I don't want browserpass to fill in my credentials and start clicking on random buttons on the page.

Relying on a domain name is a very ugly workaround :smile: Moreover, the domain name is usually something like 192.168.0.1 in such cases, not router.asus.com.

In the end, I don't think we will support this case, unfortunately. Just like the browser will not submit the form on Enter without having a proper <input type='submit'> in the form, we cannot submit the form for the same reason.

If it was a normal webpage, I would suggest to report an issue with that page to fix the markup, not sure if this is even possible in case of routers.

chrboe commented 7 years ago

Moreover, the domain name is usually something like 192.168.0.1 in such cases, not router.asus.com

That's right, I hadn't thought about that.

Just like the browser will not submit the form on Enter without having a proper in the form

The site can work around that using a snippet like the following (from the router site):

document.form.login_username.onkeyup = function(e){
    e=e||event;
    if(e.keyCode == 13){
        document.form.login_passwd.focus();
        return false;
    }
};

The problem is that we can't do anything to detect that sort of workaround 🙂

I guess it would be possible to drop ASUS an email about that, but I think the chances of a single person winning the lottery twice in a row are higher than them actually doing something about this problem (or even as much as caring about it).

ghost commented 7 years ago

Asus might be open to it, AsusWRT is GPL'd and hence what the Merlin firmware is based https://github.com/RMerl/asuswrt-merlin/blob/master/release/src/router/www/Main_Login.asp If not Asus, then Merlin. I'm fine with it working as-is. Only adds 0.1s to my login experience :)

chrboe commented 7 years ago

Hey, that's actually amazing! I really wouldn't have thought that 😃 Do you want to go ahead and create an issue on their side?

I guess that also makes this issue a "wont fix" for us?

ghost commented 7 years ago

Mmm I dunno. I suck at explaining stuff and not sure I'm knowledgeable enough to converse with those guys if they start asking questions. Would rather someone else do it :) I can live with the 'problem'.

ghost commented 7 years ago

heh, thanks

maximbaz commented 7 years ago

Thanks for following up on this 👍 I'll close this issue, as there is indeed no good way for us to fix it.

ghost commented 7 years ago

I'm actually a couple of versions behind (380.65) so that's a few months and it might be different now. Could you check their git to see if that file has the same issue?

chrboe commented 7 years ago

Nope, it's still there.