When trying to load blocked (unwanted) software in a hidden frame, it looks like "nothing happens" because the software warning page loads in the frame.

NEW
Unassigned

Status

()

Toolkit
Safe Browsing
P3
normal
2 years ago
4 months ago

People

(Reporter: Pavel Zeldin, Unassigned)

Tracking

(Blocks: 1 bug)

40 Branch
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sb-backlog] [sb-moderate])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(4 attachments)

(Reporter)

Description

2 years ago
Created attachment 8648667 [details]
halog_jzip.halog

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.155 Safari/537.36

Steps to reproduce:

Hi guys,

this issue occurs with the latest Firefox version, 40.0.2, on some selected urls with redirected links.
save as link works flawlessly, but with direct mouse click, there's no redirect and no download.
in some cases, there's an empty redirect response.
By now we've found out the following details:
If the user updates or upgrades to FF version 40.0.2 (clean machine) he can successfully download the files from the sites mentioned above in a period of ~ 20 min.
After that period it is no longer possible to download the files.
Restarting PC has no effect.
link example:
http://download.jzip.com/jZipSetup.exe
when downloading from firefox directly, the issue occurs. on any other browser and with save as link, there's no problem.


Actual results:

nothing happened, the get results in a 302, and nothing downloaded.


Expected results:

there should be a redirect to the corresponding file.
(Reporter)

Comment 1

2 years ago
Created attachment 8648668 [details]
koyotesoft_empty.halog
(Reporter)

Updated

2 years ago
OS: Unspecified → All
Hardware: Unspecified → All

Updated

2 years ago
Component: Untriaged → Networking: HTTP
Product: Firefox → Core

Comment 2

2 years ago
Your .exe link returns "Reported Unwanted Software Page!"

Comment 3

