Closed Bug 842166 Opened 7 years ago Closed 7 years ago

crash [@ mozilla::ScrollbarActivity::CancelActivityFinishedTimer() ] in destroyed frame

Categories

(Core :: Layout, defect, critical)

18 Branch
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla22
Tracking Status
firefox18 --- wontfix
firefox19 --- wontfix
firefox20 --- wontfix
firefox21 --- wontfix
firefox22 --- fixed
firefox-esr17 - wontfix
b2g18 - wontfix
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- wontfix

People

(Reporter: w3bd3vil, Assigned: mats)

References

Details

(5 keywords, Whiteboard: [adv-main22-])

Crash Data

Attachments

(4 files, 5 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:18.0) Gecko/20100101 Firefox/18.0
Build ID: 20130201065344

Steps to reproduce:

<html>
    <head>
        <style>
            li{
             display: table-footer-group;
             }
        </style>
        <meta HTTP-EQUIV="Cache-Control" content="no-cache" />
    </head>
    <body>
        <script>
            <ins>
            </ins>
        </script>
        <li contenteditable="true">
        </li>
        <object type="checkbox">
        </object>
        <select>
        </select>
    </body>
</html>


Actual results:

0:000> r
eax=00000000 ebx=f0de7fff ecx=7e26b03a edx=00000000 esi=f0de7fff edi=03120280
eip=6786c8d7 esp=0053a698 ebp=0053a774 iopl=0         nv up ei ng nz na pe nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00210286
xul!nsListScrollSmoother::Stop+0x5:
6786c8d7 8b4608          mov     eax,dword ptr [esi+8] ds:0023:f0de8007=????????

