820283 - Sites stay marked as a malware or phishing even after removal from the SafeBrowsing DB

Status: VERIFIED FIXED

(Toolkit :: Safe Browsing, defect)

Description

[Davide]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/20100101 Firefox/17.0
Build ID: 20121128204232

Steps to reproduce:

Hi all,
we had a malware problem last weekon our site http://www.vecchiasignora.com/ , now all is clean, we are out from all the blacklist like you see on sucuri.net, for example. I and many users of our forum have cleaned the cache and cookie.


Actual results:

When we go to te site appear the advise of malware content, that is impossibile, in fact if we click on "why was this site blocked?" it sent us in the google page that declares "this site is not suspect". If we clean cache ad re-enter we have the same problem. Not always in the home page but after 4-5 clicks on the site.


Expected results:

Like all other browsers there is nothing of malicious to report, the site its clean and this is a very very big problem for us and for our reputation..
Can someone help us?

Comment 1

[Cork]

WFM. Are you sure the users getting the warning isn't using one of the antivirus extensions?

Comment 2

[Cork]

Oh, and the database is provided by google, firefox just downloads it.

Comment 3

[Davide]

First of all tahnks for the reply.
I'm sure because I have this problem too, and I have no extension, and the users that have this problem are too many for think that the problem is in firefox users configuration.

I know that the database is from Google,but the page of google http://safebrowsing.clients.google.com/safebrowsing/diagnostic?site=vecchiasignora.com report that the site is not suspect. In fact Chrome dismiss the advise of malware just google has changed the status.

Thanks again for the reply

Comment 4

[Cork]

Firefox syncs with the live database ones a day if i remember correctly. So it might at worst take up to 24h from that google removes the url, till the user stops seeing the warning.

Comment 5

[Davide]

Google removes the url from the blacklist the 7th December, how is reported in the link of safebrowsing

Comment 6

[Matthias Versen [:Matti]]

The syncing should not be an issue. In case of a positive hit in the database we contact google again and check the URL against the online database.

BTW: I don't get a warning on the URL http://www.vecchiasignora.com/

We had some changes in the phishing protection, maybe the online recheck doesn't work as expected ?

Comment 7

[Davide]

The warning is still there for me. Not always in the home page appear the error, as I wrote sometimes the error appear in other page, sometimes in the home page, anyway the error is present. Maybe its the online recheck, like you have written.. What can we do?

Thanks again

Comment 8

[Davide]

reports of this problem increases daily

Comment 9

[Davide]

I found the problem: Firefox store malicious sites in the safebrowsing folder. If the user doesn't delete that, firefox show a malware messagge at every access to te site. I don't think that this is good, anyway after that all works fine.


Bye bye

Comment 10

[Matthias Versen [:Matti]]

The caching shouldn't matter since we should renew an entry if we get a positive result.

Comment 12

[remi.nectoux]

Hi Matthias.
Thanks for your work.

Just a question, i'm not familiar with bug resolution : 
Do you confirm the bug, and do you have an idea of due date resolution ?

