1329868 - Ability to block opening of certain urls with command line

Status: NEW

(Firefox :: Distributions, defect, P3)

Description

[Hector Zhao [:hectorz]]

3rd party software often trys to hijack the start page of Firefox (at least here in China).

One popular way of doing this is the appending of url in Windows desktop shortcut to Firefox, and some Chinese hijackers even found a way to do this without actually modifying the shortcut (bug 953139).

In China repack, Beijing office defends against this practice by maintaining a list of known hijacking target urls and drop them with a command line validator.

One thing on our roadmap is to correlate the short urls to their 3xx redirect/meta refresh/JavaScript redirect targets, to reduce the size of our black list.

Comment 1

[Shane Caraveo (:mixedpuppy)]

Again, I think this isn't general enough for a webextension api.  We should use a bootstrap system addon for deeper changes needed in distributions.  This isn't to say all features should go that route, but some things are very specific to changes done by distributions.

Comment 2

[:shell escalante]

talking with China has as P1 for this extension... one case of the homepage hijacking.

Comment 3

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

This is an assigned P1 bug without activity in two weeks. 

If you intend to continue working on this bug for the current release/iteration/sprint, remove the 'stale-bug' keyword.

Otherwise we'll reset the priority of the bug back to '--' on Monday, August 28th.

Comment 4

[Mike Kaply [:mkaply]]

So after further discussion, we're thinking we should add a way for the Chinese build to simply turn off the loading of URLs from the command line. I'm investigating how feasible this is.

Comment 5

[Mike Kaply [:mkaply]]

For reference, this appears to be where the command line URLs are handled:

http://searchfox.org/mozilla-central/source/browser/components/nsBrowserContentHandler.js#712

Comment 6

[Mike Kaply [:mkaply]]

Attachment: Basic patch

This is a very simple patch to do this.

It reads a preference and if set, simply ignores command line passed URLs.

Comment 7

[Mike Kaply [:mkaply]]

Comment on attachment 8905265 [details] [diff] [review]
Basic patch

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

I honestly don't know why the right person is to review this.

Feel free to pass it on.

Comment 8

[:Gijs (he/him)]

Comment on attachment 8905265 [details] [diff] [review]
Basic patch

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

Please use mozreview in future, or if you absolutely must use splinter, attach a changeset rather than a diff (so with commit message etc.).

::: browser/components/nsBrowserContentHandler.js
@@ +744,5 @@
>          }
>        }
>      }
>  
> +    if (Services.prefs.getBoolPref("browser.ignore-commandline", false)) {

This will also break opening links from external programs, even when Firefox is already open. Is that really what we want for the repack? Break all external links into Firefox? Doesn't seem very good to me...

Comment 9

[Mike Kaply [:mkaply]]

> This will also break opening links from external programs, even when Firefox is already open. Is that really what we want for the repack? Break all external links into Firefox? Doesn't seem very good to me...

Plan for China was to tell users how to turn off the pref if there are complaints, but you have a really good point.

We can detect the separate cases. I'll create a better patch.

Are you OK with the general concept though?

Comment 10

[:Gijs (he/him)]

(In reply to Mike Kaply [:mkaply] from comment #9)
> > This will also break opening links from external programs, even when Firefox is already open. Is that really what we want for the repack? Break all external links into Firefox? Doesn't seem very good to me...
> 
> Plan for China was to tell users how to turn off the pref if there are
> complaints, but you have a really good point.
> 
> We can detect the separate cases. I'll create a better patch.
> 
> Are you OK with the general concept though?

I'm a little bit apprehensive, tbh, but if this is necessary it's necessary. Might be good to get Dave to sign off on this kind of thing. Dave? :-)

Comment 11

[Mike Kaply [:mkaply]]

Attachment: Bug 1329868 - Provide a pref to disable command line URLs.

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

Comment 12

[Dave Townsend [:mossop]]

Can we only do this if Firefox was launched from a shortcut? That would mean command line calls and URL requests from other apps should still work in theory. mhowell suggests that you can detect this by seeing if the process parent is explorer.exe. Otherwise I guess I don't have a problem with this being pref controlled.

Comment 13

[:Gijs (he/him)]

Comment on attachment 8905284 [details]
Bug 1329868 - Provide a pref to disable command line URLs.

https://reviewboard.mozilla.org/r/177080/#review182224

Can we do what Dave/Matt suggested in comment #12?

Clearing review for now...

Comment 14

[Mike Kaply [:mkaply]]

I looked into Matt's idea, and getting the parent process name isn't very straightforward on Windows and I think it would be a big piece of work.

I think I'd like to pursue getting the existing patch in as a stopgap for the China edition.

Comment 15

[:Gijs (he/him)]

(In reply to Mike Kaply [:mkaply] from comment #14)
> I looked into Matt's idea, and getting the parent process name isn't very
> straightforward on Windows and I think it would be a big piece of work.

I am very far from a win32 expert, but at a glance this approach looks like it would work: https://stackoverflow.com/questions/29939893/get-parent-process-name-windows (see also comments on the first answer)?

We could hang a method off nsIXULRuntime or something like that, which already has a processID (Services.appinfo.processID).

> I think I'd like to pursue getting the existing patch in as a stopgap for
> the China edition.

OK, in that case please request review from Mossop on the patch. :-)

Comment 16

[Mike Kaply [:mkaply]]

Clearing needinfo. Nothing moving here.