656343 - After redirect to page requiring HTTP or FTP basic/username/password auth, the redirect destination is not in the address bar

Status: NEW

(Core :: Networking: HTTP, defect, P3)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

+++ This bug was initially created as a clone of Bug #623155 +++

I saw this when sent a tinyurl.com link that redirected to https://intranet.mozilla.org. The tinyurl.com URL was in the address bar when the HTTP auth dialog popped up. If an attacker could cause a user-trusted website to redirect to their own website, then they could exploit this to steal credentials.

This is similar to bug 623155 and may be caused by bug 311007.

Comment 1

[Daniel Veditz [:dveditz]]

Unrealistic we'd get a fix for Firefox 5 here.

Comment 2

[O. Atsushi (Torisugari)]

Unlike bug 623155, this is not a front-end bug. Mostly http channel, partly docshell and else.

First of all, we should decide which URI the URL bar should display.

Comment 3

[:Gavin Sharp [email: [e-mail]]]

(In reply to O. Atsushi (Torisugari) from comment #2)
> First of all, we should decide which URI the URL bar should display.

The URL of the site that's producing the auth prompt, presumably? What other URI would make sense?

Comment 4

[O. Atsushi (Torisugari)]

The auth prompt is not always from main document. For example, an IMG tag

  <img src="http://image.example.com/icon.png">

requires <image.example.com>'s user/password, but the document URI may be different, like e.g. "http://www.example.com/index.html". Probably the loadgroup's chain will tell us the "main" document's URI, but sometimes that is also confusing to the user.

Anyway, when the channel is asking password to the user, docsehll should not fire onLocationChange, in my opinion. But maybe onStateChange is OK, if we define a new state flag, say, STATE_PENDING_AUTH_INPUT or something.

Alternatively, what makes sense I think is blank bar temporally.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

(In reply to O. Atsushi (Torisugari) from comment #4)
> The auth prompt is not always from main document.

Well, that's a separate issue (related to bug 647010). For the purposes of this bug, we should just be gaining parity with the existing auth prompting cases (the URL of the page containing the resource that causes the prompt should be displayed).

Comment 6

[Honza Bambas (:mayhemer)]

This may be related or have similar solution to bug 528988.

Comment 7

[O. Atsushi (Torisugari)]

(In reply to Gavin Sharp (use [e-mail] for email) from comment #5)
> (the URL of the page containing the resource that causes the prompt should
> be displayed).

"Resource's URL" (nsIChannnel::URI) is not always what we should display. For example, a document may want to change URLbar's string per history.pushState() etc. In such a case, that call may have been done before sub-channles's auth request. That's why doc-loader is hard to fire onLocationChange on a auth request. Sometimes we want to overwrite the URLbar with the resource's URI, not always. Though I'm not too sure what bug 528988 is like...

The candidates are
1. the main resource's URI only before its "onLocationChange".
2. the main resource's URI only after redirect and before onLocationChange.

Comment 8

[:Gavin Sharp [email: [e-mail]]]

I don't really understand what the complication is. We need to be firing onLocationChange (this bug)/onSecurityChange (bug 528988) before we display any auth prompts. Why is that difficult?

Comment 9

[O. Atsushi (Torisugari)]

If on a auth request we blindly call onLocationChange (which is based on the resource URI), such a script does not work as expected:

<script>
function myOnLoadListener() {
  // Now the URLbar is <http://www.example.com/foo.html>

  // This will fire <http://www.example.com/bar.html>'s onLocationChange.
  history.pushState({}, "Bar!!", "bar.html");

  // Now, the URLbar is <http://www.example.com/bar.html>

  var img = new Image();
  img.src = "http://auth.example.com/icon.png"; //require http-auth.

  // after this auth prompt, the URLbar goes back to
  // <http://www.example.com/foo.html> again...?
}
</script>

On this auth prompt, we should not call onLocationChange. This script is very simple, but on a actual document, <img> tags are loaded asynchronously. So one image may loaded before main onLocationChange and another may loaded after main onLocationChange. Therefore we should manage whether or not we have already fired onLocationChange, more strictly than the current implementation. Well, as long as we really want to fire onLocationChange.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

Let me rephrase:
We need to be firing onLocationChange (this bug)/onSecurityChange (bug 528988) before we display any auth prompts for the top-level document.

Comment 11

[O. Atsushi (Torisugari)]

(In reply to Gavin Sharp (use [e-mail] for email) from comment #10)
> Let me rephrase:
> We need to be firing onLocationChange (this bug)/onSecurityChange (bug
> 528988) before we display any auth prompts for the top-level document.

I found another problem. If the size of the top-level document is large enough, probably the user can push the STOP button after its auth dialog. In this case, the wrong URI remains in the URLbar.

Comment 12

[:Gavin Sharp [email: [e-mail]]]

(In reply to O. Atsushi (Torisugari) from comment #11)
> I found another problem. If the size of the top-level document is large
> enough, probably the user can push the STOP button after its auth dialog. In
> this case, the wrong URI remains in the URLbar.

Why is that the wrong URI? The URI will still correctly reflect the partially loaded page.

Comment 13

[O. Atsushi (Torisugari)]

> the partially loaded page.

That is correct in terms of network, but not correct in terms of content document. Until the true-onLocationChange is called, all the navigation UIs of content area, including scrolling and another link-clicking, serve for the previous document. If the user pushes the STOP button between normal link-click (same target window) and true-onLocationChange, the previous document is alive and the user can continue web navigation without session history. If we have fired fake-onLocationChange in advance, the previous document's URLbar is wrong.

PoC:

1. Load this page with Firefox, as you are doing now.
2. Click an auth link:  https://intranet.mozilla.org/
   (even if you don't know the password).
3. In the auth dialog, set focus to the OK button with [TAB] key.
4. Hit [Enter] key.
5. Hit [Escape] key, very quickly.
6. Confirm you can continue this page's web navigation.

What this bug is requesting is to fire fake-onLocationChange between step 2. and step 3. As a result, after step 6., this page's URLbar will be <https://intranet.mozilla.org/>.

Comment 14

[O. Atsushi (Torisugari)]

Therefore maybe we should display pseudo content (an error page will do) to enable the session history (and bfcache) to hook onto, before prompting http-auth dialog. Then the content will fire onLocationChange and onSecurityChange automatically.

Comment 15

[Josh Aas]

Honza - can you lend a hand here?

Comment 16

[Honza Bambas (:mayhemer)]

Idea:

What about to add the information about the URL (and its SSL info..) that requests the auth info directly into the auth dialog?  

OS X, for instance, shows the image path and name with all the execution tree when asking for the root password.  This tells the user exactly where from the password request comes from.

As the dialog comes from chrome, it will be automatically trusted by users.

This fixes this bug, bug 528988 and can make bug 647010 invalid.

I think any plays with onStateChange and onLocationChange could bring just more (security) harm...

Opinions?

Comment 17

[O. Atsushi (Torisugari)]

I like your idea.

However, it does not seem easy to implement it, because we don't have (yet) clear definition of "Requester/Requestee", and thus we don't have a useful API to tell who is the requester. For example, bug 709892 implies we should track the caller of window.open(). And I'm not sure how many ways to request a document transition. First of all, we need a complete list of manners of requesting a URL (JS redirection and NPN_GetURL(...) etc.).

Comment 18

[Honza Bambas (:mayhemer)]

For me the request is the channel, i.e. the URL.  It might not be perfect and I don't think it would be worse then now, but at least would fix this bug.

Technically, every password request is bound to at least one channel and is easily reachable.

Could somehow principal be used as an indication?

Comment 19

[O. Atsushi (Torisugari)]

(In reply to Honza Bambas (:mayhemer) from comment #18)
> For me the request is the channel, i.e. the URL.  It might not be perfect
> and I don't think it would be worse then now, but at least would fix this
> bug.
> 
> Technically, every password request is bound to at least one channel and is
> easily reachable.

Yes, and the current password prompt also shows the URL.

Comment 20

[Johnny Stenback (:jst)]

Moving this bug to Networking: HTTP, cause that seems closer to what this really is about than core security.

Comment 21

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Filed this 3.5 years ago, and it's rated just sec-moderate. Is it OK to open this up?

Comment 25

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 26

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 33

[Martin F.]

Attachment: Screenshot of Firefox 88.0

Comment 34

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 9 duplicates.
:kershaw, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 35

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.