0:000> kb
ChildEBP RetAddr  Args to Child              
0053a698 67a1b6af f0de7fff 05004e70 67629daa xul!nsListScrollSmoother::Stop+0x5 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\layout\xul\base\src\nslistboxbodyframe.cpp @ 126]
0053a6a4 67629daa f0de7fff 04d605fc 05007fb0 xul!mozilla::ScrollbarActivity::ActivityOccurred+0xb [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\layout\generic\scrollbaractivity.cpp @ 22]
0053a774 6745992a 04d605fc 00000000 00000000 xul!nsGfxScrollFrameInner::SetCoordAttribute+0x1a345a [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\layout\generic\nsgfxscrollframe.cpp @ 3634]
0053a7e0 67440f67 0641a800 076a4480 0641a800 xul!nsGfxScrollFrameInner::ReflowFinished+0x1ea [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\layout\generic\nsgfxscrollframe.cpp @ 3400]
0053a7f8 6741eb04 076a4400 00000000 0641a800 xul!PresShell::HandlePostedReflowCallbacks+0xe7 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\layout\base\nspresshell.cpp @ 3708]
0053a850 6741e777 00000000 076a4480 06398c00 xul!PresShell::ProcessReflowCommands+0x254 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\layout\base\nspresshell.cpp @ 7686]
0053a8fc 675947ae 00000005 05007da0 076a57a0 xul!PresShell::FlushPendingNotifications+0x1d7 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\layout\base\nspresshell.cpp @ 3878]
0053a924 6734329f 00000005 0053a9b0 00000000 xul!nsDocument::FlushPendingNotifications+0xce [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\content\base\src\nsdocument.cpp @ 6527]
0053a94c 6759e1d2 076a57a0 0053a9b0 0053a988 xul!nsGenericElement::GetBoundingClientRect+0x5e [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\content\base\src\nsgenericelement.cpp @ 797]
0053a960 673cebba 076a5888 00000044 00000001 xul!NS_InvokeByIndex_P+0x27 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\xpcom\reflect\xptcall\src\md\win32\xptcinvoke.cpp @ 71]
0053ab44 673cc6ba 00000000 00000000 0053ac04 xul!XPCWrappedNative::CallMethod+0x39a [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\js\xpconnect\src\xpcwrappednative.cpp @ 2383]
0053abcc 68a435b9 05026000 00000000 03b10188 xul!XPC_WN_CallMethod+0xaa [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\js\xpconnect\src\xpcwrappednativejsops.cpp @ 1469]
0053ac28 68a4786f 05026000 00000000 03b10198 mozjs!js::InvokeKernel+0x59 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\js\src\jsinterp.cpp @ 365]
0053b778 68a42f7e 03b100b0 00000000 00000000 mozjs!js::Interpret+0x9ef [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\js\src\jsinterp.cpp @ 2473]
0053b7ec 68a43791 05026000 053a8634 03b100b0 mozjs!js::RunScript+0x8e [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\js\src\jsinterp.cpp @ 322]
0053b848 68a43488 05026000 00000000 03b100a8 mozjs!js::InvokeKernel+0x231 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\js\src\jsinterp.cpp @ 376]
0053b888 68a3a751 05026000 0053b8b0 0053b8cc mozjs!js::Invoke+0x128 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\js\src\jsinterp.cpp @ 409]
0053b8bc 673cfdd4 05026000 0539db20 053a8620 mozjs!JS_CallFunctionValue+0x41 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\js\src\jsapi.cpp @ 5889]
0053bb1c 675949a7 05b80190 0599c200 00000003 xul!nsXPCWrappedJSClass::CallMethod+0x494 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\js\xpconnect\src\xpcwrappedjsclass.cpp @ 1434]
0053bb44 6759e2d0 0599c200 00000003 031d9418 xul!nsXPCWrappedJS::CallMethod+0x47 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\js\xpconnect\src\xpcwrappedjs.cpp @ 580]
0053bc04 6759e344 059bae10 00000003 0053bc2c xul!PrepareAndDispatch+0xfa [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\xpcom\reflect\xptcall\src\md\win32\xptcstubs.cpp @ 85]
0053bc20 673f0fd6 059bae10 07696370 0623fc40 xul!SharedStub+0x16 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\xpcom\reflect\xptcall\src\md\win32\xptcstubs.cpp @ 113]
0053bc78 673f1507 05b76ec0 05961400 0623fc40 xul!nsEventListenerManager::HandleEventInternal+0x2e6 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\content\events\src\nseventlistenermanager.cpp @ 964]
0053bcc4 67405a40 0053bd00 00000006 00000000 xul!nsEventTargetChainItem::HandleEventTargetChain+0x2a7 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\content\events\src\nseventdispatcher.cpp @ 286]
0053bd84 67437017 076a57a0 05961400 0623fc40 xul!nsEventDispatcher::Dispatch+0x500 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\content\events\src\nseventdispatcher.cpp @ 636]
0053bdb8 67452989 076a57a0 00000000 05961400 xul!nsEventDispatcher::DispatchDOMEvent+0x67 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\content\events\src\nseventdispatcher.cpp @ 691]
0053bdd8 6737bfed 00000000 07696370 0053be0b xul!nsINode::DispatchEvent+0x59 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\content\base\src\nsinode.cpp @ 1078]
0053be0c 68a435b9 05919c00 00000001 03b10060 xul!nsIDOMEventTarget_DispatchEvent+0x83 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\obj-firefox\js\xpconnect\src\dom_quickstubs.cpp @ 10400]
0053be68 68a4786f 05919c00 00000000 03b10070 mozjs!js::InvokeKernel+0x59 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\js\src\jsinterp.cpp @ 365]
0053c9a8 68a42f7e 03b10020 00000000 00000000 mozjs!js::Interpret+0x9ef [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\js\src\jsinterp.cpp @ 2473]
0053ca1c 68a43791 05919c00 0459ca94 03b10020 mozjs!js::RunScript+0x8e [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\js\src\jsinterp.cpp @ 322]
0053ca78 68a43488 05919c00 00000000 03b10020 mozjs!js::InvokeKernel+0x231 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\js\src\jsinterp.cpp @ 376]
0053cab8 68a3a751 05919c00 0053cae0 0053cafc mozjs!js::Invoke+0x128 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\js\src\jsinterp.cpp @ 409]
0053caec 6744ce71 05919c00 0458e4c0 0459ca80 mozjs!JS_CallFunctionValue+0x41 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\js\src\jsapi.cpp @ 5889]
0053cb50 6744c059 05ba69b0 076a57a0 062c1f00 xul!nsXBLProtoImplAnonymousMethod::Execute+0x131 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\content\xbl\src\nsxblprotoimplmethod.cpp @ 331]
0053cb68 674046e8 00000000 06398c00 0053cb8c xul!nsXBLBinding::ExecuteAttachedHandler+0x49 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\content\xbl\src\nsxblbinding.cpp @ 1103]
0053cb8c 673feb2b 050072f8 05b0b3f0 00000008 xul!nsDocument::MaybeEndOutermostXBLUpdate+0x178 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\content\base\src\nsdocument.cpp @ 4152]
0053cbac 6744075a 00b031f0 05b0b3f0 063d2bfc xul!nsHTMLDocument::EndUpdate+0x7b [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\content\html\document\src\nshtmldocument.cpp @ 2353]
0053cbc0 6740f7b5 00b86ec4 04c55320 00b20100 xul!nsHtml5TreeOpExecutor::EndDocUpdate+0x9a [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\parser\html\nshtml5treeopexecutor.h @ 248]
0053cbf0 67329109 6742e625 04c55320 00b410e0 xul!nsHtml5TreeOpExecutor::RunFlushLoop+0x1b5 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\parser\html\nshtml5treeopexecutor.cpp @ 593]
0053cbf4 6742e625 04c55320 00b410e0 00b0e3d0 xul!nsHtml5ExecutorReflusher::Run+0xc [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\parser\html\nshtml5treeopexecutor.cpp @ 67]
0053cc60 6754879f 00b20101 00000000 0053cc9c xul!nsThread::ProcessNextEvent+0x1b5 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\xpcom\threads\nsthread.cpp @ 626]
0053cc94 6753152c 00b41001 7e757cf6 00b20100 xul!mozilla::ipc::MessagePump::Run+0x5f [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\ipc\glue\messagepump.cpp @ 82]
0053cccc 675314d4 00000001 67496600 00000000 xul!MessageLoop::RunHandler+0x21 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\ipc\chromium\src\base\message_loop.cc @ 209]
0053cce8 67529c1a 80000000 00b86ec0 67531580 xul!MessageLoop::Run+0x15 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\ipc\chromium\src\base\message_loop.cc @ 183]
0053ccf4 67531580 00b86ec0 00b172c0 00db0000 xul!nsBaseAppShell::Run+0x34 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\widget\xpwidgets\nsbaseappshell.cpp @ 165]
0053ec48 67576e5b 00b86ec0 00b172c0 672fe4e4 xul!nsAppShell::Run+0x4e [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\widget\windows\nsappshell.cpp @ 232]
0053ec54 672fe4e4 00b172c0 00000000 6ccf10a0 xul!nsAppStartup::Run+0x1e [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\toolkit\components\startup\nsappstartup.cpp @ 291]
0053ed24 675155c3 0053ed60 00db32e8 0053ed60 xul!XREMain::XRE_mainRun+0x405 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\toolkit\xre\nsapprunner.cpp @ 3794]
0053ed40 6753d505 0053ed60 00000001 006d3db0 xul!XREMain::XRE_main+0xde [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\toolkit\xre\nsapprunner.cpp @ 3860]
0053ee58 00db1742 00000001 006d3db0 00db32e8 xul!XRE_main+0x30 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\toolkit\xre\nsapprunner.cpp @ 3935]
0053f930 00db1a64 00000001 006d1160 006d11e0 firefox!wmain+0x742 [e:\builds\moz2_slave\rel-m-rel-w32-bld\build\toolkit\xre\nswindowswmain.cpp @ 105]
0053f974 77351866 7fa77000 0053f9c4 77d468f1 firefox!__tmainCRTStartup+0x122 [f:\dd\vctools\crt_bld\self_x86\crt\src\crtexe.c @ 552]
0053f980 77d468f1 7fa77000 61ae5788 00000000 KERNEL32!BaseThreadInitThunk+0xe
0053f9c4 77d4689d ffffffff 77dd5aea 00000000 ntdll!__RtlUserThreadStart+0x4a
Firefox 21.0a1 Crash Report [@ mozilla::ScrollbarActivity::CancelActivityFinishedTimer() ] 

