706740 - Firefox locks up and won't shut down when visiting a phishing test page

Status: VERIFIED FIXED

(Toolkit :: Safe Browsing, defect)

Description

[KLB]

While testing Firefox's phishing protection against the Mozilla test page at https://www.mozilla.com/firefox/its-a-trap.html Firefox locked up and then failed to shut down properly. I had to go into Windows Task manager to kill the process. I was not able to repeat this behavior consistently, but I did trigger this behavior three times using two different profiles and when I had Kris Maglione try it, he crashed his browser.

I tried at least five different profiles and only replicated the crash in two profiles for a total of three crashes. Kris tried FF8 and crashed but did not crash in Nightly.

Comment 1

[KLB]

To reproduce this bug all you have to do is open the provided link in Firefox. If the bug triggers the phishing alert won't display. Rather the waiting thobber will just keep spinning.  If this happens you won't be able to open any other URL even if you've stopped the loading of the test page.  When you shut down Firefox the browser window will go away, but when you open Windows Task Manager the Firefox process is still there and the amount of memory it is consuming continues to climb.

So far I've tried this about a dozen times in five different profiles and was only able to replicate it three times. Kris Maglione reported triggering the bug on his computer once.

Comment 2

[littlespark]

I'm able to reproduce the bug using Firefox 8 on WinXP:

1) Load trap page - https://www.mozilla.com/firefox/its-a-trap.html.
2) Drag a traditional addon to the browser.
3) Restart when asked. The trap page hangs. Close Firefox.
4) Start Firefox. Load trap page again, loads ok.
5) Remove the addon from 2) in the Add-on Manager and restart.
6) The trap page hangs again. Close Firefox.

Another way from a clean profile:
(more severe, perhaps because of longer waiting time?)

1) Load trap page https://www.mozilla.com/firefox/its-a-trap.html.
2) Drag a traditional addon into the extension folder.
3) Do something in the Add-on Manager to enable a restart. Restart.
4) Trap page tries to load unsuccessfully. Choose to allow installation of add-on and restart (might need kill Firefox with Task Manager).
5) Upon restart, the addon from 2) is still saying "xxx will be enabled after you restart Firefox" in the add-on manager. The trap page is still trying to load. Close the trap page. Kill the process in Task Manager.
6) Start Firefox. Look into the Add-on manager, the addon still isn't enabled.

Once the bug triggers, the profile is no longer stable, thus any testing done on it wouldn't tell much.

Comment 3

[Johnathan Nightingale [:johnath]]

When you say "hands during safebrowsing" I hear "cc dcamp and gcp"

Comment 4

[Johnathan Nightingale [:johnath]]

(s/hands/hangs/. Sigh.)

Comment 5

[Gian-Carlo Pascutto [:gcp]]

The PrefixSet optimizations only landed in Firefox 9, so they should be innocent. 

However, I suspect this may be a duplicate of this:
https://bugzilla.mozilla.org/show_bug.cgi?id=676064
which is caused by this:
https://bugzilla.mozilla.org/show_bug.cgi?id=667075

If that's indeed the case, this sucks because  I though the bug only triggered when you resume with a phishing site in the restored session (not a normal occurrence unless you're debugging safebrowsing, i.e. affeting 1 or maybe 2 people worldwide). However, comment #1 says this can happen during normal usage, which is far far worse. 
The STR in comment #2 are known to me, you don't need to do the thing with the add-ons to trigger it, just have a phishing site in the restored session.

Reverting bug 667075 until SQLite gone entirely is trivial. But I'm scared of doing that, because that patch is trivial, and I have no idea what the underlying issue causing this hang is.

Comment 6

[KLB]

In my case I was able to trigger the bug without doing any of the steps Little Spark did. I simply opened the link in a new tab and everything hung up. I believe in Kris' case, Firefox hung after he simply opened the link in Firefox from an email I sent to the internal AMO editors mailing list asking for others to confirm this bug.