2 years ago
(In reply to Loic from comment #2)
> Your .exe link returns "Reported Unwanted Software Page!"

Pavel, is this what you mean?
Blocks: 1136055
Flags: needinfo?(pavel)
(Reporter)

Comment 4

2 years ago
there was no response after the initial redirect, the browser simply dropped the connection, and did not provided any information or notification of the "Unwanted Software Page!".
as on direct access to the file, i think that the reported unwanted page should be shown to the end user, and let him choose if he's interested in continuing or not, just like in any other browser.
Flags: needinfo?(pavel)

Comment 5

2 years ago
(In reply to Pavel Zeldin from comment #4)
> there was no response after the initial redirect, the browser simply dropped
> the connection, and did not provided any information or notification of the
> "Unwanted Software Page!".
> as on direct access to the file, i think that the reported unwanted page
> should be shown to the end user, and let him choose if he's interested in
> continuing or not, just like in any other browser.

I also get the unwanted software warning. I don't really understand why you wouldn't, unless you've turned off safebrowsing (in the options/preferences, under "security", check for "block reported attack sites" and "block reported web forgeries".

As it is, that means we can't really reproduce the issue you seem to be seeing. Can you clarify further? Should clicking that link from this bugzilla page be enough to reproduce it? Can you try a clean Firefox profile? ( https://support.mozilla.org/kb/profile-manager-create-and-remove-firefox-profiles )
Safebrowsing should be on by default on any new Firefox profile.
Flags: needinfo?(pavel)
(Reporter)

Comment 6

2 years ago
when accessing directly to the file - from here - http://download.jzip.com/jZipSetup.exe, i do get the unwanted application.
however, when i click on this page - http://www.jzip.com/, on the Get Jzip, there is no unwanted app page, nothing happens.
Flags: needinfo?(pavel)

Comment 7

2 years ago
I can reproduce this on current beta. Not sure what's going on. Dragana and/or Christoph, can you take a look? Devtools don't even show any response, so it seems like something in networking is getting confused for some reason...
Flags: needinfo?(mozilla)
Flags: needinfo?(dd.mozilla)
So I had a look into this.
So from networking part everything is working correctly. Both do the same thing. The problem is the download gets blocked but there is no page to show the error, or something goes wrong there.
If you take the site that is not working and open download in separate tab (right -> Open in new tab) it is working properly.

When you click it tries to download from http://download.jzip.com/jZipSetup.exe but the tab stays on http://www.jzip.com/ and instead of showing error on that tab it thinks there is no tab to show the error... but I do not know this code ... 

So from log I see that the error page is constructed and when it should be shown the log shows:

2015-08-25 09:49:40.351392 UTC - 1577318976[130f1c0]: Shown Window: about:blocked?e=unwantedBlocked&u=http%3A//download.jzip.com/jZipSetup.exe&s=blacklist&c=UTF-8&f=regular&d=The%20site%20at%20download.jzip.com%20has%20been%20reported%20as%20serving%20unwanted%20software%20and%20has%20been%20blocked%20based%20on%20your%20security%20preferences.
2015-08-25 09:49:40.351397 UTC - 1577318976[130f1c0]:  Focused Window: http://www.jzip.com/

....
2015-08-25 09:49:44.010908 UTC - 1577318976[130f1c0]: <<Blur begin>>
2015-08-25 09:49:44.010913 UTC - 1577318976[130f1c0]: ISM: IMEStateManager::OnChangeFocus(aPresContext=0x0, aContent=0x0, aCause=CAUSE_UNKNOWN)
2015-08-25 09:49:44.010917 UTC - 1577318976[130f1c0]: ISM: IMEStateManager::OnChangeFocusInternal(aPresContext=0x0, aContent=0x0 (TabParent=0x0), aAction={ mCause=CAUSE_UNKNOWN, mFocusChange=FOCUS_NOT_CHANGED }), sPresContext=0x1f08e20, sContent=0x1fc0c30, sActiveTabParent=0x0, sActiveIMEContentObserver=0x0, sInstalledMenuKeyboardListener=false
2015-08-25 09:49:44.010922 UTC - 1577318976[130f1c0]: ISM:   IMEStateManager::OnChangeFocusInternal(), no nsPresContext is being activated
2015-08-25 09:49:44.010927 UTC - 1577318976[130f1c0]: Element a has been blurred



The working version write in log:
2015-08-25 09:52:07.223703 UTC - -1061078464[a2f1c0]: Shown Window: about:blocked?e=unwantedBlocked&u=http%3A//download.jzip.com/jZipSetup.ex&s=blacklist&c=UTF-8&f=regular&d=The%20site%20at%20download.jzip.com%20has%20been%20reported%20as%20serving%20unwanted%20software%20and%20has%20been%20blocked%20based%20on%20your%20security%20preferences.
2015-08-25 09:52:07.223709 UTC - -1061078464[a2f1c0]:  Focused Window: about:blocked?e=unwantedBlocked&u=http%3A//download.jzip.com/jZipSetup.ex&s=blacklist&c=UTF-8&f=regular&d=The%20site%20at%20download.jzip.com%20has%20been%20reported%20as%20serving%20unwanted%20software%20and%20has%20been%20blocked%20based%20on%20your%20security%20preferences.
2015-08-25 09:52:07.223831 UTC - -1061078464[a2f1c0]: <<Focus begin>>


I do not know this code to interpret the log.
Flags: needinfo?(dd.mozilla)
Component: Networking: HTTP → Document Navigation

Comment 9

2 years ago
I tried to run mozregression on this but I can't get it to break at all because I don't have the safebrowsing db on the clean profile that mozregression uses. Anyway, I presume this is a regression. Christoph, if you have additional insight here that'd still be really useful.

Pavel, do you know if this worked in earlier releases, and if so in which one(s) ?

I should note that downloading that file sets off Windows Defender on my windows machine. :-\
Flags: needinfo?(pavel)
Keywords: regression, regressionwindow-wanted
(In reply to :Gijs Kruitbosch from comment #9)
> I tried to run mozregression on this but I can't get it to break at all
> because I don't have the safebrowsing db on the clean profile that
> mozregression uses. Anyway, I presume this is a regression. Christoph, if
> you have additional insight here that'd still be really useful.

It sounds very similar to Bug 1136055 which we recently fixed. I also find that change uplifted to the current release [1], so we must go through a different code path/use a different channel and hence a different loadInfo. Unfortunately I don't have time to look into this problem today, but I can look into it later this week. Most likely it's a very similar issue to Bug 1136055, where mixed content after redirect blocks the request. (Somewhere around here [2]).

My guess is that the principal in the loadInfo is not the systemPrincipal and the loadingNode in the loadInfo is null, hence mixedContentBlocker bails because it can not query the root docshell and blocks the load.


[1] http://mxr.mozilla.org/mozilla-release/source/browser/base/content/nsContextMenu.js#1297
[2] http://mxr.mozilla.org/mozilla-central/source/dom/security/nsMixedContentBlocker.cpp#244
[3] http://mxr.mozilla.org/mozilla-central/source/dom/security/nsMixedContentBlocker.cpp#625
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #10)
> My guess is that the principal in the loadInfo is not the systemPrincipal
> and the loadingNode in the loadInfo is null, hence mixedContentBlocker bails
> because it can not query the root docshell and blocks the load.

See [3].

> [3]
> http://mxr.mozilla.org/mozilla-central/source/dom/security/
> nsMixedContentBlocker.cpp#625
Flags: needinfo?(mozilla)

Comment 12

2 years ago
My regression range:
good=2015-04-22
bad=2015-04-23
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=946ac85af8f4&tochange=0b202671c9e2
Looks like this regression range is where unwanted software support was added.

8b191f5f9687	Francois Marier — Bug 1147212 - Add support for goog-unwanted-shavar. r=gcp,r=matej,r=smaug

Adding Francois.

Updated

2 years ago
Blocks: 1147212
Keywords: regressionwindow-wanted

Comment 14

2 years ago
(In reply to Christoph Kerschbaumer [:ckerschb] from comment #10)
> My guess is that the principal in the loadInfo is not the systemPrincipal
> and the loadingNode in the loadInfo is null,

This doesn't make sense to me, because the loadingNode is surely the linking <a> element or whatever?

> hence mixedContentBlocker bails
> because it can not query the root docshell and blocks the load.

I would have assumed that a network error page was always loadable, mixed content blocking or no. And either way, both source and destination in this case are on http://, not https, so I don't see why this breaks... The only thing special about the setup here seems to be the fact that the load is targeted to an iframe (#downframe). There's also an onclick attribute, but removing that doesn't make a difference. Removing the target attribute from the link to avoid loading this in the iframe makes the page reach the expected error page.
Flags: needinfo?(mozilla)
Flags: needinfo?(francois)
(In reply to :Gijs Kruitbosch from comment #14)
> > hence mixedContentBlocker bails
> > because it can not query the root docshell and blocks the load.
> 
> I would have assumed that a network error page was always loadable, mixed
> content blocking or no. And either way, both source and destination in this
> case are on http://, not https, so I don't see why this breaks...

Even though it's not fixing the specific problem here, let me explain my thinking: If the loadingPrincipal is the systemPrincipal, then content policies are bypassed. If the loadingPrincipal is not the systemPrincipal and we do *not* provide a loadingNode in the loadInfo, then mixed content blocker returns false, because it cannot query the toplevel docshell [1]. Please note, in this case it doesn't matter if any involved schemes are http or https. But that's just for clarification.

I spent some time debugging, but as of now, I can only confirm that mixed content blocking is not responsible for the breakage here.

[1] http://mxr.mozilla.org/mozilla-central/source/dom/security/nsMixedContentBlocker.cpp#624
Flags: needinfo?(mozilla)
I finally got a chance to debug that.

STR:
1) visit: http://www.jzip.com/
2) click [Get Jzip]

Current Behavior: Nothing happens
Expected Behavior: Unwanted App page should be displayed

I was able to verify that indeed unwanted software blocks the load; which is correct, because we get the same behavior when pasting the link directly into the URL bar: http://download.jzip.com/jZipSetup.exe

We are then about to load the following URL:

about:blocked?e=unwantedBlocked&u=http%3A//download.jzip.com/jZipSetup.exe&s=blacklist&c=UTF-8&f=regular&d=The%20site%20at%20download.jzip.com%20has%20been%20reported%20as%20serving%20unwanted%20software%20and%20has%20been%20blocked%20based%20on%20your%20security%20preferences


The problem is that the 'about:blocked' page does *not* get displayed as it should. The stacktrace underneath shows that everything until DoURILoad() seems to be correct. From that point on, I am not sure where the actual error happens.

Francois, did you ever run into such a problem with 'unwanted software'?

Also, the title of this bug is misleading, there is no redirect involved!


#1  0x00007fffe94fab11 in nsDocShell::DoURILoad (this=0x7fffc1b18000, aURI=0x7fffca69a590, aReferrerURI=0x0, aSendReferrer=true, aReferrerPolicy=0, aOwner=0x0, aTypeHint=0x0, 
    aFileName=..., aPostData=0x0, aHeadersData=0x0, aFirstParty=true, aDocShell=0x0, aRequest=0x7fffffff90d0, aIsNewWindowTarget=false, aBypassClassifier=false, 
    aForceAllowCookies=false, aSrcdoc=..., aBaseURI=0x0, aContentPolicyType=29) at /home/ckerschb/moz/mc/docshell/base/nsDocShell.cpp:10404
#2  0x00007fffe94d140c in nsDocShell::InternalLoad (this=0x7fffc1b18000, aURI=0x7fffca69a590, aReferrer=0x0, aReferrerPolicy=0, aOwner=0x0, aFlags=1, aWindowTarget=0x0, 
    aTypeHint=0x0, aFileName=..., aPostData=0x0, aHeadersData=0x0, aLoadType=65537, aSHEntry=0x0, aFirstParty=true, aSrcdoc=..., aSourceDocShell=0x7fffc1b181a8, aBaseURI=0x0, 
    aDocShell=0x0, aRequest=0x0) at /home/ckerschb/moz/mc/docshell/base/nsDocShell.cpp:10201
#3  0x00007fffe94e3af4 in nsDocShell::LoadErrorPage (this=0x7fffc1b18000, aURI=0x7fffd6755f00, aURL=0x0, aErrorPage=0x7fffffffa680 "blocked", 
    aErrorType=0x7fffffffa9d8 u"unwantedBlocked", 
    aDescription=0x7fffc3be3c08 u"The site at download.jzip.com has been reported as serving unwanted software and has been blocked based on your security preferences.", 
    aCSSClass=0x7fffebcb9792 <.L.str254> "blacklist", aFailedChannel=0x7fffc005e058) at /home/ckerschb/moz/mc/docshell/base/nsDocShell.cpp:5185