win 7 bp-9d080b36-8a8d-40d0-bbb5-c30232130218 exploitability rating - low
linux  bp-73ceebf8-3b2f-4100-80eb-9d8652130218
Crash Signature: [@ mozilla::ScrollbarActivity::CancelActivityFinishedTimer() ]
Component: Untriaged → Layout
OS: Windows 8 → All
Product: Firefox → Core
Hardware: x86_64 → All
Attached file testcase
Backing out bug 800018 makes it not crash.
Blocks: 800018
Severity: normal → critical
Attached file stack (obsolete) —
So I guess the real culprit is bug 778810 that added the use of mScrollbarActivity
in a place where 'this' isn't safe to use.
http://hg.mozilla.org/mozilla-central/rev/c449b548784e#l5.92
Blocks: 778810
BTW, does nsGfxScrollFrameInner::UpdateScrollbarPosition() have the same problem?
Which affects ScrollToImpl, ScrollToWithOrigin, ScrollBy ...
Bug 778810 landed in 17, so setting flags appropriately.
Status: UNCONFIRMED → NEW
Ever confirmed: true
I think frame poisoning mitigates this from being exploitable.
Attached patch fix, v1 (obsolete) — Splinter Review
https://tbpl.mozilla.org/?tree=Try&rev=634c1ca11579
Assignee: nobody → matspal
Attached patch fix, v2 (obsolete) — Splinter Review
Make SetCoordAttribute static, and return true if the attribute value changed.
Move the "mScrollbarActivity->ActivityOccurred()" to the (two) consumers.
Add WeakFrame.IsAlive() checks to everything that reach SetCoordAttribute or
SetScrollbarEnabled.  (I considered using nsAutoScriptBlocker instead but
I suspect it would break stuff if I add it on a high enough level that it
would make a difference)

