1433357 - Prevent proxy bypass via file dialog (Linux)

Status: RESOLVED FIXED

(Core :: Widget: Gtk, enhancement)

Description

[Arthur Edelstein [:arthur]]

On Linux, if the user enters a remote URL in the Open File Dialog, the proxy can be bypassed. In Tor Browser, we added the following patch:
https://torpat.ch/18101
and here's the original ticket:
https://trac.torproject.org/18101

We would like to propose uplifting this patch, perhaps behind a flag.

Comment 1

[Arthur Edelstein [:arthur]]

Attachment: 0001-Bug-1433357-Part-1-Add-proxy-bypass-protection-flag.patch

Comment 2

[Arthur Edelstein [:arthur]]

Attachment: 0002-Bug-1433357-Part-2-Suppress-upload-file-dialog-proxy.patch

Hi Mike and Karl, would you be able to review these? The first patch adds the general "--enable-proxy-bypass-protection" build flag, and the second patch is specifically for the linux file dialog.

Comment 3

[Mike Hommey [:glandium]]

Comment on attachment 8946325 [details] [diff] [review]
0001-Bug-1433357-Part-1-Add-proxy-bypass-protection-flag.patch

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

::: old-configure.in
@@ +4213,5 @@
>  dnl ========================================================
> +dnl enable proxy bypass protection
> +dnl ========================================================
> +MOZ_ARG_ENABLE_BOOL(proxy-bypass-protection,
> +[  --enable-proxy-bypass-protection      Enable proxy bypass protection],

It would be better to put all this in python configure. This should go in toolkit/moz.configure, and could look like:

option('--enable-proxy-bypass-protection', help='...')

@depends_if('--enable-proxy-bypass-protection')
def proxy_bypass_protection(_):
    return True

set_config('MOZ_PROXY_BYPASS_PROTECTION', proxy_bypass_protection)
set_define('MOZ_PROXY_BYPASS_PROTECTION', proxy_bypass_protection)

Comment 4

[Karl Tomlinson (:karlt)]

Comment on attachment 8946328 [details] [diff] [review]
0002-Bug-1433357-Part-2-Suppress-upload-file-dialog-proxy.patch

Please place the preprocessor directives and comment around the whole mAllowURLs test and block to indicate that the whole block becomes irrelevant.

Please include more context in future patches:
That can be done by including this in ~/.hgrc or .hg/hgrc:
[diff]
git = true
showfunc = 1
unified = 8

Comment 5

[Arthur Edelstein [:arthur]]

Attachment: 0001-Bug-1433357-Part-1-Add-proxy-bypass-protection-flag.patch

(In reply to Mike Hommey [:glandium] from comment #3)

Thanks for the review here's a new version with the code you proposed.

Comment 6

[Arthur Edelstein [:arthur]]

Attachment: 0002-Bug-1433357-Part-2-Suppress-upload-file-dialog-proxy.patch

(In reply to Karl Tomlinson (:karlt) from comment #4)

Thanks, Karl. Here's a new version with the changes you suggested.

Comment 7

[Arthur Edelstein [:arthur]]

Attachment: 0001-Bug-1433357-Part-2-Suppress-upload-file-dialog-proxy.patch

(Oops, trying again.)

Comment 8

[Arthur Edelstein [:arthur]]

Attachment: 0001-Bug-1433357-Part-1-Add-proxy-bypass-protection-flag.patch

(Replacing incorrect file.)

Comment 9

[Arthur Edelstein [:arthur]]

Thanks!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b5ad0c384202
Part 1: Add proxy bypass protection flag r=glandium
https://hg.mozilla.org/integration/mozilla-inbound/rev/bd385579481b
Part 2: Suppress upload-file-dialog proxy bypass in Linux r=karlt

Comment 11

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/b5ad0c384202
https://hg.mozilla.org/mozilla-central/rev/bd385579481b