I am a little concerned that this should be classified as a security bug since it can cause Firefox to hang and become unresponsive. It would be highly undesirable for any phishing sites to try and exploit this bug in an effort to trick users into disabling phishing protections.

BTW, Little Spark, Kris & myself are all AMO Editors.

Comment 7

[littlespark]

I wasn't able to trigger the bug at all in Firefox 8, thus went the extra steps to produce it. However it did hang 10.0a2 and now this profile is loading poorly once in a while.

OK KLB, got what you meant.

Comment 8

[Gian-Carlo Pascutto [:gcp]]

This looks threadsafe to me:

http://dxr.mozilla.org/mozilla/mozilla-central/security/manager/ssl/src/nsRandomGenerator.cpp.html#l52

This has locks, but the locks aren't used in some circumstances:

https://mxr.mozilla.org/mozilla-central/source/security/nss/lib/pk11wrap/pk11slot.c#2171

Marking everything threadsafe was done here:
https://bugzilla.mozilla.org/show_bug.cgi?id=607115#c9

Was anything overlooked there? I have no clue for any other potential causes.

Comment 9

[KLB]

I don't seem to be having any profile corruption issues. Neither of the two profiles I was able to trigger this bug from are having problems.

I did just force the bug in Aurora. I first tried to just open the URL in Aurora by pasting the link into the address bar and the page loaded just fine. I did this a couple of times having restarted Aurora between tries. No problems.

I then opened the offending link, went to about:addons and enabled a disabled addon and restarted. Upon restart the page in question failed to load, with the favicon thobber left spinning.  Aurora then spiked to all available processor time (98%) and completely hung up. I had to use task manager to kill the process.

Comment 10

[KLB]

Since I was able to cause Aurora & FF8 to lock up 100% of the time upon restart by enabling/disabling different addons and clicking on the restart link with the offending page open in a tab, I tried some experimenting to find other ways to crash the browser.

I tried shutting down Aurora & FF8 with the offending page in one tab and about:addons in another tab and then starting it back up (going through the profile manager). I also tried this while using http://news.google.com instead of about:addons. In all attempts, Aurora started just fine, however FF8 locked up once.

I was able to get Aurora & FF8 to again lock up 100% of the time with the offending page in one tab and http://news.google.com in another tab by restarting Aurora/FF using the "restart button" extension (https://addons.mozilla.org/en-US/firefox/addon/restart-button/?src=search), which just mimics the restart link when addons are enabled/disabled.

Comment 11

[Gian-Carlo Pascutto [:gcp]]

Attachment: Patch 1. Backout bug 667075

This is the patch to back out the change that likely causes this. Can someone verify?

https://tbpl.mozilla.org/?tree=Try&rev=37d5b689c35f
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-37d5b689c35f/

Comment 12

[KLB]

I installed http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-37d5b689c35f/try-win32/firefox-11.0a1.en-US.win32.installer.exe from your link and tried to force the build to hang using the techniques I detailed in comment #10 and could not get this build to hang. I tried to force the hang about eight times using the methods that previously gave me a 100% hang rate. Given this bugs quirky behavior, this isn't a definitive answer, but it is promising.

Comment 13

[Dave Camp (:dcamp)]

Comment on attachment 578392 [details] [diff] [review]
Patch 1. Backout bug 667075

This is unfortunate, but it is reverting to stuff that worked for years, so it's safe.

Comment 14

[Gian-Carlo Pascutto [:gcp]]

Requesting Aurora/Beta approval.

- This issue is seen in all the deployed releases (8/9/10/11).
- STR are included here, they're not 100% but still give a quite high likelihood of detecting the issue.
- The patch is effectively a backout of a previous change.

Security impact: sites containing malware/phishing can hang the browser and prevent proper shutdown. If this is judged serious, release approval should be requested.

Risk factors: We don't understand why the new code doesn't work and the old one did. It might be a threading issue in NSS?

Comment 15

[KLB]

In case it matters, I just tried my normal nightly on a clean profile and could not reproduce this bug in that nightly. My normal nightly did update just before I could do anything so I don't know if that makes a difference.

Comment 16