https://tbpl.mozilla.org/?tree=Try&rev=9f8991c9e8f2

Hopefully this will also fix our nsGfxScrollFrameInner::ScrollTo* crashes.
Attachment #715280 - Attachment is obsolete: true
Attachment #715755 - Flags: review?(roc)
I don't like this approach, it seems too fragile to have to deal with frame destruction in all these places.

Can we make ActivityOccurred run off AddScriptRunner?
Me neither ;-) but there are numerous other potential crash bugs here, not just
the reference to mScrollbarActivity.  Even with ActivityOccurred running off an
AddScriptRunner the amount of added WeakFrame checks would be roughly the same.
We can only avoid all that by calling SetAttr with aNotify=false, but that's not
possible AIUI.  Or is it?
What exactly can those SetAttrs trigger that can reconstruct frames?
Just a bump, been a while since there has been any update on this.
Our current understanding is that frame-poisoning is an effective mitigation so not awarding a bug bounty
Flags: sec-bounty? → sec-bounty-
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #16)
> What exactly can those SetAttrs trigger that can reconstruct frames?

I think the attached stack already answers that to some degree.
In the absence of a ScriptBlocker, which I assume we can't use here,
it executes script runners, which may cause document mutations causing
mutation observer methods to run.

My experience is that any call that has aNotify=true generally means it
can destroy any (non-ref-counted) layout data and ref-counted data too
unless you have an explicit kungFuDeathGrip on it.  I would love to be
corrected on this if it's not true.

