581762 - Malware blocked site: Camino should prevent downloading of files from the location bar

Status: RESOLVED FIXED

(Camino Graveyard :: Security, defect)

Description

[philippe (part-time)]

Basic STR:
1. paste a url for a file hosted on a malware blocked site in the location bar
2. Option-return to save/download the file locally

AR: the save file dialog appears, the file is saved
ER: the file is not saved.

In this case Safari prevents saving the file

real world example:
At time of writing, http://destination-out.com/ is blocked as malware (probably hacked WordPress). That site (legally) hosts mp3 files. A direct URL for one:
http://destination-out.com/media/tracks/Dixon_Solo-8.mp3 (clean file, afaict).
Loading this URL results in the malware overlay, but option-return in the location bar allows downloading the file.
(note that the files on that site are only available for a limited time, in theory)

That is the most straight forward scenario I see:
1. user follows link to mp3
2. link target (site) is blocked.
3. user tries opt-return in location bar to save file locally

----
Safari, Camino and Firefox allow downloading the file from a link by right-clicking on the link and choosing 'Download linked file'/'Save link target as'/… or option-clicking the link. Google Chrome (tested: dev channel) prevents saving the link target in this case.

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Ought to be pretty straightforward to fix the opt-return stuff; we just have to have cl's opt-return code (http://mxr.mozilla.org/camino/source/camino/src/browser/BrowserWindowController.mm#355) use hendy's menu disabling code (bug 501246), or, rather, the (![[self browserWrapper] isBlockedErrorOverlayShowing]) part, to check.

Comment 2

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: Fix

Like this, actually.

Comment 3

[philippe (part-time)]

Comment on attachment 460109 [details] [diff] [review]
Fix

Does what I expected: opt-return in the location bar is disabled.

Comment 4

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

And I guess if this does the right thing, we should take it in 2.0.x, too.

And, another too-late-night thought: is there a better way to write this that
doesn't require me to re-indent all of cl's code?  I apparently can't "return"
inside a (void), so says the compiler--or I was trying to return the wrong way
when I tried that initially.

Comment 5

[philippe (part-time)]

(In reply to comment #3)
> Comment on attachment 460109 [details] [diff] [review]
> Fix
> 
> Does what I expected: opt-return in the location bar is disabled.

via irc:
[3:59pm] ardissone: phiw: did you check to see if it does the right thing after clicking through the warning?
[3:59pm] ardissone: just thought of that
[3:59pm] ardissone: (both for a binary and a webpage)

yes, after clicking through the warning, saving via opt-return works perfectly fine

Comment 6

[Stuart Morgan]

Comment on attachment 460109 [details] [diff] [review]
Fix

Yes, you just want:

if ([[bwc browserWrapper] isBlockedErrorOverlayShowing])
  return;

at the beginning.

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: Fix, v2

Yes, that seems much more pleasant.

Comment 8

[Stuart Morgan]

Comment on attachment 460177 [details] [diff] [review]
Fix, v2

sr=smorgan

Do we want to leave this open to handle this in a robust way? This doesn't cover the case of skipping to step 3.

Comment 9

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

> Do we want to leave this open to handle this in a robust way? This doesn't
> cover the case of skipping to step 3.

I was leaning more towards filing a new bug, because it seems to me the way to fix that is to have the Save code itself run a check (or check flags, or whatever) on the URL before saving, and that covers the context menu "Download link target" and opt-click the link cases as well--and it's probably not something I can fix.

If you'd prefer to leave this open, that's fine by me, too, but the above is why I was leaning towards filing a new bug for fixing the Save code itself.

Comment 10

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

http://hg.mozilla.org/camino/rev/2e0518cb2180 and CAMINO_2_0_BRANCH.

Filed bug 581885 for the additional work.