Closed
Bug 526194
Opened 15 years ago
Closed 15 years ago
Switching in/out of Private Browsing mode too fast will restore windows tons of times and freeze/crash the browser [@ nsXULDocument::ResumeWalk | nsXULDocument::OnStreamComplete]
Categories
(Firefox :: Private Browsing, defect, P2)
Tracking
()
RESOLVED
FIXED
Firefox 3.7a1
Tracking | Status | |
---|---|---|
status1.9.2 | --- | beta4-fixed |
status1.9.1 | --- | wontfix |
People
(Reporter: whimboo, Assigned: ehsan.akhgari)
References
(Depends on 2 open bugs)
Details
(5 keywords, Whiteboard: [sg:dos][mozmill])
Crash Data
Attachments
(4 files, 1 obsolete file)
41.65 KB,
text/plain
|
Details | |
1.05 MB,
text/plain
|
Details | |
35.68 KB,
patch
|
vlad
:
review+
|
Details | Diff | Splinter Review |
35.55 KB,
patch
|
Details | Diff | Splinter Review |
Found by running the Mozmill tests with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.5) Gecko/20091102 Firefox/3.5.5 ID:20091102134505 Users who turn off the warning when switching into Private Browsing mode can enter/leave the PB mode really quick by holding down the Cmd/Ctrl+Shift+P keys. If another windows have been opened before those will be restored multiple times. Holding down the keys for about 3-4s will hang/freeze Firefox for a long time with a high cpu load and completely fill the memory. After a couple of minutes waiting I get the warning that the virtual memory minimum limit is too low. The only way to close the browser is to kill the process via the task manager. After a couple of tries the browser crashed now. There are failures with sending the report and those are not listed under about:crashes afterward. I will try to get it into the debugger instead. Starting Firefox again with the same profile will result in the same state because session restore wants to restore all of the >100 windows again. Steps: 1. Start the browser and start the Private Browsing mode 2. Uncheck the checkbox so the PB mode will be started immediately 3. Leave the Private Browsing mode 4. Hold down Cmd/Crtl+Shift+P for about 3-4s (or longer to get the crash)
Flags: blocking-firefox3.6?
Reporter | ||
Updated•15 years ago
|
Summary: Switching in/out of Private Browsing mode too fast will restore windows multiple times → Switching in/out of Private Browsing mode too fast will restore windows multiple times and freeze/crash the browser
Reporter | ||
Comment 1•15 years ago
|
||
Results from the exception analysis with windbg below. Complete stack will be attached shortly. Marking as security sensitive. FAULTING_IP: xul!ns_if_addref<nsIContent *>+4 [e:\builds\moz2_slave\win32_build\build\obj-firefox\dist\include\xpcom\nsisupportsutils.h @ 114] 102868d0 8b08 mov ecx,dword ptr [eax] EXCEPTION_RECORD: ffffffff -- (.exr 0xffffffffffffffff) ExceptionAddress: 102868d0 (xul!ns_if_addref<nsIContent *>+0x00000004) ExceptionCode: c0000005 (Access violation) ExceptionFlags: 00000000 NumberParameters: 2 Parameter[0]: 00000000 Parameter[1]: 00690072 Attempt to read from address 00690072
Group: core-security
Summary: Switching in/out of Private Browsing mode too fast will restore windows multiple times and freeze/crash the browser → Switching in/out of Private Browsing mode too fast will restore windows tons of times and freeze/crash the browser [@ nsXULDocument::ResumeWalk | nsXULDocument::OnStreamComplete]
Reporter | ||
Comment 2•15 years ago
|
||
Reporter | ||
Comment 3•15 years ago
|
||
This is the sessionstore.js which gets created when following the steps in comment 0. It has about 2000 windows.
Assignee | ||
Comment 5•15 years ago
|
||
Oh, I was already looking at this. This seems like a serious issue which I think should block. I have a half-baked patch which I think would be ready by the end of today. The problem is that the user is allowed to change the private browsing status before another transition is fully finished. I have a fix which disables the private browsing command until the new session is fully restored. We were already handling a weaker type of this protection (not allowing to recursively set the private browsing status), but this fix should make any such future problems go away as well. This needs a sessionstore fix for which I'm going to file a bug in a moment.
Assignee | ||
Comment 6•15 years ago
|
||
Of course, the fix that I'm talking about is only restricted to the private browsing code. Basically we make it impossible for this issue to surface through private browsing. The security impact of this is a denial of service, in which an external app can sent successive Ctrl+Shift+P sequences to the Firefox process which would effectively result in: a) user's browser crashing b) other apps running on the desktop face similar problems (crash or DOS) because of exhaustion of physical memory c) dataloss in the sense that the user's session would become impossible to restore unless they have the expertise to manually edit their sessionstore.js
Keywords: dataloss
Assignee | ||
Comment 7•15 years ago
|
||
My work on bug 526613 is done now, and I'll have the patch ready tomorrow morning...
Assignee | ||
Comment 8•15 years ago
|
||
Patch + unit tests. This needs to be applied over the patch in bug 526613.
Attachment #410599 -
Flags: review?(mconnor)
Updated•15 years ago
|
Flags: blocking-firefox3.6? → blocking-firefox3.6+
Priority: -- → P2
Assignee | ||
Comment 9•15 years ago
|
||
I pushed the patch to the try server which revealed some test failings. Most of them were test problems, and one was actually a minor bug in my patch, which I fixed them all. Here's the new patch which should pass the entire test suite.
Attachment #410599 -
Attachment is obsolete: true
Attachment #410907 -
Flags: review?(mconnor)
Attachment #410599 -
Flags: review?(mconnor)
Reporter | ||
Updated•15 years ago
|
Status: NEW → ASSIGNED
Updated•15 years ago
|
Whiteboard: [needs review mconnor]
Updated•15 years ago
|
Whiteboard: [needs review mconnor] → [needs review mconnor][sg:dos]
Assignee | ||
Comment 10•15 years ago
|
||
Bug 528440 and bug 528451 together can affect the fix for this bug in extreme cases. I'm going to request those bugs to block the release of Firefox 3.6 as well.
Assignee | ||
Comment 11•15 years ago
|
||
mconnor: ping?
Assignee | ||
Updated•15 years ago
|
Attachment #410907 -
Flags: review?(mconnor) → review?(vladimir)
Attachment #410907 -
Flags: review?(vladimir) → review+
Assignee | ||
Comment 12•15 years ago
|
||
Here is another version of the patch which uses the disabled property. Which version do you prefer? Gavin suggested it on IRC...
Attachment #412898 -
Flags: review?(vladimir)
Assignee | ||
Comment 13•15 years ago
|
||
Comment on attachment 412898 [details] [diff] [review] Patch (v2.1) Actually I'm going to go with the version 2 of the patch, since it's already reviewed, and other places in browser.js use the same convention.
Attachment #412898 -
Flags: review?(vladimir)
Assignee | ||
Comment 14•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/9ec2968743dc
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [needs review mconnor][sg:dos] → [sg:dos]
Target Milestone: --- → Firefox 3.7a1
Assignee | ||
Updated•15 years ago
|
Flags: in-testsuite+
Assignee | ||
Comment 15•15 years ago
|
||
I landed a test bustage fix as <http://hg.mozilla.org/mozilla-central/rev/60ad411785a3>.
Assignee | ||
Comment 16•15 years ago
|
||
Landed both patches combined on 1.9.2: <http://hg.mozilla.org/releases/mozilla-1.9.2/rev/da8f87127f2d>
status1.9.2:
--- → final-fixed
Reporter | ||
Updated•15 years ago
|
Whiteboard: [sg:dos] → [sg:dos][mozmill]
Reporter | ||
Comment 17•15 years ago
|
||
I would like to verify this fix but because of bug 529669 I'm not able to. I will have to wait until bug 529669 is fixed.
Updated•13 years ago
|
Group: core-security
Updated•13 years ago
|
Crash Signature: [@ nsXULDocument::ResumeWalk | nsXULDocument::OnStreamComplete]
You need to log in
before you can comment on or make changes to this bug.
Description
•