Closed Bug 578810 Opened 10 years ago Closed 10 years ago

Crash at [@ nsTArray<unsigned char>::AppendElements<unsigned char>(nsTArray<unsigned char> const&) ] using DOMi inspector on Page Info Window

Categories

(Core :: CSS Parsing and Computation, defect, P2, critical)

defect

Tracking

()

VERIFIED FIXED
mozilla2.0b2

People

(Reporter: mozilla.bugs, Assigned: dbaron)

References

Details

Attachments

(1 file)

In testing possible CSS values for Bug 431644, I found that setting the border-top-width to 0px on the xul splitter in the Media Panel caused a crash at [@ nsTArray<unsigned char>::AppendElements<unsigned char>(nsTArray<unsigned char> const&) ]

http://crash-stats.mozilla.com/report/index/53d16c06-387a-4139-9642-9a5e72100714

STR:
1. Install DOMi
2. Open a Page Info dialog with a Media Panel
3. View Chrome Window in DOMi
4. Select #document > window > deck > vbox (mediaPanel) > splitter
5. Change splitter[orient="vertical"] border-top-width to 0px

This has been reproduced in two clean profiles.

Mozilla/5.0 (Windows; Windows NT 6.0; WOW64; en-US; rv:2.0b2pre) Gecko/20100714 Minefield/4.0b2pre ID:20100714040818

xul!nsTArray<unsigned char>::AppendElements<unsigned char>+0x4:
63383447 8b00            mov     eax,dword ptr [eax]  ds:002b:00000000=????????

The line that WinDBG tells me is crashing is nstarray.h:665

      return AppendElements(array.Elements(), array.Length());

http://mxr.mozilla.org/mozilla-central/source/xpcom/glue/nsTArray.h#665
Component: General → Style System (CSS)
Product: Firefox → Core
QA Contact: general → style-system
This is a null-dereference:  the this pointer in the nsCSSDeclaration::Clone call is null.  (Investigating further now.)
The problem here is, I think, that |this| in frame #22 and |this| in frame #12 are the same, but we shouldn't be cloning a sheet while it's in the inconsistent state caused by frames 22/23/24.

It wouldn't surprise me if we had another bug on this...

#12 0x00007fa452f85ab6 in CSSStyleRuleImpl::Clone (this=0x250a620, 
    aClone=@0x7fffc589a748)
    at /home/dbaron/builds/mozilla-central/mozilla/layout/style/nsCSSStyleRule.cpp:1501
#13 0x00007fa452f866bc in CloneRuleInto (aRule=0x10, aArray=0x3535a90)
    at /home/dbaron/builds/mozilla-central/mozilla/layout/style/nsCSSStyleSheet.cpp:796
#14 0x00007fa465a6f5fd in nsVoidArray::EnumerateForwards (this=0x250b6f0, 
    aFunc=0x7fa452f866a0 <CloneRuleInto>, aData=0x3535a90)
    at nsVoidArray.cpp:724
#15 0x00007fa452f8b721 in nsCOMArray_base::EnumerateForwards (this=0x3535a20, 
    aCopy=..., aPrimarySheet=0x250e910) at ../../dist/include/nsCOMArray.h:65
#16 nsCOMArray<nsICSSRule>::EnumerateForwards (this=0x3535a20, aCopy=..., 
    aPrimarySheet=0x250e910) at ../../dist/include/nsCOMArray.h:232
#17 nsCSSStyleSheetInner (this=0x3535a20, aCopy=..., aPrimarySheet=0x250e910)
    at /home/dbaron/builds/mozilla-central/mozilla/layout/style/nsCSSStyleSheet.cpp:868
#18 0x00007fa452f8b7d6 in nsCSSStyleSheetInner::CloneFor (this=0x250b680, 
    aPrimarySheet=0x250e910)
    at /home/dbaron/builds/mozilla-central/mozilla/layout/style/nsCSSStyleSheet.cpp:886
#19 0x00007fa452f8c075 in nsCSSStyleSheet::EnsureUniqueInner (this=0x250e910)
    at /home/dbaron/builds/mozilla-central/mozilla/layout/style/nsCSSStyleSheet.cpp:1385
#20 0x00007fa452f8c105 in nsCSSStyleSheet::WillDirty (this=0x10)
    at /home/dbaron/builds/mozilla-central/mozilla/layout/style/nsCSSStyleSheet.cpp:1503
#21 0x00007fa452f8d163 in nsCSSStyleSheet::ReplaceStyleRule (this=0x250e910, 
    aOld=0x250a648, aNew=0x2adbd58)
    at /home/dbaron/builds/mozilla-central/mozilla/layout/style/nsCSSStyleSheet.cpp:1319
#22 0x00007fa452f7f3d8 in CSSStyleRuleImpl::DeclarationChanged (
    this=0x250a620, aHandleContainer=1)
    at /home/dbaron/builds/mozilla-central/mozilla/layout/style/nsCSSStyleRule.cpp:1545
#23 0x00007fa452f80efb in DOMCSSDeclarationImpl::DeclarationChanged (
    this=0x352eb78)
    at /home/dbaron/builds/mozilla-central/mozilla/layout/style/nsCSSStyleRule.cpp:1137
#24 0x00007fa452faa92c in nsDOMCSSDeclaration::ParsePropertyValue (
    this=0x352eb78, aPropID=<value optimized out>, 
    aPropValue=<value optimized out>, aIsImportant=<value optimized out>)
    at /home/dbaron/builds/mozilla-central/mozilla/layout/style/nsDOMCSSDeclaration.cpp:298
Really, it seems like the problem is that DOMI shouldn't have its hands on a declaration for a rule in a non-uniqued style sheet.
... which probably means inDOMUtils::GetRuleNodeForContent's call to presContext->EnsureSafeToHandOutCSSRules() isn't doing what it should.
... which I'm guessing is because the style sheet in question (chrome://global/skin/splitter.css) is a sheet reached via XBL <resources>.
Assignee: nobody → dbaron
I think this probably doesn't need to be security-sensitive, since it only affects debugging tools like DOM inspector that use the inDOMUtils CSS rules API (and also only affects debugging of documents that use XBL).

This particular case is also a null-dereference, although I suspect some other manifestations of this bug would not be.
Attached patch patchSplinter Review
This fixes it.

An automated test would be nice, but it seems like an awful lot of work...
Attachment #457433 - Flags: review?(bzbarsky)
Flags: in-testsuite?
Status: NEW → ASSIGNED
Comment on attachment 457433 [details] [diff] [review]
patch

r=bzbarsky.
Attachment #457433 - Flags: review?(bzbarsky) → review+
http://hg.mozilla.org/mozilla-central/rev/53a5c3bb6c5c
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
OS: Windows Vista → All
Priority: -- → P2
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b2
And this doesn't need to be security-sensitive; it's a missing piece of bug 536379.  I should have unmarked sooner.
Group: core-security
Verified Fixed on Mozilla/5.0 (Windows; Windows NT 6.0; WOW64; en-US; rv:2.0b2pre) Gecko/20100717 Minefield/4.0b2pre ID:20100717054110
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.