1091287 - [e10s] If a background tab requests HTTP auth the tab isn't switched to before displaying the auth dialog

Status: RESOLVED FIXED

(Toolkit :: Password Manager, defect)

Description

[Dave Townsend [:mossop]]

This is a means to spoof the user into giving the background tab their credentials for whatever the foreground tab is.

I think the bug lies here:

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/nsLoginManagerPrompter.js#581

In e10s we don't have the content window, this._window is just the main browser window.

Comment 1

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

I noticed this too, thanks for filing it.

I had assumed it was caused by issues with DOMWillOpenModalDialog not firing or not properly being propagated to the parent process, but looking into it just a bit further, http://hg.mozilla.org/mozilla-central/annotate/80e18ff7c7b2/toolkit/components/passwordmgr/LoginManagerParent.jsm#l210 looks pretty fishy.

Comment 2

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

I suppose the setE10sData call is already forwarding the browser over to the nsILoginManagerPrompter, but there's no nice way to forward that even further along to the underlying prompt service here:

http://hg.mozilla.org/mozilla-central/annotate/80e18ff7c7b2/toolkit/components/passwordmgr/nsLoginManagerPrompter.js#l367

Prompting APIs are all window based :(

Comment 3

[Dave Townsend [:mossop]]

Attachment: patch

This makes TabParent forward along the browser to the auth prompter. I'm not a fan of having to use the tabbrowser APIs to switch to the tab but can't think of an alternative right now.

I'm not positive that "*aResult = prompt.forget().take()" is the right way to return the prompter from C++ so let me know if there is a better way to do that.

Comment 4

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

(In reply to Dave Townsend [:mossop] from comment #3)
> I'm not a fan of having to use the tabbrowser APIs to switch to the tab but can't
> think of an alternative right now.

Adjust the promptservice API to allow passing in the opener somehow, and have it fire DOMWillOpenModalDialog properly? We'll need to do this eventually anyhow to fix a bunch of other prompts.

Comment 5

[Dave Townsend [:mossop]]

(In reply to :Gavin Sharp [email: [e-mail]] from comment #4)
> (In reply to Dave Townsend [:mossop] from comment #3)
> > I'm not a fan of having to use the tabbrowser APIs to switch to the tab but can't
> > think of an alternative right now.
> 
> Adjust the promptservice API to allow passing in the opener somehow, and
> have it fire DOMWillOpenModalDialog properly? We'll need to do this
> eventually anyhow to fix a bunch of other prompts.

Aha, I wasn't aware we had that. Looks like it takes a browser already so I can just use that.

Comment 6

[Dave Townsend [:mossop]]

Attachment: patch rev 2

Comment 7

[Dave Townsend [:mossop]]

Attachment: patch rev 3

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8516172 [details] [diff] [review]
patch rev 3

Review of attachment 8516172 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/ipc/TabParent.cpp
@@ +71,5 @@
>  #include "nsAuthInformationHolder.h"
>  #include "nsICancelable.h"
>  #include "gfxPrefs.h"
>  #include <algorithm>
> +#include "nsILoginManagerPrompter.h"

Nit: I believe the style guide asks us to keep #include ""s blocked separately from #include <>s.

@@ +1890,5 @@
> +    nsCOMPtr<nsIDOMElement> browser = do_QueryInterface(mFrameElement);
> +    prompter->SetE10sData(browser, nullptr);
> +  }
> +
> +  *aResult = prompt.forget().take();

prompt.forget(aResult);

Comment 9

[Dave Townsend [:mossop]]

(In reply to Blake Kaplan (:mrbkap) from comment #8)
> @@ +1890,5 @@
> > +    nsCOMPtr<nsIDOMElement> browser = do_QueryInterface(mFrameElement);
> > +    prompter->SetE10sData(browser, nullptr);
> > +  }
> > +
> > +  *aResult = prompt.forget().take();
> 
> prompt.forget(aResult);

I tried that before and it fails to compile with "error: assigning to 'void *' from incompatible type 'already_AddRefed<nsISupports>'"

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

Gah, sorry. Your original attempt was the right way to go.

Comment 11

[Dave Townsend [:mossop]]

https://hg.mozilla.org/integration/fx-team/rev/56019d2d1da2

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/56019d2d1da2