#4  0x00007fffe94e1525 in nsDocShell::DisplayLoadError (this=0x7fffc1b18000, aError=NS_ERROR_UNWANTED_URI, aURI=0x7fffd6755f00, aURL=0x0, aFailedChannel=0x7fffc005e058)
    at /home/ckerschb/moz/mc/docshell/base/nsDocShell.cpp:5058
#5  0x00007fffe94f13f9 in nsDocShell::EndPageLoad (this=0x7fffc1b18000, aProgress=0x7fffc1b18028, aChannel=0x7fffc005e058, aStatus=NS_ERROR_UNWANTED_URI)
    at /home/ckerschb/moz/mc/docshell/base/nsDocShell.cpp:7626
#6  0x00007fffe94ee3be in nsDocShell::OnStateChange (this=0x7fffc1b18000, aProgress=0x7fffc1b18028, aRequest=0x7fffc005e058, aStateFlags=131088, aStatus=NS_ERROR_UNWANTED_URI)
    at /home/ckerschb/moz/mc/docshell/base/nsDocShell.cpp:7158
#7  0x00007fffe94f15a5 in non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) () at Unified_cpp_docshell_base0.cpp:7165
#8  0x00007fffe615b42c in nsDocLoader::DoFireOnStateChange (this=0x7fffc1b18000, aProgress=0x7fffc1b18028, aRequest=0x7fffc005e058, aStateFlags=@0x7fffffffb6b4: 131088, 
    aStatus=NS_ERROR_UNWANTED_URI) at /home/ckerschb/moz/mc/uriloader/base/nsDocLoader.cpp:1250
