Closed Bug 1145541 Opened 5 years ago Closed 4 years ago

Unauthorized redirection while opening tabs in Firefox Hello

Categories

(Hello (Loop) :: Client, defect, P4)

defect
Points:
2

Tracking

(firefox38 fixed, firefox38.0.5 fixed, firefox39 fixed, firefox40 fixed)

RESOLVED FIXED
mozilla40
Iteration:
40.2 - 27 Apr
Tracking Status
firefox38 --- fixed
firefox38.0.5 --- fixed
firefox39 --- fixed
firefox40 --- fixed

People

(Reporter: shahmeerbond, Assigned: standard8)

Details

(Keywords: sec-low)

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/40.0.2214.115 Safari/537.36 OPR/27.0.1689.76

Steps to reproduce:

Hello there firefox team
I found out an issue in the firefox hello 
When you open a link in a new tab ( target="_blank" ), the page that opens in a new tab can access the initial tab and change it's location using the window.opener property.
Paste the link in firefox hello to give it to the victim
http://daniel-tomescu.com/hackerone/landpage.php
Don't right-click it and open it in new tab, don't use the mouse wheel to open it, don't Ctrl+Click, just do a normal click on the link.


Actual results:

When the victim visits the link he will be redirected in a new tab
But at the same time in his initial tab he will redirected to a scampage


Expected results:

The Redirection in the initial tab should not have occured, So using this vulnerability an attacker can redirect anyone to any page in the initial tab
Flags: needinfo?(standard8)
The javascript code that does all the magic:
window.opener.location.replace(newURL);

Ways to solve this:

Don't open links from Hello application in new tabs using the target="_blank";
Set the window.opener attribute to null on the new tab before redirecting to the landing page, like this: a) open in a new tab  b) use a javascript code: <script>window.opener = null; </script> c) redirect to the landing page: <script>window.location.reload(landingpage)</script>
I hope you see why this is dangerous: this method has huge potential for tricking Hello that click on external links from this site to be a victim of a scam page because the redirecting is made in the background, while the user is focused on another tab.

Shall i email you on the address?
(In reply to Muhammad Shahmeer from comment #1)
> Shall i email you on the address?

There is no need to email, the bugreport is enough. People are looking into it.
Ok, we managed to reproduce this. Slightly more detail on the steps to reproduce:

1) Start Firefox, Open the Hello panel, skip the Tour
2) Go back into the Panel, and click the cog icon, click Help

=> New tab opens

Then either:

3a) Open the browser console, enter:

window.opener.location.href = "google.com"

3b) Copy the url http://daniel-tomescu.com/hackerone/landpage.php and paste it into the url bar

4) Re-open the Hello panel

Actual Results:

The Hello panel has changed to the page requested

Expected Results: 

The Hello panel didn't change.
Status: UNCONFIRMED → NEW
Component: Untriaged → Client
Ever confirmed: true
Flags: needinfo?(standard8)
OS: Windows 8.1 → All
Product: Firefox → Loop
Hardware: x86_64 → All
Version: Firefox 38 → unspecified
Marking this as sec-low - it requires manual and specific user interaction to get load the attacking page at the right time. It also only replaces the panel content - there's no access to the Hello data.

