1417779 - Open with application triggers when showing new tab page

Status: NEW

(Firefox :: New Tab Page, defect, P4)

Description

[Coenraad]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:56.0) Gecko/20100101 Firefox/56.0
Build ID: 20170928135628

Steps to reproduce:

I recently installed telegram to open up a join group link that redirected to a tg:// url. This URL now comes up whenever I type tg as part of any other url in the location bar, with an immediate prompt if I want to open it in the application.


Actual results:

The "launch application" dialog pops up, interrupting entry of the url.

If the prompt is disabled, this could potentially be used to launch any application - if it is registered under, say a:// or b:// - without the user's knowledge. 


Expected results:

Application url's should not be autocompleted - or at the very least, should not fire up the application launch dialog unless the full url is confirmed.

Comment 1

[:Gijs (he/him)]

Marco, any idea what's going on here? Seems very odd...

Comment 2

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

My suspects are either speculative connection being too aggressive, or something related to an installed add-on using the omnibox API.

Could you please check if browser.urlbar.speculativeConnect.enabled is set to true? If it is, does changing it to false solve the problem? (don't recall if that needs a restart of Firefox, in case I'd suggest to restart)
Could you also please try disabling all of your add-ons (or using Safe Mode https://support.mozilla.org/kb/troubleshoot-firefox-issues-using-safe-mode)

Comment 3

[Coenraad]

browser.urlbar.speculativeConnect.enabled is false.

Still happens in safe mode.

Comment 4

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

This is what I could reproduce on 57:
1. install Telegram Desktop on Ubuntu
2. create a group
3. paste&go the link it generates (https://t.me/...) into Firefox Address Bar
4. select "Open with Telegram", but don't check the "remember" checkbox (In about:preferenced, under General / Applications I see the tg protocol marked as "Ask Every Time".)
5. Star the page
6. At this point closing and reopening Firefox shows that bookmark into the AS page, it tries to take a screenshot and the Open App dialog opens.

This is all I could reproduce, I couldn't reproduce the bug from the Address Bar. I wonder if your case was just a coincidence, maybe AS or something else was taking a screenshot while you were typing.

Do you have browser.ctrlTab.previews set to True?
The old NewTab page was also taking screenshots iirc, though I don't recall how to disable those to verify on your side, pinging Mardak.

If you can constantly reproduce this EVERY time you type "tg" in the Address Bar, then I need some better steps to reproduce the problem, possibly in a clean profile, because I can't think of any other stuff out of speculative connections (but those are only for certain entries and actions, should not really act here).

Comment 5

[Coenraad]

Mystery solved: While trying to make a video to show the problem, I found the cause. It's not the address bar, that was a coincidence - it's the shortcut screen. Sometimes the tg:// link ends up in the new tab window and sometimes not, there's a slight delay when it pops up.

(I didn't say anything about a screenshot :-)

Comment 6

[Coenraad]

Shortcut screen = new tab page. It hasn't happened since I removed the preview block, which I'm pretty sure linked to https://t.me/joinchat/(random code) 

It could also be triggered by the renderer trying to render the preview picture.

Comment 7

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Yes, by screenshot I meant the preview. To show a previews it has to load the page in background and take a screenshot of it.

Comment 8

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

[Tracking Requested - why for this release]: if a protocol is associated to always be opened with an app, a page could be able to open that app just by being part of the new tab page previews list.
A fix could be worth considering for uplift.

Comment 9

[Daniel Veditz [:dveditz]]

This sounds broken, but it doesn't need to be hidden as a security bug (not a remote attack, more of a local problem).

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

Tracking for 59 assuming it's also affected. We could potentially take an uplift for 58.

Comment 11

[Ed Lee :Mardak]

Activity Stream should only be trying to show http/s bookmarks, so I'm assuming the https page is redirecting to a different protocol or triggering a load of the other protocol? BackgroundPageThumbs does indeed allow for redirects and I can trigger the application prompt:

Cu.import("resource://gre/modules/BackgroundPageThumbs.jsm"); BackgroundPageThumbs.capture(`data:text/html,<meta http-equiv="refresh" content="1;magnet://" />`)

Maybe some more flags should be set here to prevent redirects to other protocols somehow?

https://searchfox.org/mozilla-central/source/toolkit/components/thumbnails/content/backgroundPageThumbsContent.js#42-50

Or I suppose it's already getting onStateChange / webProgress, so cancel the request ?

Comment 12

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

(In reply to Ed Lee :Mardak from comment #11)
> Activity Stream should only be trying to show http/s bookmarks, so I'm
> assuming the https page is redirecting to a different protocol or triggering
> a load of the other protocol?

Yes, the page after load tries to open the other protocol.
I didn't check the page code, so I can't tell if it's a redirect or js inited load, it's easy to reproduce just by starting from the link telegram gives you in the chat info.

Unfortunately there is no component for pagethumbs, and we risk to lose view of this bug in toolkit/general.
Most of the thumbs bugs so far were handled in NewTab.
Panos, how/where may we track this better?

Comment 13

[Panos Astithas (he/him) [:past] (please ni?)]

(Oops, fat-fingered the comment, sorry!)
I'm not concerned as much with which component this ends up in as with who is going to work on it. Since this is a tracked bug we should get somebody to investigate. Tim can someone in the AS team own this?

Comment 14

[Tim Spurway [:tspurway]]

Ursula, could you take a look at this one?

Comment 15

[Ursula Sarracini (:ursula)]

Yep, I'll take a look at this.

Comment 17

[:Gijs (he/him)]

So this was tracked for two releases but wontfixed for both... I'm a bit concerned about this considering that for permitted external protocols (esp. mailto!) this just straight up opens external applications.

I tried to look into this a bit today. It seems this is trickier than expected via the webprogress listeners. They don't seem to be called for loads that don't actually happen in that docshell (ie external protocol loads), which kind of makes sense but doesn't help us here.

I couldn't see a sandboxing flag that does what we want here, though that would in principle seem desirable. Sandboxing Automatic Features works: https://dxr.mozilla.org/mozilla-central/rev/5faab9e619901b1513fd4ca137747231be550def/dom/base/nsSandboxFlags.h#77 but that would presumably block all meta redirects. I don't know that that is what we want, and it might not work with 302 requests (I haven't tested this).

Really, what we want is a way to tell the docshell: don't load external protocols. If that doesn't exist, "don't load external protocols without user interaction" would do.

I'm not aware of such a thing existing, but I don't know. Christoph or Olli, do you have advice ?

Comment 18

[Christoph Kerschbaumer [:ckerschb]]

(In reply to :Gijs (lower availability until Jan 27) from comment #17)
> I'm not aware of such a thing existing, but I don't know. Christoph or Olli,
> do you have advice ?

I don't think that we have a "don't load external protocols" flag for docshell, but I think we could extend the load type enums [1] and pass that information to the docshell. E.g. we could add:
  const long loadNormalBlockExternalProtocols = ...;

and then within docshell we can check for that flag. I think this approach would do the trick, but then again I am not a docshell peer. Olli is probably in a better position to answer here.


[1] https://dxr.mozilla.org/mozilla-central/source/docshell/base/nsIDocShellLoadInfo.idl#80

Comment 19

[bob]

Thanks for upping the priority.

Have you managed to track this bug down sufficiently to give user advice on how to prevent it happening? I'm willing to help narrow it down in whatever way you ask.

Even tho I have configured FF to open a clean new tab page, I'm still intermittently getting new occurrences of this bug. You will see from my postings under bug 1386288, that I have scoured my config, my history, etc. for occurrences of each offending URL, and it is not clear (to me) where FF is finding the URL that it is opening.

I'm not sure why dveditz thinks this is not a potential remote attack (see my posting about security under bug 1386288#c2).

In the case where this bug opens the zoom app, not only is FF opening a local external application without user interaction, but that application is then sending an email to the host of the (long finished) meeting warning them that someone is waiting in a conference room for the host to join. The hosts of various meetings are getting pretty aggrieved about all these emails (often at odd hours in different time-zones), given how often I open new FF tabs.

Comment 20

[Olli Pettay [:smaug][[e-mail]]]

I'd rather add some explicit flag on docshell to stop loading external protocols.
nsIDocShellLoadInfo is such a messy list, and I assume here we won't be loading any normal tabs to the docshell we use for the preview.

Comment 21

[Ursula Sarracini (:ursula)]

Attachment: Bug 1417779 - Open with application triggers when showing new tab page

Review commit: https://reviewboard.mozilla.org/r/220214/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/220214/

Comment 22

[Ursula Sarracini (:ursula)]

Not sure if I should be canceling to requesting and setting AbortProcess = true here or if returning NS_ERROR_ABORT is good enough?

Comment 23

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8950954 [details]
Bug 1417779 - Open with application triggers when showing new tab page

https://reviewboard.mozilla.org/r/220214/#review226086

::: docshell/base/nsDSURIContentListener.cpp:147
(Diff revision 1)
>    NS_ENSURE_TRUE(mDocShell, NS_ERROR_FAILURE);
>  
>    *aAbortProcess = false;
>  
> +  // do not allow loads from external protocols if specified by the docshell
> +  bool allowExternalProtocols = true;

This doesn't look right place to add this code.
This basically prevents all loads.

::: docshell/base/nsDSURIContentListener.cpp:221
(Diff revision 1)
>      *aAbortProcess = true;
>      return NS_OK;
>    }
>  
>    if (NS_FAILED(rv)) {
>      // we don't know how to handle the content

I would expect the check somewhere here, or perhaps in the caller of DoContent, since this doesn't get called always, it seems.
nsDocumentOpenInfo::DispatchContent might be high up enough in the stack. See the helper app service usage there etc.

Comment 24

[Coenraad]

There's another way to fix this and that is not adding anything with an external protocol handler to the shortcut page in the first place, or even better - removing any such links whenever Firefox is run, or before the new tab page is displayed.  

Someone looking for hooks to execute their malicious code from could just search bugzilla for "open with" or "protocol handler" (or now "malicious") - and find this - yet another hook in addition to startup, explorer- and autorun hooks, (this is even cross platform)... where they can trivially register protocol handlers and add shortcut page links... so that even if you remove all links to malware from all the well known places, whenever you show a new tab page in Firefox the deeper level hidden malicious code again invisibly executes and re-runs itself... New tab page gets shown a few times every time you run Firefox, so its a significant vector, that has been around for a few months now...

Comment 25

[:Gijs (he/him)]

(In reply to Coenraad from comment #24)
> There's another way to fix this and that is not adding anything with an
> external protocol handler to the shortcut page in the first place, or even
> better - removing any such links whenever Firefox is run, or before the new
> tab page is displayed.  

We're already not adding anything with an external protocol handler. We're adding an http(s) link, that then redirects (e.g. via <meta> tag, or JS, or ...). So we need to prevent the thumbnailer from following the redirect to an external protocol handler page...

Comment 26

[Sherry Nichols]

Attachment: same problem

Thanks from bogmolena1988.