Closed Bug 1121119 Opened 6 years ago Closed 4 years ago

Breakdown: Consider ignoring the formSubmitURL for passwords

Categories

(Toolkit :: Password Manager, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: MattN, Assigned: tanvi)

References

(Blocks 1 open bug)

Details

We should re-evaluate the tradeoffs between the user experience and security.
Taking this bug to outline the security risks we take on by removing this from the password manager.

Original bug filed that added this feature in 2007: https://bugzilla.mozilla.org/show_bug.cgi?id=360493
Assignee: nobody → tanvi
Blocks: 1118400
Priority: -- → P1
The form action is an additional key that we use when we decide whether to fill a password field.  This bug is to consider the risks and benefits of removing this key.

The form action key was originally added in bug 360493.  The issue is that user generated content can add a login form with username and password fields.  The action can be set to a third party site owned by the attacker entering the data.  Websites may be doing proper input validation to protect against XSS (ex: removing <script>) while also allowing users to include some html markup.  If we do not key off of the form action url, the password manager will autofill the password.  However, since the attacker doesn't have javascript access to the page, I don't believe it can actually submit the username and password.  It would have to somehow trick the user into doing that (comments on the feasibility of this would be appreciated - Dan, thoughts?).

If the password manager does not autofill, the user generated content attack becomes less likely, since the attacker has to trick the user into prompting the password manager to fill in their password and hitting submit.

The second attack that this "protects" against is an attacker changing the action url on the original login form.  However, if the attacker can do that, then presumambly they have javascript access via XSS or a MITM.  In which case, they can just capture the password field and send it to their server in a different way.

However, if we implement some variation of secure autofilling (where the password manager autofill a token instead of the actual password until the user hits submit and submits the actual password), then using the form action as a key in the password manager does benefit the user.  In this case, an XSS on the page can't get to the actual password value, but can change the form action url.  If we do implement secure autofilling and keep the form action as a key, we should also be double checking the form action on submit time (to see if javascript on the page has changed it after page load time).

The answer of whether we should or should not remove this key is somewhat dependent on how we handle autofilling.

It might be worth adding telemetry to see how often we don't fill in the password because the form action has changed.  If it is rare, we may consider just leaving it in for now until and if we see a spike.
Flags: needinfo?(dveditz)
(In reply to Tanvi Vyas [:tanvi] from comment #2)
> The form action key was originally added in bug 360493.  The issue is that
> user generated content can add a login form with username and password
> fields.  The action can be set to a third party site owned by the attacker
> entering the data.  Websites may be doing proper input validation to protect
> against XSS (ex: removing <script>) while also allowing users to include
> some html markup.  If we do not key off of the form action url, the password
> manager will autofill the password.  However, since the attacker doesn't
> have javascript access to the page, I don't believe it can actually submit
> the username and password.  It would have to somehow trick the user into
> doing that (comments on the feasibility of this would be appreciated - Dan,
> thoughts?).

The page can also prevent the submission to a third-party @action via CSP form-action[1]. The downside is that a page has to opt-in to this.

> It might be worth adding telemetry to see how often we don't fill in the
> password because the form action has changed.  If it is rare, we may
> consider just leaving it in for now until and if we see a spike.

I can think of at least 3 non-malicious cases where this problem occurs:
A) The site change their submit URL during some rewrite.
B) The site upgrades from HTTP to HTTPS
C) The site does some weird thing with javascript just before the form is submitted and swaps the value of the @action. There are bugs in BMO about this but I didn't look for them now. I don't know why some sites do this. It could be that they are trying to workaround (A) and want browsers to auto-fill existing saved passwords since their users complained about them not being auto-filled anymore or it could be that they are intentionally trying to prevent the password manager from auto-filling..

[1] http://www.w3.org/TR/CSP2/#directive-form-action (Gecko bug 529697)
(In reply to Matthew N. [:MattN] from comment #3) 
> The page can also prevent the submission to a third-party @action via CSP
> form-action[1]. The downside is that a page has to opt-in to this.
> 
Only a small percentage of pages on the web have CSP today .  So we would still be removing this protection for 99% of pages.

> I can think of at least 3 non-malicious cases where this problem occurs:
> A) The site change their submit URL during some rewrite.
Yes, this is definitely the most likely reason the form action changes.

> B) The site upgrades from HTTP to HTTPS
We can and should fix this, as you propose in bug 667233.  Upgrading to the HTTPS url to submit the form would be more than fine.

> C) The site does some weird thing with javascript just before the form is
> submitted and swaps the value of the @action. There are bugs in BMO about
> this but I didn't look for them now. I don't know why some sites do this. It
> could be that they are trying to workaround (A) and want browsers to
> auto-fill existing saved passwords since their users complained about them
> not being auto-filled anymore or it could be that they are intentionally
> trying to prevent the password manager from auto-filling..
>
Hmm.  Intentionally trying to prevent the password manager from auto-filling?  By using a different action url everytime, and then changing it at the last second to the actual action url?
You guys are talking about the submit "URL" (because that's what the data field is called) but it's really an origin. How often does a site change so much that the _site_ to which they submit changes (without also having changed the site the form is on which would break PwdMgr anyway)?

I'd be reluctant to remove this check unless we have data showing that the submitURL is actually a problem for the password manager. I'm happy to address comment 0 but not as a thought experiment.

I'd be happier dropping the requirement (in the absence of data) if autofill required user interaction (if it's a malicious injected form and the user interacts with it then likely they would be fooled into submitting their password whether or not we fill it in for them).