Many customers leave the website when they have the alert :(

And we can't tell to them to delete their safebrowsing folder

Comment 13

[Matthias Versen [:Matti]]

I confirmed the bug because we have 2 independent reports about the issue.
The date of resolution will not help you because this seems to be a bug in the code based on my assumption that Firefox should validate a positive hit in the downloaded database directly with the google API server. 
It could be also a "bug" in the google safebrosing API. 
Chrome is AFAIK using a different version of this API and is probably not affected by a server site issue.

In case that a developer have to fix a bug in the Firefox code, the fix will be in one of the next Firefox versions but unlikely the next one (FF18).

Comment 14

[Gian-Carlo Pascutto [:gcp]]

>...that Firefox should validate a positive hit in the downloaded database directly 
>with the google API server. 

Unfortunately things are much more complicated than that. We keep a local database of sites, but this one has known false positives. When we hit there, we verify with Google. If there is a positive result (as was the case here, see original report), we cache the positive result. We will keep the cached result as long as Google doesn't send us a message to remove the entry from the local database.

If the user recently started his browser, there is an additional complication in that we will *not* consult our own cache because we don't know if it is up to date. The first such update typically happens after 5-15 minutes of browsing. This could cause the warning to initially not appear, but then pop up later (which to the user might appear to be when he is deeper inside the site).

This bug very much sounds like we don't correctly expire the cached (full) entries upon receiving the removal from the local database. That is this code:
https://mxr.mozilla.org/mozilla-beta/source/toolkit/components/url-classifier/HashStore.cpp#976 
It's fairly complicated, which is reason why I suspect it.

The code is new in Firefox 17. In Firefox 19, our behavior changed again due to bug 806422, where we always drop our cache on an update (it makes the code I suspect to have a bug redundant).

Dave, Sid, do any of you have time to dive into this right now and see if anything jumps out at you in the relevant code? 

If the bug is as I suspect, users will see the warning on affected sites (sites that had malware, got into the database, had users visit them, fixed themselves, and then got themselves removed from the db) *forever*. This is BAD!

Alternately, we can push for bug 806422 to be included in Firefox 18.

Comment 15

[Gian-Carlo Pascutto [:gcp]]

Attachment: Affected safebrowsing dir

I was able to reproduce on one of my installations that had been used for testing. This profile will give a warning on foto.com, with the restrictions from the comment above (the first update must have happened).

With some more logging on a beta build, I can see a hit in Completes, but not in Prefixes. This confirms the problem must be that (Sub)Prefixes are failing to knock out (Add)Completes.

Comment 16

[Gian-Carlo Pascutto [:gcp]]

Attachment: Patch 1. Verify in both Completes and Prefixes before reporting a hit.

This is the smallest patch I can come up with that solves the problem for affected sites. It does not solve the bug in updating itself. Just putting it up in case we don't want the one from bug 806422 for some reason.

Comment 17

[Dave Camp (:dcamp)]

Comment on attachment 694088 [details] [diff] [review]
Patch 1. Verify in both Completes and Prefixes before reporting a hit.

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

I'd prefer to take the patch in bug 806422, but this would work too.

Comment 18

[Alex Keybl [:akeybl]]

We'll need this for mozilla-beta (Firefox 18). We're still deciding how to help affected users work around this issue in the Firefox 17 timeframe.

Comment 19

[Alex Keybl [:akeybl]]

(I meant to say we'll need to fix the issue this bug tracks, we're still deciding on which patch to take)

Comment 20

[Alex Keybl [:akeybl]]

(In reply to Dave Camp (:dcamp) from comment #17)
> Comment on attachment 694088 [details] [diff] [review]
> Patch 1. Verify in both Completes and Prefixes before reporting a hit.
> 
> Review of attachment 694088 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'd prefer to take the patch in bug 806422, but this would work too.

Can you give us a bit more context as to why you prefer the patches in bug 806422? I think this bug proves that we can't really rely on our pre-release audience to find major issues with SafeBrowsing, so my intuition tells me to go with the smallest, most readable change (and hopefully therefore the least regression prone).

Comment 21

[Gian-Carlo Pascutto [:gcp]]

>Can you give us a bit more context as to why you prefer the patches in bug 806422?

From my side:

1) It has both had testing in Aurora/Nightly and has tests included. The patch attached here is the most minimal change I could came up with and has only been tested by me and no-one else. (It does share a lot with bug 807822.)
2) This patch does not prevent the broken cache (URLCLASSIFIER_LC_COMPLETIONS) from growing indefinitely (...till Firefox 19). There's a theoretical chance this will eventually lead to a false positive showing again. Though it's something like 1/5000 odds per affected site, so we should be able to live with it.
3) Bug 806422 includes a functional change we eventually want. It will permanently reduce the impact of bugs such as this one.

Comment 22

[Gian-Carlo Pascutto [:gcp]]

As a workaround/hotfix for Firefox 17, it may be possible to reduce the setting "urlclassifier.confirm-age" to 0 in about:config. This should consider cached results to be considered invalid immediately.

Comment 23

[u279076]

