Closed
Bug 909022
Opened 11 years ago
Closed 11 years ago
Mark all executables as coming from the Internet zone on Windows
Categories
(Toolkit :: Downloads API, defect)
Toolkit
Downloads API
Tracking
()
RESOLVED
FIXED
mozilla26
Tracking | Status | |
---|---|---|
firefox26 | --- | fixed |
People
(Reporter: Paolo, Assigned: Paolo)
References
Details
(Keywords: sec-want)
Attachments
(1 file, 1 obsolete file)
7.42 KB,
patch
|
enndeakin
:
review+
|
Details | Diff | Splinter Review |
In the Downloads API, we should mark executables saved to an NTFS file system as
coming from the Internet zone, regardless of the currently applied Security Zone
Policy for the download source.
More information in the firefox-dev thread:
https://mail.mozilla.org/pipermail/firefox-dev/2013-August/000848.html
Assignee | ||
Comment 1•11 years ago
|
||
This is implemented on top of bug 853901, and is still missing the platform
conditionals, the executable check and the hidden preference to disable.
I tested this before editing the code a little, and it worked as expected.
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•11 years ago
|
||
Attachment #795059 -
Attachment is obsolete: true
Attachment #798603 -
Flags: review?(enndeakin)
Comment 3•11 years ago
|
||
Comment on attachment 798603 [details] [diff] [review]
The patch
> gDownloadPlatform.downloadDone(NetUtil.newURI(aDownload.source.url),
> new FileUtils.File(aDownload.target.path),
>- aDownload.contentType, aDownload.source.isPrivate);
>+ aDownload.contentType,
>+ aDownload.source.isPrivate);
> this.downloadDoneCalled = true;
>- return Promise.resolve();
>- } catch(ex) {
>- return Promise.reject(ex);
>- }
>+ }.bind(this));
> },
Do we need to report errors here if downloadDone fails?
>+#ifndef XP_WIN
>+ // Ask for confirmation if the file is executable, except on Windows where
>+ // the operating system will show the prompt based on the security zone.
>+ // We do this here, instead of letting the caller handle the prompt
>+ // separately in the user interface layer, for two reasons. The first is
>+ // because of its security nature, so that add-ons cannot forget to do
>+ // this check. The second is that the system-level security prompt would
>+ // be displayed at launch time in any case.
> if (file.isExecutable() && !this.dontOpenFileAndFolder) {
What happens if the saveZoneInformation preferences is false? Neither this block of code will run, nor the downloadDone code.
Assignee | ||
Comment 4•11 years ago
|
||
(In reply to Neil Deakin from comment #3)
> Do we need to report errors here if downloadDone fails?
No, those result in the rejection of the function and subsequent propagation to the caller of the "start" method.
> What happens if the saveZoneInformation preferences is false? Neither this
> block of code will run, nor the downloadDone code.
Yes, as noted in the about:config preference comment, disabling it means that we get no executable prompts at all on Windows. I think this is in line with the expectations of users toggling that preference.
Updated•11 years ago
|
Attachment #798603 -
Flags: review?(enndeakin) → review+
Assignee | ||
Comment 5•11 years ago
|
||
Comment 6•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Backed out in https://hg.mozilla.org/integration/fx-team/rev/ae42fd34f893 for causing lots of XPCShell failures like https://tbpl.mozilla.org/php/getParsedLog.php?id=27601606&tree=Fx-Team
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 8•11 years ago
|
||
Re-landed separately from bug 908240, as not related to the test failure:
https://hg.mozilla.org/integration/fx-team/rev/94e489c3dc98
Comment 9•11 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Comment 10•11 years ago
|
||
Hi,
I am sorry to tell but your assumption that you have to mark a file downloaded from the internet so that it get scanned is wrong.
Yes, there is a ScanWithAntiVirus option. Per default, this option is disabled. Currently, only Microsoft's own scanner will turn this option on (programs like Symantec Anti-Virus are causing problem with Mozilla Firefox, when this option is enabled, please try it, you won't be able to download *any* file, because your downloaded file will be deleted immediately when download the download has finished on a system with Norton Antivirus 2013/2014 when ScanWithAntiVirus is set).
But, the security feature your are talking about is not relying on the zone information. The zone information are just another protection layer for explorer, to warn the user when she wants to execute a downloaded file. Nothing more.
See http://support.microsoft.com/kb/883260/en
So this "feature" is causing a regression on bug 760889. When the administrator has set
[HKEY_CURRENT_USER\Software\Microsoft\Windows\CurrentVersion\Policies\Attachments]
"SaveZoneInformation"=dword:00000001
because he don't want zone information, you are now ignoring his wishes.
Thanks :)
Assignee | ||
Comment 11•11 years ago
|
||
Hi Igor, as you've correctly noticed, we don't use the system-level configuration to determine whether to set zone information anymore. The reason is actually related to performance, code complexity, and the APIs provided by Windows, rather than the virus scanning itself (even if there is a correlation between them). Since the issue is complex, if you're curious feel free to check out the mailing list thread referenced in comment 0 for more details.
The system-level configuration that Windows uses includes the registry value you mentioned, as well as several other options. We think that we cannot easily interpret all the possible configuration changes made by system administrators in the registry to determine if we should set zone information or not.
Thus, we have implemented a simpler "about:config" preference to toggle this behavior:
"browser.download.saveZoneInformation"
If set to false, zone information is not saved anymore, covering this use case we know may exist.
Comment 12•11 years ago
|
||
Well, I am not sure if it is the right way to move away from system-level configuration. In a corporate environment it becomes hard to manage Firefox installations (maybe there are new tools I am not aware of). And Google Chrome adds more and more features like that. But this is another discussion.
Anyway, thanks for the explanation. That was more than I expected. As a desktop user I can live with the the new option.
![]() |
||
Comment 13•11 years ago
|
||
This bug doesn't show up in the extended buglist linked in 26's release-notes.
status-firefox26:
--- → ?
Updated•11 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•