Maybe Boris can give us a more precise answer though?
Flags: needinfo?(bzbarsky)
The basic problem I know of is in fact that stack: where a totally unrelated EndUpdate suddenly triggers editor instantiation....
Flags: needinfo?(bzbarsky)
Yes, basically anywhere we notify something, there is a chance of the editor being instantiated and that can trigger all sorts of DOM mutations and frame constructions.  Mats, why can't we just avoid notifying here?
IIRC, we already tried not notifying in some older crash bug and it broke scrolling
somehow.  I have a vague memory of CurPosAttributeChanged not being called when
we wanted...  I can try again and see what breaks I suppose, problem is our
test coverage here is not great.
Attached file stack (obsolete) —
Boris, what stuff do we have that runs off an nsAsyncDOMEvent?
It seems it can create all sorts of havoc...  note the SetAttr
in stack frame #72.
Um...  we run all sorts of stuff from nsAsyncDOMEvent, either via runnables or scriptrunners.

The stack there around #72 is just showing that SetAttr can cause an nsAsyncDOMEvent to be posted as a scriptrunner, right?
Yeah, I guess it's not a problem unless we run into something that creates them.
Attached patch SetAttr with aNotify=false (obsolete) — Splinter Review
Ehsan, is this what you suggested?  It makes the viewport scrollbar unusable.
Comment on attachment 727498 [details] [diff] [review]
SetAttr with aNotify=false