#9  0x00007fffe615aef1 in nsDocLoader::doStopDocumentLoad (this=0x7fffc1b18000, request=0x7fffc005e058, aStatus=NS_ERROR_UNWANTED_URI)
    at /home/ckerschb/moz/mc/uriloader/base/nsDocLoader.cpp:831
#10 0x00007fffe6159b4b in nsDocLoader::DocLoaderIsEmpty (this=0x7fffc1b18000, aFlushLayout=true) at /home/ckerschb/moz/mc/uriloader/base/nsDocLoader.cpp:721
#11 0x00007fffe615a933 in nsDocLoader::OnStopRequest (this=0x7fffc1b18000, aRequest=0x7fffc005e058, aCtxt=0x0, aStatus=NS_ERROR_UNWANTED_URI)
    at /home/ckerschb/moz/mc/uriloader/base/nsDocLoader.cpp:605
#12 0x00007fffe615ad3d in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) () at Unified_cpp_uriloader_base0.cpp:609
#13 0x00007fffe50805b8 in nsLoadGroup::RemoveRequest (this=0x7fffc3612b00, request=0x7fffc005e058, ctxt=0x0, aStatus=NS_ERROR_UNWANTED_URI)
    at /home/ckerschb/moz/mc/netwerk/base/nsLoadGroup.cpp:643