[KLB]

Update, I just got Nightly (2011-12-01) to crash on this bug. Oddly when I started Nightly again after the crash the phishing warning didn't intercept the page and I landed on the real page instead.

Comment 17

[Gian-Carlo Pascutto [:gcp]]

https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=4be41994deb7

Comment 18

[Gian-Carlo Pascutto [:gcp]]

This reintroduced a PRBool which caused the inbound tree to burn. Surprisingly, this didn't cause a failure on the tryserver?

https://hg.mozilla.org/integration/mozilla-inbound/rev/aeb035da5328

Comment 20

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4be41994deb7
https://hg.mozilla.org/mozilla-central/rev/aeb035da5328

Comment 21

[Alex Keybl [:akeybl]]

If this is considered low risk, please nominate for aurora/beta approval and be prepared to land later today after the 2PM triage meeting if approved.

Comment 22

[Alex Keybl [:akeybl]]

I should have specified - 2PM PT 12/6

Comment 23

[Gian-Carlo Pascutto [:gcp]]

As per comment 13, this backs out a code cleanup and brings us back to a state that has been working correctly for a long time.

Comment 24

[Alex Keybl [:akeybl]]

Comment on attachment 578392 [details] [diff] [review]
Patch 1. Backout bug 667075

[Triage Comment]
Approved for Aurora and Beta. Do we plan to re-open 667075 now that this has been backed out on m-c?

Comment 25

[Gian-Carlo Pascutto [:gcp]]

>Do we plan to re-open 667075 now that this has been backed out on m-c?

No. Bug 673470 is close to landing and rips out the entire SQLite infrastructure.

Comment 26

[Gian-Carlo Pascutto [:gcp]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1292439858c4
https://hg.mozilla.org/releases/mozilla-beta/rev/fcec1cc2c18f

Comment 27

[Ioana (away)]

I verified this issue on Firefox 8.0.1, 9 beta 5, 10a2 and 11a1. 

On 
Mozilla/5.0 (Windows NT 5.1; rv:10.0a2) Gecko/20111207 Firefox/10.0a2
Mozilla/5.0 (Windows NT 5.1; rv:11.0a1) Gecko/20111208 Firefox/11.0a1
this issue doesn't reproduce anymore.

On
Mozilla/5.0 (Windows NT 5.1; rv:8.0.1) Gecko/20100101 Firefox/8.0.1 (20111120135848)
Mozilla/5.0 (Windows NT 5.1; rv:9.0) Gecko/20100101 Firefox/9.0 (20111206234556)
it still reproduces - firefox hangs. I have to close the process to close it.

STR:
1. Open Firefox with a clean profile.
2. Load https://www.mozilla.com/firefox/its-a-trap.html in a Firefox tab.
3. Open the Add-ons Manager.
4. Drag an extension into the Add-ons Manager (I used FlashGot).
5. Install it and restart Fx when requested.
6. Remove the add-on and restart Fx when requested.

Firefox 9 almost always hangs at step 6, while Firefox 8.0.1 many times hangs from step 4.

Please let me know if you want me to reopen this bug or file a different one.

Comment 28

[Ioana (away)]

Regarding comment 27, Firefox 8.0.1 many times hangs from step 5, not 4.

Comment 29

[u279076]

Ioana, given the landing comments on this bug I would expect this to be fixed in:

 * Nightly from 2011-12-02 and onward
 * Aurora from 2011-12-08 and onward
 * Beta built after 2011-12-08 (ie. 9.0b6 -- the next beta)
 * Release built after 2011-12-08 (ie. 9.0 -- the next release)

As 8.0.1 and 9.0b5 were both built before this fix landed, you should not expect to see it fixed in those builds.

Based on your testing, we can call this VERIFIED on Nightly and Aurora, but we will need to reverify on 9.0b6 next week.

Comment 30

[Ioana (away)]

Verified as fixed using the steps from comment 27 on:
Mozilla/5.0 (Windows NT 5.1; rv:9.0) Gecko/20100101 Firefox/9.0