Yes, this is what I was suggesting... Sigh :(
How about making sure that nsTextEditRules::CreateMozBR does not notify?  You would probably need to propagate down a boolean to the CreateElementTxn to ask it to not notify if it's set, but I don't know what that could break either... :/  However, if that passes all of our tests, then I don't feel extremely bad about taking that patch.
As a side note, can you guys who understand the issue fix up the bug summary to be more specific what this one is about?
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #28)
> How about making sure that nsTextEditRules::CreateMozBR does not notify? 
> You would probably need to propagate down a boolean to the CreateElementTxn
> to ask it to not notify if it's set

There's no way to prevent nsINode::InsertBefore from notifying mutation
observers, afaict.  And even if there is, I suspect it would break stuff.
Not creating the frame for the inserted fake <br> seems like it would
cause blocks with a caret to have zero height?
Summary: Firefox 18.0.2 - Memory Corruption → crash [@ mozilla::ScrollbarActivity::CancelActivityFinishedTimer() ] in destroyed frame
(In reply to Boris Zbarsky (:bz) from comment #20)
> The basic problem I know of is in fact that stack: where a totally unrelated
> EndUpdate suddenly triggers editor instantiation....

If editor instantiation is the *only* way SetAttr can cause unrelated
frames to be destroyed then I guess we could get away with addressing
just that problem.
I'm investigating DeferredContentEditableCountChange,
MaybeEditingStateChanged and friends...
Attachment #715755 - Attachment is obsolete: true
Attachment #715755 - Flags: review?(roc)
A few WIPs, each fixes the crash individually...

Make editor instantiation async:
https://tbpl.mozilla.org/?usebuildbot=1&tree=Try&rev=f8b7f5773082

Still sync, but suppress MaybeEditingStateChanged script runners from a nested scope:
https://tbpl.mozilla.org/?usebuildbot=1&tree=Try&rev=59fa830e0d83

Script blocker in nsGfxScrollFrameInner::ReflowFinished:
https://tbpl.mozilla.org/?usebuildbot=1&tree=Try&rev=42cce4513fb7

Ehsan, do you see any reason why the first wouldn't work?

I think 1 or 2 is required, 3 is additional for extra safety just in case
editor instantiation isn't the only way.
Attachment #727489 - Attachment is obsolete: true
(In reply to Mats Palmgren [:mats] from comment #33)
> Ehsan, do you see any reason why the first wouldn't work?

No, but it's a bit hard to reason about all of the things which could go wrong.  Let's see what TBPL thinks.
The async approach didn't work at all (I guess script is using editor stuff
before the editor was instantiated, execCommand and the like).

Patch 2 and 3 each passed tests individually so let's go with 2 + 3.
Comment on attachment 728371 [details] [diff] [review]
Ignore MaybeEditingStateChanged() during nsDocument::EndUpdate since nsHTMLDocument::EndUpdate will do it

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

You forgot to initialize this new member in the ctor.  r=me with that.
Attachment #728371 - Flags: review?(ehsan) → review+
Attachment #728372 - Flags: review?(ehsan) → review+
> You forgot to initialize this new member in the ctor.  r=me with that.

Thanks, but it's not necessary:
http://mxr.mozilla.org/mozilla-central/source/content/html/document/src/nsHTMLDocument.cpp#195
Attached file stack
For the record, the GetBoundingClientRect in stack frame #73 came from
browser.js according DumpJSStack() in a debugger.

I wonder if it could be harmful to performance to flush layout like that.
Attachment #715096 - Attachment is obsolete: true
(In reply to Mats Palmgren [:mats] from comment #39)
> > You forgot to initialize this new member in the ctor.  r=me with that.
> 
> Thanks, but it's not necessary:
> http://mxr.mozilla.org/mozilla-central/source/content/html/document/src/
> nsHTMLDocument.cpp#195

Heh, nasty!

(In reply to Mats Palmgren [:mats] from comment #40)
> Created attachment 728491 [details]
> stack
> 
> For the record, the GetBoundingClientRect in stack frame #73 came from
> browser.js according DumpJSStack() in a debugger.
> 
> I wonder if it could be harmful to performance to flush layout like that.

Yeah, flushing layout is usually harmful for performance.  Please file a bug on that
Filed bug 854054.
(In reply to Mats Palmgren [:mats] from comment #19)
> My experience is that any call that has aNotify=true generally means it
> can destroy any (non-ref-counted) layout data and ref-counted data too
> unless you have an explicit kungFuDeathGrip on it.  I would love to be
> corrected on this if it's not true.

That's probably true in general but here we're setting an attribute on specific anonymous content elements which shouldn't have crazy event listeners or malicious XBL or anything like that attached.
Please nominate this bug for ESR tracking and change the status to affected if you all disagree with wontfixing. Seems like the worst case scenario here isn't very bad.
I think frame poisoning mitigates this from being exploitable so I agree with
wontfix for esr17 and b2g18.  The fix is low-risk though so it seems worth
taking on Aurora.  I just checked - it applies cleanly and fixes the crash.
There's 250 crash incidents matching the signature on crash-stats in the past
4 week period.
Comment on attachment 728372 [details] [diff] [review]
Block scripts during nsGfxScrollFrameInner::ReflowFinished()

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 778810
User impact if declined: crash (non-exploitable)
Testing completed (on m-c, etc.): on m-c since 2013-03-23
Risk to taking this patch (and alternatives if risky): low risk
String or UUID changes made by this patch: none

Request is for both patches.
Attachment #728372 - Flags: approval-mozilla-aurora?
Comment on attachment 728372 [details] [diff] [review]
Block scripts during nsGfxScrollFrameInner::ReflowFinished()

If we wontfix for ESR, we should just let the fix ride the trains for sake of consistency.
Attachment #728372 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
Target Milestone: --- → mozilla22
Group: core-security
Whiteboard: [adv-main22-]
Landed the crashtest:
https://hg.mozilla.org/integration/mozilla-inbound/rev/70c9239286e5
Flags: in-testsuite? → in-testsuite+
Blocks: 1450883
No longer blocks: 1450883
You need to log in before you can comment on or make changes to this bug.