Closed Bug 807847 Opened 7 years ago Closed 7 years ago

Noise entries are erroneously entered into the misscache

Categories

(Toolkit :: Safe Browsing, defect, critical)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
Firefox 19
Tracking Status
firefox17 + verified
firefox18 + verified
firefox19 --- verified

People

(Reporter: gcp, Unassigned)

References

Details

Attachments

(1 file)

When we receive completion results for noise entries, we have a bug in our logic that causes them to be entered in the miss-cache. This could cause us to miss a malware/phishing warning if the user is unlucky enough to hit the site that corresponds to the noise entries before the next update arrives.

This bug is made worse by a second bug: the original entry is added again when adding noise entries. Not only does this allow a trivial defeat of the noise (the entry that you get twice is the correct one), if the user forces a reload or revisits a phishing page, it's possible he will not get any warning because the entry is in the misscache.
To reproduce, you must hit a new phishing site (that you haven't seen before), and then force the reload before any SafeBrowsing update hits. I'm able to reproduce on Nightly. The phishing site must also be one that is only in the database a single time for the URL that you are visiting, i.e. if www.foo.com and foo.com are both in the database, you won't get this problem.
Actually, it's reproducible on Nightly+bug 782106. In retrospect, it's obvious why: without bug 782106 fixed, the noise entries are encoded with the per-client key and don't correspond to anything. This also applies to the original entry, which is sent in the encoded version. Even if those replies are miss-cached, the wont correspond to any real entry and the bug isn't visible.

So this bug has been there for a while but fixing bug 782106 will expose it.
Blocks: 782106
Attachment #677742 - Flags: review?(dcamp) → review+
https://tbpl.mozilla.org/?tree=Try&rev=caa492f7913b

https://hg.mozilla.org/mozilla-central/rev/30bdcaf34723
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
Comment on attachment 677742 [details] [diff] [review]
Patch 1. Fix noise entries in misscache. Fix duplicated noise.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 782106
User impact if declined: Users hitting the same malware site twice will not see a warning.
Testing completed (on m-c, etc.): Landed on m-c last friday.
Risk to taking this patch (and alternatives if risky): Very low. Disables some caching.
Attachment #677742 - Flags: approval-mozilla-beta?
Attachment #677742 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/releases/mozilla-beta/rev/6e071059b71c
https://hg.mozilla.org/releases/mozilla-aurora/rev/da1a7707fd86

Landed this already, based on email from akeybl where we discussed that this has to go together with bug 782106 and that he was going to approve it.
Comment on attachment 677742 [details] [diff] [review]
Patch 1. Fix noise entries in misscache. Fix duplicated noise.

Approving (even though it's landed) just for bug data cleanliness and transparent process.
Attachment #677742 - Flags: approval-mozilla-beta?
Attachment #677742 - Flags: approval-mozilla-beta+
Attachment #677742 - Flags: approval-mozilla-aurora?
Attachment #677742 - Flags: approval-mozilla-aurora+
:gcp, can you please advise QA on how we might do some regression testing to ensure this is fixed?
Visit a phishing page. (Go to http://www.phishtank.com/ and try some URLs, not all of them will hit)

Examples at the current time:
mos.motowell.hu/images/1/index.html
accountinfo-paypal.de/login_scre.html

Hit refresh a few times. You should always hit the warning, and never go through to the site automatically. Close the tab, visit some other sites, enter the URL again. You should still see the warning.
Keywords: verifyme
QA Contact: ioana.budnar
Everything works as described in comment 9 on Firefox 17 beta 5 (20121106195758 - post-fix), but it also works like that on beta 2 (20121017073013 - pre-fix).

What did should go wrong specifically on pre-fix builds? I followed the details in all the above comments when checking on beta 2, but I do have one point I am not too sure about: when do SafeBrowsing updates hit?
>What did should go wrong specifically on pre-fix builds?

Exposing this bug in a reasonable manner needs bug 782106 (see comment 2). This fix was landed together with that bug, so there shouldn't have been any build where it would have been observable.

If this fix had been ineffective, you would have seen the explained behavior on the latest beta builds (because bug 782106 is fixed in them).

>I do have one point I am not too sure about: when do SafeBrowsing updates hit?

A first update will hit in a random time 5 minutes after starting the browser. Subsequent updates are usually 30-45 minutes apart. An update contains part of the database.

You can monitor the <userdata>/Local/Mozilla/Profiles/xxx.xxxx/safebrowsing folder and check the timestamps.
Considering comment 11, verified as fixed on:
Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/17.0 Firefox/17.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:17.0) Gecko/17.0 Firefox/17.0
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/17.0 Firefox/17.0
Build ID: 20121106195758

The warning was displayed at every phishing page open/refresh.
Verified as fixed on Firefox 18 beta 1:
Mozilla/5.0 (Windows NT 6.1; rv:18.0) Gecko/18.0 Firefox/18.0
Mozilla/5.0 (X11; Linux i686; rv:18.0) Gecko/18.0 Firefox/18.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:18.0) Gecko/18.0 Firefox/18.0
Verified as fixed on:
Mozilla/5.0 (Windows NT 6.1; rv:19.0) Gecko/20100101 Firefox/19.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:19.0) Gecko/20100101 Firefox/19.0
Mozilla/5.0 (X11; Linux i686; rv:19.0) Gecko/20100101 Firefox/19.0
BuildID: 20130109111322
mass remove verifyme requests greater than 4 months old
Keywords: verifyme
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.