#14 0x00007fffe5352a53 in mozilla::net::nsHttpChannel::OnStopRequest (this=0x7fffc005e000, request=0x7fffca99f800, ctxt=0x0, status=NS_ERROR_UNWANTED_URI)
    at /home/ckerschb/moz/mc/netwerk/protocol/http/nsHttpChannel.cpp:5941
#15 0x00007fffe5352f3d in non-virtual thunk to mozilla::net::nsHttpChannel::OnStopRequest(nsIRequest*, nsISupports*, nsresult) () at Unified_cpp_protocol_http1.cpp:5949
#16 0x00007fffe507d24c in nsInputStreamPump::OnStateStop (this=0x7fffca99f800) at /home/ckerschb/moz/mc/netwerk/base/nsInputStreamPump.cpp:717
#17 0x00007fffe507c222 in nsInputStreamPump::OnInputStreamReady (this=0x7fffca99f800, stream=0x7fffc3ffb000) at /home/ckerschb/moz/mc/netwerk/base/nsInputStreamPump.cpp:436
#18 0x00007fffe507d36f in non-virtual thunk to nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) () at Unified_cpp_netwerk_base2.cpp:498
#19 0x00007fffe4eec970 in nsInputStreamReadyEvent::Run (this=0x7fffcad9b540) at /home/ckerschb/moz/mc/xpcom/io/nsStreamUtils.cpp:91
#20 0x00007fffe4f1a283 in nsThread::ProcessNextEvent (this=0x7ffff6b7ba00, aMayWait=false, aResult=0x7fffffffc0ee) at /home/ckerschb/moz/mc/xpcom/threads/nsThread.cpp:864
#21 0x00007fffe4f907b7 in NS_ProcessNextEvent (aThread=0x7ffff6b7ba00, aMayWait=false) at /home/ckerschb/moz/mc/xpcom/glue/nsThreadUtils.cpp:277
#22 0x00007fffe55a2adb in mozilla::ipc::MessagePump::Run (this=0x7fffe1e7d9c0, aDelegate=0x7ffff6b6a540) at /home/ckerschb/moz/mc/ipc/glue/MessagePump.cpp:95
#23 0x00007fffe5528335 in MessageLoop::RunInternal (this=0x7ffff6b6a540) at /home/ckerschb/moz/mc/ipc/chromium/src/base/message_loop.cc:234
#24 0x00007fffe5528265 in MessageLoop::RunHandler (this=0x7ffff6b6a540) at /home/ckerschb/moz/mc/ipc/chromium/src/base/message_loop.cc:227
#25 0x00007fffe552823d in MessageLoop::Run (this=0x7ffff6b6a540) at /home/ckerschb/moz/mc/ipc/chromium/src/base/message_loop.cc:201
#26 0x00007fffe89a0113 in nsBaseAppShell::Run (this=0x7fffd9715040) at /home/ckerschb/moz/mc/widget/nsBaseAppShell.cpp:156
#27 0x00007fffe9a0e9f7 in nsAppStartup::Run (this=0x7fffd9719100) at /home/ckerschb/moz/mc/toolkit/components/startup/nsAppStartup.cpp:281
#28 0x00007fffe9ae4085 in XREMain::XRE_mainRun (this=0x7fffffffc9b8) at /home/ckerschb/moz/mc/toolkit/xre/nsAppRunner.cpp:4292
#29 0x00007fffe9ae49df in XREMain::XRE_main (this=0x7fffffffc9b8, argc=1, argv=0x7fffffffdfa8, aAppData=0x7fffffffcc60) at /home/ckerschb/moz/mc/toolkit/xre/nsAppRunner.cpp:4389
#30 0x00007fffe9ae523f in XRE_main (argc=1, argv=0x7fffffffdfa8, aAppData=0x7fffffffcc60, aFlags=0) at /home/ckerschb/moz/mc/toolkit/xre/nsAppRunner.cpp:4478
#31 0x0000000000405a20 in do_main (argc=1, argv=0x7fffffffdfa8, xreDirectory=0x7ffff6b4c900) at /home/ckerschb/moz/mc/browser/app/nsBrowserApp.cpp:212
#32 0x000000000040515c in main (argc=1, argv=0x7fffffffdfa8) at /home/ckerschb/moz/mc/browser/app/nsBrowserApp.cpp:399