Assigning myself as QA contact. I can take care of testing and coordinating testing of whichever path we decide to take. Note, due to the holidays I will be available Monday, Thursday, and Friday for both of the next two weeks.

Comment 24

[Alex Keybl [:akeybl]]

We've filed bug 823667 as a workaround for FF17 (if it ends up being necessary, for instance a major web property is impacted).

Comment 25

[Dave Camp (:dcamp)]

(In reply to Alex Keybl [:akeybl] from comment #20)
> (In reply to Dave Camp (:dcamp) from comment #17)
> Can you give us a bit more context as to why you prefer the patches in bug
> 806422? I think this bug proves that we can't really rely on our pre-release
> audience to find major issues with SafeBrowsing, so my intuition tells me to
> go with the smallest, most readable change (and hopefully therefore the
> least regression prone).

I don't feel strongly about this either way, I'd be pretty happy to go with the smaller patch here.

Comment 26

[Alex Keybl [:akeybl]]

Comment on attachment 694088 [details] [diff] [review]
Patch 1. Verify in both Completes and Prefixes before reporting a hit.

Given that, let's take the smaller fix for FF18.

Comment 27

[Alex Keybl [:akeybl]]

Comment on attachment 694088 [details] [diff] [review]
Patch 1. Verify in both Completes and Prefixes before reporting a hit.

Also approving for mozilla-esr17.

Comment 28

[Dave Camp (:dcamp)]

https://hg.mozilla.org/releases/mozilla-beta/rev/f4467b37dfef

Comment 29

[u279076]

(In reply to Dave Camp (:dcamp) from comment #28)
> https://hg.mozilla.org/releases/mozilla-beta/rev/f4467b37dfef

What specifically should I be on the lookout for when testing this in the next Beta?

Comment 30

[Dave Camp (:dcamp)]

gcp's steps to reproduce:
- Use Firefox 17 or 18
- Unzip the "Affected safebrowsing dir" zip from bug 820283 in the
safebrowsing subdir in the Local profile.
- Start Firefox and immediately go to foto.com
- You should not see a warning.
- Restart Firefox
- Wait 15 minutes
- Go to foto.com
- On an affected Firefox, you will see a warning. This is the bug, no
warning should be given (behavior of Firefox 16, 19, 20, ...).

Comment 31

[u279076]

Okay, thanks Dave. I'll add this to my to-do list for next Thursday when we have builds.

Comment 32

[Dave Camp (:dcamp)]

https://hg.mozilla.org/releases/mozilla-beta/rev/f4467b37dfef

Comment 33

[Dave Camp (:dcamp)]

Oops, https://hg.mozilla.org/releases/mozilla-esr17/rev/aabe158bba40

Comment 34

[Dave Camp (:dcamp)]

Backed out:

https://hg.mozilla.org/releases/mozilla-esr17/rev/f78bd45651f8
https://hg.mozilla.org/releases/mozilla-beta/rev/878ffb3da483

Comment 35

[Dave Camp (:dcamp)]

So this patch doesn't break real stuff, but it *does* break the -simple files that don't add prefixes - only complete.  So test_classifier.html is failing, and I bet the test urls (http://www.mozilla.org/firefox/its-an-attack.html) won't catch after this patch either.

So we can either try to find a fix for that (I can't think of a quick-n-easy one offhand, can you gcp?) or fall back to 806422.

Comment 36

[Dave Camp (:dcamp)]

I guess we could do:

if (found || mTableName.Find("-simple") != kNotFound) {
  CheckCompletes(); 
}

I'll try that.

Comment 37

[Dave Camp (:dcamp)]

Attachment: v2

This somewhat-more-ugly patch will trust completions on a -simple table.

Comment 38

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 694663 [details] [diff] [review]
v2

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

::: toolkit/components/url-classifier/LookupCache.cpp
@@ +219,5 @@
>  
>    LOG(("Probe in %s: %X, found %d", mTableName.get(), prefix, found));
>  
> +  if (found || mTestTable) {
> +    *aHas = found;

This is going to cause us to report an urlclassifier hit in test-* for every URL a user visits. We're saved by this check:
https://mxr.mozilla.org/mozilla-beta/source/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp#881
But I'd rather not rely on this. I'd add an if (found) just before this line. i.e.

if (found || mTestTable) {
  if (found)
    *aHas = found;

r+ if this is addressed.

Very sorry for not catching this myself earlier. Just more evidence our tests aren't testing the same codepaths that are used in the field...

::: toolkit/components/url-classifier/LookupCache.h
@@ +152,5 @@
>    CompletionArray mCompletions;
>    // Set of prefixes known to be in the database
>    nsRefPtr<nsUrlClassifierPrefixSet> mPrefixSet;
> +
> +  // Fx19 temporary; true if this is a -simple test table.

Firefox 18.

Comment 39

[Gian-Carlo Pascutto [:gcp]]

Oh oops, it's actually saying *aHas = found not *aHas = true. So that's correct, r+ no remarks just fix that Fx19 comment.

Comment 40

[Gian-Carlo Pascutto [:gcp]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6f33886f5de5

Comment 41

[Gian-Carlo Pascutto [:gcp]]

https://hg.mozilla.org/releases/mozilla-esr17/rev/8c642abdfd03

Comment 42

[Jack Yan]

Thank you to all those above for working on this (one of our sites has been affected by this for a week or more).

Comment 43

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/6f33886f5de5

Comment 44

[u279076]

(In reply to Gian-Carlo Pascutto (:gcp) from comment #40)
> https://hg.mozilla.org/releases/mozilla-beta/rev/6f33886f5de5

We now have Firefox 18.0b6 candidate builds available. Anyone who was experiencing this before, please see if it's fixed:
ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/18.0b6-candidates/build1/

Thank you

Comment 45

[u279076]

FWIW, the fix should also be in the latest Aurora and Nightly builds.

Comment 46

[u279076]

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #45)
> FWIW, the fix should also be in the latest Aurora and Nightly builds.

Scratch that, this never affected 19 or 20.

Comment 47

[Davide]

I tried the 18.0b6 candidate version with the safebrowsing folder not updated, all seems to work, no advise or malware messages!

Comment 48

[u279076]

Thank you Davide. Would you mind also checking one of the latest esr17 builds?
ftp://ftp.mozilla.org/pub/firefox/nightly/latest-mozilla-esr17/

Comment 49

[Davide]

All seems to work well also with this version. the official 18.6b when it comes out?

Thank you to all!

Comment 50

[u279076]

Thank you Davide. Marking this verified based on your testing.

Official 18.0b6 builds are already available on FTP:
ftp://ftp.mozilla.org/pub/firefox/releases/18.0b6/

Updates to existing Beta users are being pushed out as I write this comment and should be live shortly. The links on beta.mozilla.org will be updated soon after that.

Comment 51

[binhaus]

use 17.0.1 and still have this issues , how about other version ? how can my user manage update the fix! I lose around 40 visitor per day  from 24/12 .. how bad this is .. please fix asap for every version

Comment 52

[binhaus]

I just come over some webmaster forums, and many discussion on badwarebusters forum, i believe there are thousand good website got this risk ..  
Plese put out the fix to update ff user asap..

Comment 53

[Tony Mechelynck [:tonymec]]

(In reply to binhaus from comment #51)
> use 17.0.1 and still have this issues , how about other version ? how can my
> user manage update the fix! I lose around 40 visitor per day  from 24/12 ..
> how bad this is .. please fix asap for every version

(In reply to binhaus from comment #52)
> I just come over some webmaster forums, and many discussion on
> badwarebusters forum, i believe there are thousand good website got this
> risk ..  
> Plese put out the fix to update ff user asap..

Bug has recently been fixed on Firefox 18 (beta) and later, and on the ESR version of Firefox 17, see the "Tracking flags" near top right of this page.

See also https://bugzilla.mozilla.org/page.cgi?id=etiquette.html and in particular §§ 1.1 and 1.2.

Comment 54

[u279076]

binhaus, we have no intention to fix this for Firefox 17 given that we are a week away from releasing Firefox 18. Any users who cannot wait are welcome to use the latest Firefox 18 Beta build in the meantime. The current Beta is very close to what we will actually be releasing on January 8.

Sorry, I know it's not ideal but this is the best solution we can offer at this time.

Comment 55

[Jen R.]

FYI - I've been having this same problem intermittently for a couple of weeks now. Testing with Firefox 18.0b6 today and I haven't encountered it once. Hopefully it's fixed.

Comment 56

[Eugene]

Hello Mozilla team 



I have been having this same problem with my site for some time now

Its actually not important if we ourselves see the warning or not, it is what the visitors see which matters most

The problem is that the visitors to the site have been turned away because of this warning

This is especially damaging for the traffic of which we got by spending money to pay and advertise for

This also translates to loss in opportunity and damage in reputation caused by this problem especially that it has actually been going on for some time now



Can you please help us ?

I dont understand why we the victims of hacking attacks have to suffer like this while the hackers run loose

This is not fair

Comment 57

[u279076]

Thank you for reaching out, Eugene. All users of your site should see this issue resolved after receiving an update to Firefox 18 (releasing on Tuesday, January 8, 2013). Anyone using Firefox 18 or later should not experience this bug.

Comment 58

[julieasimons]

I have had the exact same issue with Firefox after our website was hacked and we had a malware issue come up on Friday January 4th. We got it cleaned up within 24 hours, and were removed from Google's blacklist by Sunday morning. HOWEVER....Firefox is still showing us as a site with malware and tries to block customers. This is enormously frustrating. I am the victim, and I got the problem cleared up right away. So to have Firefox still showing MY site as contaminated because someone at Mozilla was able to figure out how to tell users when a site it unsafe, but unable to figure out how to tell them when the site is clean again is completely unacceptable! While it is nifty that you have put out an "update", I use Firefox every single day and have not seen the update, which means my customers have not seen the update, which of course means I am losing business because it is showing my site at dangerous. This is the first time I have every run into something like this, and it was terrible all by itself without Firefox adding to an already rotten situation. WHEN IS THE UPDATE GOING TO BE APPLIED???

Comment 59

[u279076]

The update is now live but is currently throttled. Any users who manually check for updates in Firefox will be served an update. This is can be done by clicking the Help menu > About Firefox on Windows and Linux, or About Firefox through the Firefox menu on Mac OSX. Automatic updates are scheduled to be turned on Thursday after evaluating preliminary release feedback.

Comment 60

[Equipment Manager]

Mr Hughes,

I like many use and enjoy Firefox as our defacto browser. My preliminary feedback is Mozilla should release the fix for 17 ASAP and not wait for 18 and Thursday, or release 18 urgently, or revise the malware alert page to reflect the upgrade information.

When Google detects malware they shut everyone out of Chrome and Firefox (probably some agreement with Mozilla). But upon problem resolution Chrome discontinues the malware messages quickly and Firefox continues to dish it up. We fixed our problem in a matter of hours and Firefox doesn't fix the problem for a matter of days. This lag of response is highly injurious financially to some sites, and is detrimental to many, probably hundreds or thousands of sites' reputations. We may lose customers forever. Who wants to revisit a site whose pages report malware, true or not. Firefox appears irresponsible for these delays. This is a type of problem which drives people away from businesses, a problem the sites, as also does Firefox.

Thank you in advance for an urgent and prompt response.

Comment 61

[Matthias Versen [:Matti]]

Note: I'm only a community member and I don't speak for Mozilla
The fix for Firefox17 is Firefox18 and I don't think that it makes sense to start the automatic updates to Firefox18 much earlier. In case of a really big issue with the update we could really lose our Users. This bug is only a big problem for you because you failed to secure your webpage in the first place and that affected our users !
You fixed your page and we fixed the stupid bug in Firefox as fast as we could.
I'm sorry that delivering software to millions of users can't be faster.

Comment 62

[u279076]

To those of you unsatisfied with Mozilla's response and roll-out plan, please start a discussion on the mailing lists. This bug is not the appropriate platform to have these types of discussions. This is now resolved in Firefox 18 and will be fixed for your users when they update. There is no further action we can take on our part.