506172 - going to malware site with redirect causes info bar to disappear

Status: VERIFIED FIXED

(Toolkit :: Safe Browsing, defect)

Description

[Samuel Sidler (old account; do not CC)]

(These steps may not last if the site gets fixed...)

STR:
  1. Go to http://unamas.com/
  2. "Ignore this warning"

AR:
Info bar appears and disappears as the site redirects to http://unamas.com/index.asp

ER:
Info bar stays put throughout my browsing on unamas.com since it's a malware site.

Note that if I refresh http://unamas.com/index.asp the malware warning appears and if I click through it, the info bar remains. This seems to be only because http://unamas.com/ redirects to http://unamas.com/index.asp.

Chrome (current version) redirects to http://unamas.com/index.asp but then shows the warning page again. I don't think they have an info bar at all.

Comment 1

[Mike Beltzner [:beltzner, not reading bugmail]]

I don't think this needs to be a closed bug; it's not really a security risk, IMO.

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#2379 is where we do the page load and pass along LOAD_FLAGS_BYPASS_CLASSIFIER - somehow that's also being passed along with the page redirect.

Comment 2

[Samuel Sidler (old account; do not CC)]

(In reply to comment #1)
> I don't think this needs to be a closed bug; it's not really a security risk,
> IMO.

As I told beltzner in person, it potentially since depending on how the redirect is done. If a malware creator can do the redirect using an htaccess file, they already have access to the server so it wouldn't be hard to do. If it's using a DNS redirect, sure. Maybe it's not a major security issue and we can open it, but wiser minds than mine can make that call.

But the most important question here: Does this mean we can no longer order lunch from Una Mas?

Comment 3

[Daniel Veditz [:dveditz]]

Confirming for Firefox 3.0.x as well. Firefox 2 had a different UI for phishing sites and this bug does not apply.

I don't think we want to re-vet the URL at each redirect once the user has already said to "ignore" (which maybe Chrome is based on your description). That just forces the user to repeatedly stomp on the ignore link and get ticked off at us. The problem is just that we're losing the warning bar along the way, but I don't think there's any real security problem since the user has already been warned for that load and explicitly ignored it.

If the user hasn't hit the ignore button then we should be checking each redirect and will put up the block page if we need to.

Comment 4

[Emm]

I don't know how much this is related to phishing protection but this also happens sometimes on yahoo mail if viewed with js disabled and on IP Boards, when doing a search from index.php?act=Search&CODE=01 I should get the see bar to allow the redirect to index.php?act=Search&CODE=show&searchid=<ID>&<OTHER_VARS>. First attempt it just closes itself in a split second, second attempt (after waiting for the forum's anti-spam counter to finish) it stays.

FX 3.5.3 on Win 7 RC x64.

Comment 5

[scialdone1]

Had my anti-phising on etc. Was not given a choice to ignore, but given no option as I was automatically directed to the My Space site which I don't even use, then would completely freeze up.

Comment 6

[Frank Yan (:fryn)]

After discussing this with :MattN, it seems that, in https://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#8694 (DoURILoad), aBypassClassifier needs to be set to false for redirects.

If I'm understanding this correctly, it makes more sense to make the fix there rather than in the front-end. Am I correct in this assessment? If so, I'm not the best person to fix docshell stuff, but I can give it a try if someone can give me pointers.

Comment 7

[Justin Dolske [:Dolske]]

Frank and I chatted about this a couple weeks ago, and came up with what should be a simpler solution. (At least, it sure avoids docshell :).

Notification bars already support .persistence and .timeout properties, which are used by login manager for exactly the problem described here. [Sometimes when you log in to a site -- Google being one huge example -- there can be a number of redirects between clicking the "login" button and ending up at your final destination. Having the "Remember Password?" notification disappear before you have a chance to see if it's successful was why these properties got implemented.]

Same solution should work here. If the user continues on to the malware site, use the timeout to make sure the notification bar remains for X seconds. This will persist across redirects during that time period. There's a slight quirk in that if you click a link during that period, the bar will also remain. But let's call that a feature -- if you're clicking a link on a malware site and a warning remains, it's quite likely still relevant (since malware sites often try to trap you in a web of malware/spam/phishing links).

We could handle _typing_ a URL as a special case, and immediately clear any existing malware notification before loading the new URL. Fine by me without with or without.

Basic idea here is that since we're already in sketchy territory (malware site, user explicitly continues through warning), having an imperfect-but-simple solution is better than nothing. Especially when the imperfection is biased toward showing the notification longer than it maybe really should.

Comment 8

[Justin Dolske [:Dolske]]

Easy STR:

1) Load "http://mozilla.org/firefox/its-a-trap.html" (note: no "www")
2) Click "ignore this warning"
3) Page loads and redirects to www.mozilla.org (with "www")

At step 3, the warning bar flashes into view and then disappears.

Comment 9

[Emm]

Most of us probably moved on to bigger and better things, like FX13 right now. This bug was important to be fixed when the browser was in popular use. Now it's not so much of an issue... FX13 doesn't load any bar, it just redirects to "www". Spending development resources on a minor use project is a waste.

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

I talked with Frank and he said he was cool with me taking this bug.

This patch sets persistence = -1 on the notification bar so it will remain visible until the user removes it explicitly.

I tested it out with the STR in comment #8 and it worked as desired.

Comment 11

[Matthew N. [:MattN]]

Comment on attachment 638956 [details] [diff] [review]
Patch

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

This seems straightforward and I think the quirk mentioned in comment 7 is acceptable for this case.

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

Pushed to mozilla-inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6e084c98a617

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 638956 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): no bug #
User impact if declined: users may not see critical warning bar if malware site uses a redirect
Testing completed (on m-c, etc.): locally, just landed on m-i
Risk to taking this patch (and alternatives if risky): none expected, standard way to make notification bars persistent
String or UUID changes made by this patch: none

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: chance that users wont see a security warning
Fix Landed on Version: fx16
Risk to taking this patch (and alternatives if risky): none expected
String or UUID changes made by this patch: none

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 14

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/6e084c98a617

Comment 15

[Alex Keybl [:akeybl]]

Comment on attachment 638956 [details] [diff] [review]
Patch

Taking a lingering sg:low in our final beta isn't worth non-zero risk. Let's get this on 15 and the corresponding ESR after the version bump.

Comment 16

[Jared Wein [:jaws] (please needinfo? me)]

Pushed to mozilla-aurora:
https://hg.mozilla.org/releases/mozilla-aurora/rev/98a27303eb94

Comment 17

[:Gavin Sharp [email: [e-mail]]]

This is a longstanding low-severity bug, I don't think we need to take it on ESR.

Comment 18

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 638956 [details] [diff] [review]
Patch

Agree with Gavin, this doesn't fit in ESR criteria so no need to take it.

Comment 19

[Paul Silaghi, QA [:pauly]]

(In reply to Justin Dolske [:Dolske] from comment #8)
> Easy STR:
> 
> 1) Load "http://mozilla.org/firefox/its-a-trap.html" (note: no "www")
> 2) Click "ignore this warning"
> 3) Page loads and redirects to www.mozilla.org (with "www")
> 
> At step 3, the warning bar flashes into view and then disappears.

The warning bar remains on the screen.
Verified fixed on FF 15b3 on Win 7/64, Ubuntu 12.04 and Mac OS X 10.6.

Comment 20

[Paul Silaghi, QA [:pauly]]

Verified fixed on FF 16b1 on Win 7/64, Ubuntu 12.04 and Mac OS X 10.6.