https://wiki.mozilla.org/Security_Severity_Ratings
Flags: firefox-backlog+
Keywords: sec-low
Priority: -- → P1
But the  vulnerability still does exists and can be used to redirect users. This is nowhere near low sec. Does this get me a reward. I am ready to explain more
(In reply to Muhammad Shahmeer from comment #5)
> But the  vulnerability still does exists and can be used to redirect users.
> This is nowhere near low sec.

I didn't say it doesn't exist. It is just a lower risk. The main reason being is that it requires a specific set of actions that could be classed as not "normal browsing actions". At best, I think this puts it at sec-moderate.

However, there's also no direct access to user data, which points more to sec-low. Yes, if the user gets spoofed then it could be bad for the user, but its a difficult route to get the user into that state.

This is the assessment of a colleague and myself, but I'll ask one of the main security team for feedback on this assessment in case we've missed something.

> Does this get me a reward. I am ready to explain more

I'm not up to date with the current rules, hopefully the security team can answer.
Flags: needinfo?(dveditz)
Can we workaround this by specifying the hello page in the panel / browser as sandboxed if we don't already?
(In reply to :Gijs Kruitbosch from comment #7)
> Can we workaround this by specifying the hello page in the panel / browser
> as sandboxed if we don't already?

There's a privileged API that we have that we can extend to open a new tab (or whatever), that should get us to do the right thing. I suspect Mike or myself can pick this up early next week.
(In reply to Mark Banner (:standard8) from comment #8)
> (In reply to :Gijs Kruitbosch from comment #7)
> > Can we workaround this by specifying the hello page in the panel / browser
> > as sandboxed if we don't already?
> 
> There's a privileged API that we have that we can extend to open a new tab
> (or whatever), that should get us to do the right thing. I suspect Mike or
> myself can pick this up early next week.

I meant using the sandbox CSP directive... but it looks like that's bug 671389 and it's stalled.
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
Iteration: --- → 39.3 - 30 Mar
Points: --- → 2
Flags: sec-bounty?
Assignee: mdeboer → nobody
Status: ASSIGNED → NEW
Iteration: 39.3 - 30 Mar → ---
As a low rated security issue, this is not eligible for a bounty.
Flags: sec-bounty? → sec-bounty-
Priority: P1 → P4
So this is eligible for a bounty?
(In reply to Muhammad Shahmeer from comment #14)
> So this is eligible for a bounty?

See comment 13. This question was already answered.
Assignee: nobody → standard8
Iteration: --- → 40.2 - 27 Apr
Attached patch The fixSplinter Review
This fixes us so that we use the backend rather than window.open. This avoids the exploit. I've tested the patch against the given exploit and it all works fine now - the panel stays the same.
Attachment #8595328 - Flags: review?(mdeboer)
Comment on attachment 8595328 [details] [diff] [review]
The fix

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

Nice work, thanks!

::: browser/components/loop/MozLoopAPI.jsm
@@ +709,5 @@
>  
>      /**
> +     * Opens a URL in a new tab in the browser.
> +     *
> +     * @param {String} [url] The new url to open

I don't think a url should be 'optional' here ;)

::: browser/components/loop/MozLoopService.jsm
@@ +1652,5 @@
>  
>    /**
> +   * Opens a URL in a new tab in the browser.
> +   *
> +   * @param {String} [url] The new url to open

Optional?

@@ +1656,5 @@
> +   * @param {String} [url] The new url to open
> +   */
> +  openURL: function(url) {
> +    let win = Services.wm.getMostRecentWindow("navigator:browser");
> +    MozLoopService.log.warn("opening via backend", url);

Hmm, I'm quite sure that this is 'debug' level info at most.

::: browser/components/loop/content/js/panel.jsx
@@ +342,5 @@
>      },
>  
>      handleHelpEntry: function(event) {
>        event.preventDefault();
> +      var helloSupportUrl = this.props.mozLoop.getLoopPref('support_url');

While you're here, can you change this to double quotes?

::: browser/components/loop/test/desktop-local/panel_test.js
@@ +199,5 @@
> +          }));
> +
> +        expect(view.getDOMNode().querySelectorAll(".icon-account"))
> +          .to.have.length.of(0);
> +      });

nit: indentation
Attachment #8595328 - Flags: review?(mdeboer) → review+
(In reply to Mike de Boer [:mikedeboer] from comment #17)
> @@ +1656,5 @@
> > +    MozLoopService.log.warn("opening via backend", url);
> 
> Hmm, I'm quite sure that this is 'debug' level info at most.

That's an oops I left it in ;-)
http://hg.mozilla.org/mozilla-central/rev/b75551ab6fca
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Comment on attachment 8595328 [details] [diff] [review]
The fix

Approval Request Comment
[Feature/regressing bug #]: Firefox Hello
[User impact if declined]: Low risk of security issue via social engineering attack
[Describe test coverage new/current, TreeHerder]: Landed in m-c, manual testing of links performed, some automated checks for unit test functions where possible.
[Risks and why]: Low - only changing the way links are opened.
[String/UUID change made/needed]: None
Attachment #8595328 - Flags: approval-mozilla-beta?
Attachment #8595328 - Flags: approval-mozilla-aurora?
Comment on attachment 8595328 [details] [diff] [review]
The fix

[Triage Comment]
38.0 is in m-r now
should be in 38 beta 7.
Attachment #8595328 - Flags: approval-mozilla-release+
Attachment #8595328 - Flags: approval-mozilla-beta?
Attachment #8595328 - Flags: approval-mozilla-aurora?
Attachment #8595328 - Flags: approval-mozilla-aurora+
Why are we approving this on release but not beta?
(In reply to Al Billings [:abillings] from comment #23)
> Why are we approving this on release but not beta?

Because release will merge to beta. Long story. release will become 38, beta will become 38.0.5.
Can this be a CVE or a bounty>
(In reply to Muhammad Shahmeer from comment #25)
> Can this be a CVE or a bounty>

This was minused for bounty (see sec-bounty flag) because it is rated "sec-low" and is not much of a security issue. We generally pay bounties only for sec-high and sec-critical rated issues. See https://www.mozilla.org/en-US/security/bug-bounty/faq/#eligible-bugs for details.
https://hg.mozilla.org/releases/mozilla-aurora/rev/532ce9f42266

(In reply to Al Billings [:abillings] from comment #23)
> Why are we approving this on release but not beta?

Don't worry, it'll get everywhere it needs to go :)
Flags: in-testsuite+
Also, this is the third time you've asked about a bounty and been told that it is not eligible. Please don't keep asking here. If you have further questions, email security@mozilla.org.

This issue will be assigned a CVE by the time it ships in a version of Firefox.
Okay Sorry
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.