Comment 17

2 years ago
Gah, I should have spotted this sooner.

This works as expected. The error page is simply loaded into the iframe (which makes sense - that's where the link points), and the iframe is invisible.

It's really not clear what else we would be expected to do here - loading the error page in the parent frame will break other usecases, even if it might seem to be the "right" thing here.

I'll leave the needinfo on francois to decide if we need to do something else here, but on the whole I'm tempted to suggest invalid/wontfix.
Flags: needinfo?(pavel)
Summary: no reply on redirected 302 download links → When trying to load blocked (unwanted) software in a hidden frame, it looks like "nothing happens" because the software warning page loads in the frame.
(In reply to Gijs Kruitbosch (PTO until Sep 6) from comment #17)
> This works as expected. The error page is simply loaded into the iframe
> (which makes sense - that's where the link points), and the iframe is
> invisible.

Ah, now it all makes sense to me. Thanks Gijs.

> I'll leave the needinfo on francois to decide if we need to do something
> else here, but on the whole I'm tempted to suggest invalid/wontfix.

In that case I also suggest invalid/wontfix.
I think it would be great if we had a better console log message (than the current "NS_ERROR_NOT_AVAILABLE:"), but I think it is indeed working as intended. So perhaps it's not entirely invalid/wontfix.

Is this "loading a download in an invisible iframe" pattern common?
Flags: needinfo?(francois)

Comment 20

2 years ago
(In reply to François Marier [:francois] from comment #19)
> I think it would be great if we had a better console log message (than the
> current "NS_ERROR_NOT_AVAILABLE:"), but I think it is indeed working as
> intended. So perhaps it's not entirely invalid/wontfix.
> 
> Is this "loading a download in an invisible iframe" pattern common?

It's definitely not unheard of, yes. For instance, iTunes does something similar, and then redirects the toplevel/parent page to a different "success, you downloaded iTunes!" page (which has caused issues for us in the past because the downloading iframe goes away).

The thing that worries me here is that the "unwanted software" page is big and red and scary - and it's big and red and scary for a reason. Anything else that doesn't require the iframe/window, like (say) an infobar, wouldn't be nearly as scary.

We could consider making that page break out of its iframe when displayed... François, does that sound sane?
Flags: needinfo?(francois)
Created attachment 8654217 [details]
Screenshot of the error page in Chrome

Alternatively, we could do like Chrome and navigate the top-level page to the error page.
(In reply to Gijs Kruitbosch (PTO until Sep 6) from comment #20)
> We could consider making that page break out of its iframe when displayed...
> François, does that sound sane?

Sounds good to me because it tells users what's going on instead of silently blocking the download and just looking like Firefox is broken.
Flags: needinfo?(francois)

Comment 23

2 years ago
I just realized we actually have telemetry for how much stuff happens in frames and how much in toplevel loads:

https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2015-09-29&keys=__none__!__none__!__none__&max_channel_version=release%252F41&measure=SECURITY_UI&min_channel_version=null&product=Firefox&sanitize=1&sort_keys=submissions&start_date=2015-09-17&table=0&trim=1&use_submission_date=0

This shows that about 1/3 of the malwareBlocked pages are shown in iframes, and more like 2/5 of the unwantedBlocked pages.

It seems fair to want to fix this so that people are notified about these attempts and there's an impact (and therefore impetus to fix, either by contacting Google to get off the list, or the ad network to fix its ads, or...) to the sites to actually do something about it (rather than the framing of stuff to try to evade user notice where possible).

François, do you have cycles to work on this?
Flags: needinfo?(francois)
So the consensus is that we want to block the whole page (with the big red interstitial warning page) whenever a page contains an iframe on the unwanted list?
Flags: needinfo?(francois)

Comment 25

2 years ago
(In reply to François Marier [:francois] from comment #24)
> So the consensus is that we want to block the whole page (with the big red
> interstitial warning page) whenever a page contains an iframe on the
> unwanted list?

I haven't asked anyone else - who did the UX for the original implementation? It'd be good to check with them.

That said, considering Chrome already does this, and considering this bugreport + telemetry, it seems like it:
1) solves a real problem we have today (see this bugreport);
2) shouldn't violate assumptions that sites have been making about how this is supposed to work;
3) may actively help protect users by the negative incentive of having this error page appear across the entire thing (instead of just in a frame that is as likely as not to be invisible anyway).

So it does seem like it'd be sensible. Anyway, let's doublecheck with UX before proceeding, I guess. Can you forward this appropriately?
Flags: needinfo?(francois)
(In reply to :Gijs Kruitbosch from comment #25)
> I haven't asked anyone else - who did the UX for the original
> implementation? It'd be good to check with them.

I'm not sure who that would be (and not entirely sure there ever was a UX person involved), but I'll CC Bram who is the UX person behind the new TLS error pages.
 
> So it does seem like it'd be sensible. Anyway, let's doublecheck with UX
> before proceeding, I guess. Can you forward this appropriately?

I agree, the Chrome behavior is better than what we've got, so we may as well copy it as a first step.
Flags: needinfo?(francois)

Comment 27

2 years ago
I’ve adapted the security error pages to handle warnings for attack sites and sites carrying unwanted software.

http://brampitoyo.github.io/fx-untrusted-connection/attack.xhtml
http://brampitoyo.github.io/fx-untrusted-connection/unwanted.xhtml

In each case, you’ll see that I’ve titled the page “Harmful Site” and wrote a strong warning that says “This site is trying to infect your computer”. Under the Advanced link, I’ve linked to the Google Safe Browsing diagnostic page so technical users can see it if they’re interested.

Lastly, there’s of course an unsafe link to go ahead and load the site in question.

Google Chrome uses an “x” padlock icon on a striking red background, and Safari uses a red text. How does this design fare in comparison? Thoughts?

Comment 28

2 years ago
(In reply to Bram Pitoyo [:bram] from comment #27)
> I’ve adapted the security error pages to handle warnings for attack sites
> and sites carrying unwanted software.
> 
> http://brampitoyo.github.io/fx-untrusted-connection/attack.xhtml
> http://brampitoyo.github.io/fx-untrusted-connection/unwanted.xhtml
> 
> In each case, you’ll see that I’ve titled the page “Harmful Site” and wrote
> a strong warning that says “This site is trying to infect your computer”.
> Under the Advanced link, I’ve linked to the Google Safe Browsing diagnostic
> page so technical users can see it if they’re interested.
> 
> Lastly, there’s of course an unsafe link to go ahead and load the site in
> question.
> 
> Google Chrome uses an “x” padlock icon on a striking red background, and
> Safari uses a red text. How does this design fare in comparison? Thoughts?

The problem is not to do with the styling of the page, it's to do with the page not showing up at all if it's inside a frame that is not visible to the user. The question is whether we should replace the toplevel page with the warning in that case.
Flags: needinfo?(bram)
Component: Document Navigation → Safe Browsing
Product: Core → Toolkit
(In reply to Bram Pitoyo [:bram] from comment #27)
> I’ve adapted the security error pages to handle warnings for attack sites
> and sites carrying unwanted software.

I moved these mocks to bug 1214859 since it's separate from fixing this one.

Comment 30

2 years ago
(In reply to :Gijs Kruitbosch from comment #28)
> […] The question is whether we should replace the toplevel page with the
> warning in that case.

Sorry for misunderstanding the bug. I thought it covered the design and copy of the warning page.

My answer would be “yes”. Our warning page should replace the top level page, so malicious sites can’t be sneaky and hide our warning inside an invisible frame.
Flags: needinfo?(bram)
Blocks: 1239729

Comment 31

2 years ago
Created attachment 8709910 [details]
MozReview Request: Bug 1195242 - force malware and phishing block warnings to break out of frames at the docshell level, r?bz

Review commit: https://reviewboard.mozilla.org/r/31603/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/31603/
Attachment #8709910 - Flags: review?(bzbarsky)

Comment 32

2 years ago
(In reply to :Gijs Kruitbosch from comment #31)
> Created attachment 8709910 [details]
> MozReview Request: Bug 1195242 - force malware and phishing block warnings
> to break out of frames at the docshell level, r?bz
> 
> Review commit: https://reviewboard.mozilla.org/r/31603/diff/#index_header
> See other reviews: https://reviewboard.mozilla.org/r/31603/

FWIW, I am a little bit... uneasy... about this, in that if your site gets XSS'd you can use the blocking mechanism to defeat e.g. iframe sandboxing. That said, hopefully the page is scary enough that people *don't* just click through (do we have data on that?), and it would likewise be bad if we did allow the page to be loaded in the iframe if it were sandboxed or somehow (CSP?) prevented from breaking out of the iframe at the content level (which is why I elected not to just make this a bit of JS on the page itself).

Boris, if you want to delegate this review, I totally understand, but I wanted to at least get your thoughts on that issue, and it seems you, smaug and jst (maybe?) are the only active docshell peers at this point...
> That said, hopefully the page is scary enough that people *don't* just click through 

Can we just disable the option of clicking through if we forced the load into toplevel like that?

If not, I'm really not terribly happy with this.  How does Chrome handle this in the sandboxed case, out of curiosity?
Flags: needinfo?(gijskruitbosch+bugs)

Comment 34

2 years ago
You should see bug 1239729 bz.  I use click jacking there, but I'm fairly sure I could have done some nasty things even without having about:blocked in an iframe(was a slight tweak at one point).

I really just don't want to see a solution that relies on about:blocked taking over the whole tab, or an at best not quite right tweak of how chrome handles this.  Whatever you guys decide on I'll put it on my list to get to quickly.

Both of these scenarios make me thirsty in terms of new ways to possibly attack them :-)

Comment 35

2 years ago
(In reply to Boris Zbarsky [:bz] from comment #33)
> > That said, hopefully the page is scary enough that people *don't* just click through 
> 
> Can we just disable the option of clicking through if we forced the load
> into toplevel like that?

I'm not sure how much use that is considering we keep the URL - hit enter in the URL bar and it'll re-prompt without the 'ignore' thing being disabled...

> If not, I'm really not terribly happy with this.  How does Chrome handle
> this in the sandboxed case, out of curiosity?

Not sure off-hand, will check.
> hit enter in the URL bar and it'll re-prompt without the 'ignore' thing being disabled

Um... changing the URL of a page the user actually loaded to the URL of a known attack page seems _really_ fishy.  :(

Comment 37

2 years ago
(In reply to Boris Zbarsky [:bz] from comment #36)
> > hit enter in the URL bar and it'll re-prompt without the 'ignore' thing being disabled
> 
> Um... changing the URL of a page the user actually loaded to the URL of a
> known attack page seems _really_ fishy.  :(

Maybe we should just deny the load when not top-level and not display a page at all. (I'll still check Chrome in a bit, in a mtg...)

Comment 38

2 years ago
(In reply to Boris Zbarsky [:bz] from comment #33)
> If not, I'm really not terribly happy with this.  How does Chrome handle
> this in the sandboxed case, out of curiosity?

It loads the error page at the top-level (even when the iframe has a sandbox="" attribute), but keeps the URL in the URL field the same as it was.

We could probably fix the URL somehow... I'm actually not sure how it ends up there right now, maybe frontend distills it from the error page's URL...

(In reply to Cody Crews from comment #34)
> You should see bug 1239729 bz.  I use click jacking there, but I'm fairly
> sure I could have done some nasty things even without having about:blocked
> in an iframe(was a slight tweak at one point).

FWIW, I'm aware that we should still add mitigations to about:blocked similar to what we have done for some of our dialogs/panels, but that is in principle orthogonal to what we do here.
Flags: needinfo?(gijskruitbosch+bugs)

Comment 39

2 years ago
Comment on attachment 8709910 [details]
MozReview Request: Bug 1195242 - force malware and phishing block warnings to break out of frames at the docshell level, r?bz

Whatever we do decide, this does not seem like the right thing.
Attachment #8709910 - Flags: review?(bzbarsky) → review-
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P2
Duplicate of this bug: 442929

Updated

a year ago
Whiteboard: [sb-backlog]

Updated

a year ago
Whiteboard: [sb-backlog] → [sb-backlog] [sb-moderate]

Updated

8 months ago
See Also: → bug 1328824
Depends on: 1288633
Duplicate of this bug: 1345011
Duplicate of this bug: 1345051
Priority: P2 → P3
The commit identified in comment 13 increased the likelihood that the problem will show up because we expanded the list of sites we show warnings for. It's not a regression since this has always been the behavior when encouraging unsafe things in iframes.
Keywords: regression
You need to log in before you can comment on or make changes to this bug.