> Websites may be doing proper input validation to protect against XSS (ex: removing <script>) while
> also allowing users to include some html markup.  If we do not key off of the form action url, the
> password manager will autofill the password.  However, since the attacker doesn't have javascript
> access to the page, I don't believe it can actually submit the username and password.  It would
> have to somehow trick the user into doing that (comments on the feasibility of this would be
> appreciated - Dan, thoughts?).

HTML injection + clickjacking would work fine without scripts (on the injected site). This is why X-Frames-Option is so popular on login forms and an army of amateur security fame-n-bounty seekers reporting the lack of XFO to anyone they can think of.
  * if you could inject script you don't need the clickjacking
  * if passwords don't autofill the attack is useless

If the "user action" that triggers autofill is simply clicking on the form then that doesn't really help in the clickjacking case, the submitURL would still  be a useful defense.
Flags: needinfo?(dveditz)
So sounds like the decision of whether to remove the submitURL as a key depends on two things:

1) Data suggesting that this is causing problem for password manager - forms aren't getting filled in because of this criteria, causing bad user experience.  (Can we add telemetry for this?  Record the percentage of times the top level url matches and a password is saved but the form submitURL does not match?)

2) Whether we are going to autofill or not.
(In reply to Tanvi Vyas [:tanvi] from comment #6)
> So sounds like the decision of whether to remove the submitURL as a key
> depends on two things:
> 
> 1) Data suggesting that this is causing problem for password manager - forms
> aren't getting filled in because of this criteria, causing bad user
> experience.  (Can we add telemetry for this?  Record the percentage of times
> the top level url matches and a password is saved but the form submitURL
> does not match?)
> 
Bug 1124888

> 2) Whether we are going to autofill or not.
Bug 1118511
Depends on: 1124888, 1118511
Priority: P1 → --
MattN gathered telemetry about how often we don't match records because of a mismatched formSubmitUrl.  (Thanks Matt!):
https://bugzilla.mozilla.org/show_bug.cgi?id=1124888
http://telemetry.mozilla.org/#filter=beta%2F37%2FPWMGR_FORM_ACTION_EFFECT%2Fsaved_session%2FFirefox&aggregates=multiselect-all!Submissions&evoOver=Builds&locked=true&sanitize=true&renderhistogram=Graph

The data is quite alarming.  17% of password fields that would have had a match don't have any matches after we consider the formSubmitUrl.

There are a few changes we could make to help with this problem.

First, we could stop matching against the scheme for form submit urls and only match the hostname.  Then we can see if the 17% reduces.  Let's consider the cases to see if this is okay from a security perspective:
* HTTP post to HTTPS record, allow HTTP post to HTTP
The page is HTTP already, so the password is already vulnerable to attack.  This makes the attack a bit worse because it sends the password in the clear, but if a MITM is the one changing the form action from HTTPS to HTTP, they could just as easily just scrape the password.

* HTTP post to HTTP record, allow HTTP post to HTTPS
This is a security win.

* HTTPS post to HTTP record, allow HTTPS post to HTTPS
Also a security win.

* HTTPS post to HTTPS record, allow HTTPS post to HTTP
If this happens, the user has to click through this ugly UI before their password is sent:
https://people.mozilla.org/~tvyas/HTTPS-post-HTTP.jpg
Hence, I'm less worried about this case.  We have some telemetry on this UI[1], that does show that most users unfortunately click through it, but it is for forms in general.  Will users still click through for passwords?  For an attack to work here you would need:
* User generated content that adds username/password fields with an HTTP form action to the same domain that the website sends the password to
* Click jack the user into hitting submit
* Then the user has to click through the ugly alert box.

Matt is ready to write a patch for this and land it to see if it helps reduce the 17%.  Dan, are you okay with this?

[1] http://telemetry.mozilla.org/#filter=beta%2F36%2FSECURITY_UI%2Fsaved_session%2FFirefox&aggregates=multiselect-all!Submissions&evoOver=Builds&locked=true&sanitize=true&renderhistogram=Graph
Look at the counts for number 9 and 10
Flags: needinfo?(dveditz)
Other things we could do to help the UX here:
Provide autocomplete UI if form action doesn’t match
Then if the (username password) is the same, update the form action in the record without prompting the user (similar to way we update the lastUsed timestamp).  If the (username,password) different, offer capture.  

Should we support more than one form submit url per record?  Can sites flip back and forth between two different submit domains?  What are other browsers doing?  Do they support more than one form submit url?


What happens for javascript: actions?  What do we store (is it truncated?)  What do we match?
And what happens for javascript: actions?  What do we store (is it truncated?)  What do we match?
(In reply to Tanvi Vyas [:tanvi] from comment #8)
> The data is quite alarming.  17% of password fields that would have had a
> match don't have any matches after we consider the formSubmitUrl.

I have trouble understanding that large a number. It's hard to believe sites are willy-nilly changing the origin to which they submit forms. If the site does change or the original saved form differs from the user's normal login that should be a one-time update/addition of the password and it's hard to believe 20% of login attempts are "second visits" (but not third or later).

Are these on sites where we're unsuccessful at capturing the password if the user does fill it in? Are we trying to autocomplete on pages the user doesn't care about and had no intention of logging in to at that moment (so the password never gets updated)?

> First, we could stop matching against the scheme for form submit urls and
> only match the hostname.

We have no idea that's the problem. Seems unlikely. That sort of change is a one-time site switch, or they have a legit login on each scheme, and in both cases the user ought to have saved the password for "the other one" the first time they logged in that way.

> Let's consider the cases to see if this is okay from a security perspective:

The form submit URL mechanism had nothing to do with transport security; analyzing it from that perspective is not all that informative. We should look at the attack it was designed for and see if we still care about that, or whether we have a better alternative.

It was a very narrow attack scenario:
 1. Attacker can inject HTML (e.g. a <form>) but NOT script.
 2. We autofill injected form (which may be invisible)
 3. User clicks on the submit (which may look like something else)

If the attacker can inject script then they use the correct formSubmitURL and then harvest the data out of the autofilled form and this protection doesn't help at all.

Refusing to autofill UNTIL the user shows an intention to use the form prevents /both/ variations of this attack and we would not need the formSubmitURL at all. My belief (with no data except my own use) is that the 17% figure is inflated by autofill attempts on sites where the user has logged in once to leave a comment, but generally can't be bothered to log into that site otherwise. Firefox is always trying to fill in my password on places like Reddit or Slashdot, and I generally don't think the site needs to be able to track my reading habits by name.

Another variation of the attack is the login form is injected visibly, and the fact that we autofill convinces the user that it's a legit form and they go ahead and submit to the 3rd party location. But really, at that point it's a phishing attack and if the user wants to log in they're going to go get their credentials and log in.

I don't really care much about the formSubmitURL mechanism because it was designed for a very narrow and unlikely attack scenario: a site vulnerable to XSS, but successfully preventing script injection. [I suppose with CSP this scenario is now more likely than when we added formSubmitURL.] Meanwhile if the attacker does have script injection they can 1) bypass this protection easily, and 2) mass harvest autofilled credentials by injecting iframes of all the broken sites they know about. So autofilling is the real danger.

> Matt is ready to write a patch for this and land it to see if it helps
> reduce the 17%.  Dan, are you okay with this?

Sure. The attack was about sending forms offsite not capturing them in transit. I'm not optimistic it will change the numbers, but it's not making the user any less safe to allow http/https equivalence for formSubmitURL.

Just checking, even though it's called formSubmitURL we only look at the origin right? If we are really matching an entire URL then I could easily believe the 17% figure as sites change things around or have multiple submit points.
Flags: needinfo?(dveditz)
My understanding is that a not uncommon practice is to capture the form submit and dynamically fill the form submit action. This causes us to save a different form submit origin at capture time (a real URL) than what is there on page load (nothing or some javascript: URL). 

FWIW, I personally have some sites I use that do this and the password manager captures fine but does not fill or autocomplete. Frustrating. I'm not sure what % of these kinds sites account for the 17%, but it would be nice to drill down on that.

IMO, the right thing to do is to ignore the formSubmit origin entirely, or at least ignore it in the autocomplete UX.
If the user is interacting with the form we're not greatly increasing the risk by then filling it in for them. With or without our help the user is going to submit that form--if it's malicious they've been successfully phished. formSubmitURL is not saving the user in this case, we might as well ignore it and make users happy in the non-malicious cases.

If we're autofilling without user interaction then there might be some value in checking formSubmitURL first.
Chris presents a good explanation for the 17% - the action at fill time is different than the action at capture time.

To proceed here, I think we can safely:
1) not check the scheme of formSubmitURL when we autofill (just check the hostname).
2) allow autocomplete when formSubmitURL doesn't match.

One might help the 17% go down.  Two is not going to change it, but will improve user experience.

Maybe we can do something else to optimize UX for the fill/capture formSubmitUrl differences, but I need to be more familiar with the code before I can make a reasonable suggestion.  Needinfo'ing myself to get back to this.
Flags: needinfo?(tanvi)
Depends on: 1147561
Depends on: 1147563
(In reply to Tanvi Vyas [:tanvi] from comment #14)
> To proceed here, I think we can safely:
> 1) not check the scheme of formSubmitURL when we autofill (just check the
> hostname).

Filed bug https://bugzilla.mozilla.org/show_bug.cgi?id=1147561

> 2) allow autocomplete when formSubmitURL doesn't match.

Filed bug https://bugzilla.mozilla.org/show_bug.cgi?id=1147563
Flags: needinfo?(tanvi)
See Also: → CVE-2006-6077
Blocks: 1167657
No longer blocks: 1167657
Closing this bug per comment 15.  If there is anything left to address, please reopen.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.