Last Comment Bug 702043 - Firefox 8 "layers::ContainerLayer::GetType" Memory Corruption
: Firefox 8 "layers::ContainerLayer::GetType" Memory Corruption
Status: VERIFIED FIXED
[sg:dos stack exhaustion] fixed by 21...
: regression
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: 8 Branch
: All All
: P3 normal (vote)
: mozilla9
Assigned To: Nobody; OK to take it and work on it
:
: Jet Villegas (:jet)
Mentors:
Depends on: 211636
Blocks: 678671
  Show dependency treegraph
 
Reported: 2011-11-12 12:50 PST by Tom Ferris
Modified: 2012-02-16 14:17 PST (History)
7 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
fixed
unaffected


Attachments
layers_ReadbackSink.xhtml (474 bytes, application/xhtml+xml)
2011-11-12 12:50 PST, Tom Ferris
no flags Details
Original test w/out the meta refresh (487 bytes, application/xhtml+xml)
2011-11-14 19:10 PST, Daniel Veditz [:dveditz]
no flags Details

Description Tom Ferris 2011-11-12 12:50:33 PST
Created attachment 574084 [details]
layers_ReadbackSink.xhtml

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/535.2 (KHTML, like Gecko) Chrome/15.0.874.106 Safari/535.2

Steps to reproduce:

A memory corruption vulnerability exists when parsing a specially crafted html page that contains a css tag with clip: rect and also a vertical-align, Firefox 8 (all platforms) will crash on a write av.

The issue seems to be within the follow code: 

http://dxr.mozilla.org/mozilla/mozilla-central/gfx/layers/Layers.cpp.html

In regards to exploitability, this is a write av but it appears to be a unbounded recursion issue because there are around 500 stack frames.

Below is the debug/disassembly information:

http://dxr.mozilla.org/mozilla/mozilla-central/gfx/layers/Layers.cpp.html

(69c.d1c): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
eax=00000000 ebx=00000000 ecx=055edd18 edx=00000001 esi=055edd18 edi=000310b4
eip=01048c70 esp=00031000 ebp=00031074 iopl=0         nv up ei pl nz na po nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00210202
xul!mozilla::layers::ContainerLayer::GetType+0x12cec:
01048c70 55              push    ebp
0:000> U . L64
xul!mozilla::layers::ContainerLayer::GetType+0x12d01:
01048c85 53              push    ebx
01048c86 8b5d78          mov     ebx,dword ptr [ebp+78h]
01048c89 56              push    esi
01048c8a 8b349dd012a201  mov     esi,dword ptr xul!gfxFontUtils::gMacFontNameCharsets+0x574a8 (01a212d0)[ebx*4]
01048c91 57              push    edi
01048c92 8d04f500000000  lea     eax,[esi*8]
01048c99 894d68          mov     dword ptr [ebp+68h],ecx
01048c9c 897558          mov     dword ptr [ebp+58h],esi
01048c9f e85c682000      call    xul!NS_InvokeByIndex_P+0x23f25 (0124f500)
01048ca4 85f6            test    esi,esi
01048ca6 896550          mov     dword ptr [ebp+50h],esp
01048ca9 7617            jbe     xul!mozilla::layers::ContainerLayer::GetType+0x12d3e (01048cc2)
01048cab 8bc4            mov     eax,esp
01048cad 8bce            mov     ecx,esi
01048caf 90              nop
01048cb0 85c0            test    eax,eax
01048cb2 7406            je      xul!mozilla::layers::ContainerLayer::GetType+0x12d36 (01048cba)
01048cb4 c70000000000    mov     dword ptr [eax],0
01048cba 83c008          add     eax,8
01048cbd 83e901          sub     ecx,1
01048cc0 75ee            jne     xul!mozilla::layers::ContainerLayer::GetType+0x12d2c (01048cb0)
01048cc2 8b7568          mov     esi,dword ptr [ebp+68h]
01048cc5 8b06            mov     eax,dword ptr [esi]
01048cc7 8945dc          mov     dword ptr [ebp-24h],eax
01048cca a15c0ad701      mov     eax,dword ptr [xul!gfxFontUtils::gMSFontNameCharsets+0x78f98 (01d70a5c)]
01048ccf 8bcb            mov     ecx,ebx
01048cd1 ba01000000      mov     edx,offset <Unloaded_ta.dll> (00000001)
01048cd6 d3e2            shl     edx,cl
01048cd8 8b4d7c          mov     ecx,dword ptr [ebp+7Ch]
01048cdb 2bc4            sub     eax,esp
01048cdd c1e803          shr     eax,3
01048ce0 894de0          mov     dword ptr [ebp-20h],ecx
01048ce3 33c9            xor     ecx,ecx
01048ce5 8955d4          mov     dword ptr [ebp-2Ch],edx
01048ce8 c645d801        mov     byte ptr [ebp-28h],1
01048cec 894de4          mov     dword ptr [ebp-1Ch],ecx
01048cef 8965e8          mov     dword ptr [ebp-18h],esp
01048cf2 8945ec          mov     dword ptr [ebp-14h],eax
01048cf5 8945f0          mov     dword ptr [ebp-10h],eax
01048cf8 8945f4          mov     dword ptr [ebp-0Ch],eax
01048cfb 8945f8          mov     dword ptr [ebp-8],eax
01048cfe 8945fc          mov     dword ptr [ebp-4],eax
01048d01 894500          mov     dword ptr [ebp],eax
01048d04 894504          mov     dword ptr [ebp+4],eax
01048d07 894508          mov     dword ptr [ebp+8],eax
01048d0a 89450c          mov     dword ptr [ebp+0Ch],eax
01048d0d 894510          mov     dword ptr [ebp+10h],eax
01048d10 894514          mov     dword ptr [ebp+14h],eax
01048d13 894518          mov     dword ptr [ebp+18h],eax
01048d16 89451c          mov     dword ptr [ebp+1Ch],eax
01048d19 894520          mov     dword ptr [ebp+20h],eax
01048d1c 894524          mov     dword ptr [ebp+24h],eax
01048d1f 894528          mov     dword ptr [ebp+28h],eax
01048d22 89452c          mov     dword ptr [ebp+2Ch],eax
01048d25 894530          mov     dword ptr [ebp+30h],eax
01048d28 894534          mov     dword ptr [ebp+34h],eax
01048d2b 894538          mov     dword ptr [ebp+38h],eax
01048d2e 89453c          mov     dword ptr [ebp+3Ch],eax
01048d31 894540          mov     dword ptr [ebp+40h],eax
01048d34 894544          mov     dword ptr [ebp+44h],eax
01048d37 895554          mov     dword ptr [ebp+54h],edx
01048d3a 894c9dec        mov     dword ptr [ebp+ebx*4-14h],ecx
01048d3e 894d64          mov     dword ptr [ebp+64h],ecx
01048d41 894d60          mov     dword ptr [ebp+60h],ecx
01048d44 897548          mov     dword ptr [ebp+48h],esi
01048d47 33ff            xor     edi,edi
01048d49 eb0a            jmp     xul!mozilla::layers::ContainerLayer::GetType+0x12dd1 (01048d55)
01048d4b eb03            jmp     xul!mozilla::layers::ContainerLayer::GetType+0x12dcc (01048d50)
01048d4d 8d4900          lea     ecx,[ecx]
01048d50 8b5554          mov     edx,dword ptr [ebp+54h]
01048d53 33c9            xor     ecx,ecx
01048d55 855620          test    dword ptr [esi+20h],edx
01048d58 752c            jne     xul!mozilla::layers::ContainerLayer::GetType+0x12e02 (01048d86)
01048d5a 85561c          test    dword ptr [esi+1Ch],edx
01048d5d 0f85dd010000    jne     xul!mozilla::layers::ContainerLayer::GetType+0x12fbc (01048f40)
01048d63 83fb09          cmp     ebx,9
01048d66 0f8cb6000000    jl      xul!mozilla::layers::ContainerLayer::GetType+0x12e9e (01048e22)
01048d6c 8b4618          mov     eax,dword ptr [esi+18h]
01048d6f 3bc1            cmp     eax,ecx
01048d71 0f84b6000000    je      xul!mozilla::layers::ContainerLayer::GetType+0x12ea9 (01048e2d)
01048d77 8b4498dc        mov     eax,dword ptr [eax+ebx*4-24h]
01048d7b 3bc1            cmp     eax,ecx
01048d7d 894564          mov     dword ptr [ebp+64h],eax
01048d80 0f84aa000000    je      xul!mozilla::layers::ContainerLayer::GetType+0x12eac (01048e30)
01048d86 8b4d7c          mov     ecx,dword ptr [ebp+7Ch]
01048d89 8b4118          mov     eax,dword ptr [ecx+18h]
01048d8c 85c0            test    eax,eax
01048d8e 0f856f030000    jne     xul!mozilla::layers::ContainerLayer::GetType+0x1317f (01049103)
01048d94 8b4d60          mov     ecx,dword ptr [ebp+60h]
01048d97 33c0            xor     eax,eax
01048d99 83fb09          cmp     ebx,9
01048d9c 0f9dc0          setge   al
01048d9f 85c9            test    ecx,ecx
01048da1 7506            jne     xul!mozilla::layers::ContainerLayer::GetType+0x12e25 (01048da9)
01048da3 8b4d48          mov     ecx,dword ptr [ebp+48h]
01048da6 894d60          mov     dword ptr [ebp+60h],ecx
01048da9 8a55d8          mov     dl,byte ptr [ebp-28h]
01048dac 84d2            test    dl,dl
01048dae 0f84eaea2f00    je      xul!NS_InvokeByIndex_P+0x11c2c3 (0134789e)
01048db4 85ff            test    edi,edi

===============

0:000> .load msec
0:000> !exploitable -v
HostMachine\HostUser
Executing Processor Architecture is x86
Debuggee is in User Mode
Debuggee is a live user mode debugging session on the local machine
Event Type: Exception
Exception Faulting Address: 0x30ffc
Second Chance Exception Type: STATUS_ACCESS_VIOLATION (0xC0000005)
Exception Sub-Type: Write Access Violation

Exception Hash (Major/Minor): 0x565f2a7d.0x2b722867

Stack Trace:
xul!mozilla::layers::ContainerLayer::GetType+0x12cec
xul!gfxContext::CurrentMatrix+0x3648
xul!NS_InvokeByIndex_P+0x11327e
xul!mozilla::layers::ThebesLayer::GetType+0x4cf5
xul!NS_InvokeByIndex_P+0x11422e
xul!NS_InvokeByIndex_P+0x11c539
xul!gfxContext::Clip+0x3554
xul!gfxContext::CurrentMatrix+0x3648
xul!NS_InvokeByIndex_P+0x6430b
xul!gfxTextRunCache::MakeTextRun+0x82a
xul!NS_InvokeByIndex_P+0x11327e
xul!mozilla::layers::ThebesLayer::GetType+0x4cf5
xul!NS_InvokeByIndex_P+0x11422e
xul!NS_InvokeByIndex_P+0x11c539
xul!gfxContext::Clip+0x3554
xul!gfxContext::CurrentMatrix+0x3648
xul!NS_InvokeByIndex_P+0x6430b
xul!gfxTextRunCache::MakeTextRun+0x82a
xul!NS_InvokeByIndex_P+0x11327e
xul!mozilla::layers::ThebesLayer::GetType+0x4cf5
xul!NS_InvokeByIndex_P+0x11422e
xul!NS_InvokeByIndex_P+0x11c539
xul!gfxContext::Clip+0x3554
xul!gfxContext::CurrentMatrix+0x3648
xul!NS_InvokeByIndex_P+0x6430b
xul!gfxTextRunCache::MakeTextRun+0x82a
xul!NS_InvokeByIndex_P+0x11327e
xul!mozilla::layers::ThebesLayer::GetType+0x4cf5
xul!NS_InvokeByIndex_P+0x11422e
xul!NS_InvokeByIndex_P+0x11c539
xul!gfxContext::Clip+0x3554
xul!gfxContext::CurrentMatrix+0x3648
xul!NS_InvokeByIndex_P+0x6430b
xul!gfxTextRunCache::MakeTextRun+0x82a
xul!NS_InvokeByIndex_P+0x11327e
xul!mozilla::layers::ThebesLayer::GetType+0x4cf5
xul!NS_InvokeByIndex_P+0x11422e
xul!NS_InvokeByIndex_P+0x11c539
xul!gfxContext::Clip+0x3554
xul!gfxContext::CurrentMatrix+0x3648
xul!NS_InvokeByIndex_P+0x6430b
xul!gfxTextRunCache::MakeTextRun+0x82a
xul!NS_InvokeByIndex_P+0x11327e
xul!mozilla::layers::ThebesLayer::GetType+0x4cf5
xul!NS_InvokeByIndex_P+0x11422e
xul!NS_InvokeByIndex_P+0x11c539
xul!gfxContext::Clip+0x3554
xul!gfxContext::CurrentMatrix+0x3648
xul!NS_InvokeByIndex_P+0x6430b
xul!gfxTextRunCache::MakeTextRun+0x82a
xul!NS_InvokeByIndex_P+0x11327e
xul!mozilla::layers::ThebesLayer::GetType+0x4cf5
xul!NS_InvokeByIndex_P+0x11422e
xul!NS_InvokeByIndex_P+0x11c539
xul!gfxContext::Clip+0x3554
xul!gfxContext::CurrentMatrix+0x3648
xul!NS_InvokeByIndex_P+0x6430b
xul!gfxTextRunCache::MakeTextRun+0x82a
xul!NS_InvokeByIndex_P+0x11327e
xul!mozilla::layers::ThebesLayer::GetType+0x4cf5
xul!NS_InvokeByIndex_P+0x11422e
xul!NS_InvokeByIndex_P+0x11c539
xul!gfxContext::Clip+0x3554
xul!gfxContext::CurrentMatrix+0x3648
Instruction Address: 0x0000000001048c70

Description: User Mode Write AV
Short Description: WriteAV
Exploitability Classification: EXPLOITABLE
Recommended Bug Title: Exploitable - User Mode Write AV starting at xul!mozilla::layers::ContainerLayer::GetType+0x0000000000012cec (Hash=0x565f2a7d.0x2b722867)

User mode write access violations that are not near NULL are exploitable.
Comment 1 Tom Ferris 2011-11-12 12:53:50 PST
Comment on attachment 574084 [details]
layers_ReadbackSink.xhtml

PoC to trigger the bug.
Comment 3 Daniel Veditz [:dveditz] 2011-11-14 19:10:59 PST
Created attachment 574507 [details]
Original test w/out the meta refresh

On Mac Firefox 8 crashes but Firefox 9 does not. Here's a modified version of the testcase, removing the meta refresh which does not affect the crash and results in less confusion on versions without the crash.

Need to bisect to find a fix range to see if it was really a fix or simply masking it, leaving a lurking bug waiting to bite us.
Comment 4 Daniel Veditz [:dveditz] 2011-11-22 13:06:50 PST
So far 2011-08-01 and 2011-07-18 nightlies don't crash on Mac.
Comment 5 Al Billings [:abillings] 2011-11-22 15:09:42 PST
Does crash with comment 3 testcase:

2011-08-16: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:8.0a1) Gecko/20110816 Firefox/8.0a1

Does crash (post-Firefox 8) with:

2011-08-21: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:9.0a1) Gecko/20110821 Firefox/9.0a1

Does not crash (post-Firefox 8) with:

2011-08-22: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:9.0a1) Gecko/20110822 Firefox/9.0a1
Comment 6 Daniel Veditz [:dveditz] 2011-11-22 16:43:28 PST
And on the other end, 2011-08-15 8.0a1 is fine and 2011-08-16 crashes.

So it broke in
http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2011-08-15&enddate=2011-08-16+03%3A00

And went away in
http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2011-09-21&enddate=2011-09-22+03%3A00
Comment 7 Daniel Veditz [:dveditz] 2011-11-23 09:23:56 PST
For the regressing bug I'm going to blame bug 678671 after a partial bisect. The crashes I see are recursive stack exhaustion with lots of RuleNodes http://hg.mozilla.org/mozilla-central/rev/3a378e08192f and PresShell http://hg.mozilla.org/mozilla-central/rev/83291ec2e28e
Comment 8 Daniel Veditz [:dveditz] 2011-11-23 09:36:55 PST
Narrower regression range used for comment 7 was
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8994b8638803&tochange=e32b302039ca and it didn't seem worth going further.
Comment 9 Daniel Veditz [:dveditz] 2011-11-23 10:23:40 PST
Misread Al's comment, bug fix range was a month earlier 
http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2011-08-21&enddate=2011-08-22
Comment 10 David Baron :dbaron: ⌚️UTC-10 2011-11-23 12:06:00 PST
I see the same fix range Al sees in terms of nightlies (on Linux x86_64), but the correct pushlog range for when it got fixed would be:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6dc468c41136&tochange=f41df039db03

(which is when the nightlies are generated, from their own metadata, rather than at 00:00 on the day, which is too early).
Comment 11 David Baron :dbaron: ⌚️UTC-10 2011-11-23 12:13:05 PST
My guess would be that http://hg.mozilla.org/mozilla-central/rev/4738b38a2f3c made it go away due to removal of some GetStyleDisplay() calls.
Comment 12 David Baron :dbaron: ⌚️UTC-10 2011-11-23 12:30:28 PST
Yes, reintroducing the code

-  if (aData->mSIDs & NS_STYLE_INHERIT_BIT(Visibility)) {
-    const nsStyleDisplay* readDisplay = aData->mStyleContext->GetStyleDisplay();

-  }

from that patch makes it crash again.


The underlying issue here is one of CSS property computation dependencies.  
http://wiki.csswg.org/spec/property-dependencies describes CSS's dependencies between properties; CSS must avoid circular dependencies between properties.  We also have to avoid circular dependencies between style structs; we should probably write out what those dependencies are, but I think it's correct for Visibility to depend on Display (as bug 678671 adds such a dependency... although it may not be the only one), and incorrect for Visibility to depend on Display (which bug 211636 fixes).


I don't see any signs that this is anything other than a stack exhaustion crash.
Comment 13 David Baron :dbaron: ⌚️UTC-10 2011-11-23 12:32:40 PST
Created attachment 576596 [details]
one iteration of the recursion

Here's one iteration of the recursion (with which we exhaust the stack) after I readd the code in comment 12.
Comment 14 David Baron :dbaron: ⌚️UTC-10 2011-11-23 12:45:57 PST
Further, I checked that now that bug 211636 is fixed, we no longer have any attribute mapping functions that get style data.  This means that auditing for struct dependencies should now be isolated to auditing code in nsRuleNode (which is how it should have been from the start).  Note that when auditing there's no need to count calls to the parent style context.

The messy implicit dependencies are that:

CalcLength can call GetStyleVisibility() (via GetMetricsFor, introduced in bug 678671) or GetStyleFont()

SetColor can call GetStyleColor

AdjustLogicalBoxProp can call GetStyleVisibility()


The explicit dependencies are:

SetFont (helper for ComputeFontData) calls GetStyleVisibility()

ComputeTextData calls GetStyleFont

ComputeTextResetData calls GetStyleColor

So the only targets of cross-struct dependencies are the Font, Visibility, and Color structs.  This makes sense give http://wiki.csswg.org/spec/property-dependencies (since a bunch of the dependencies listed there are within-struct and therefore less risky in terms of code).

nsStyleVisibility has no lengths, colors, or logical box properties.

nsStyleColor has only a single color.

nsStyleFont has no colors or logical box properties, and we already handle its length issues specially.

So I think we're in good shape now.
Comment 15 David Baron :dbaron: ⌚️UTC-10 2011-11-23 12:55:08 PST
Given that this is a stack exhaustion crash, I think we should just mark it fixed and open it up when 9 ships.
Comment 16 David Baron :dbaron: ⌚️UTC-10 2011-11-23 12:57:13 PST
(In reply to David Baron [:dbaron] from comment #12)
> The underlying issue here is one of CSS property computation dependencies.  
> http://wiki.csswg.org/spec/property-dependencies describes CSS's
> dependencies between properties; CSS must avoid circular dependencies
> between properties.  We also have to avoid circular dependencies between
> style structs; we should probably write out what those dependencies are, but
> I think it's correct for Visibility to depend on Display (as bug 678671 adds
> such a dependency... although it may not be the only one), and incorrect for
> Visibility to depend on Display (which bug 211636 fixes).

Also, I don't see any cases of this that existed prior to bug 678671.
Comment 17 David Baron :dbaron: ⌚️UTC-10 2011-11-23 12:58:55 PST
Also, when we open it up (but not before), we should change the summary to something like "Firefox 8 stack exhaustion with certain units in clip: rect() on <table>".
Comment 18 Daniel Veditz [:dveditz] 2011-11-23 22:48:10 PST
The fix in that range turns out to be http://hg.mozilla.org/mozilla-central/rev/4738b38a2f3c (bug 214636)

The one thing that leaves me uncomfortable is that I never saw a stack like the one in comment 0. I always saw RuleNode stuff as noted in comment 7, infinite recursion with the following on the stack:

nsRuleNode::GetStyleVisibility
DoGetStyleVisibility
GetStyleVisibility
GetMetricsFor
CalcLengthWith
CalcLength
nsRuleNode::ComputeDisplayData
nsRuleNode::WalkRuleTree
nsRuleNode::GetStyleDisplay
DoGetStyleDisplay
GetStyleDisplay
MapAttributesIntoRule
nsRuleNode::WalkRuleTree
 ... repeat ...

The crash I'm seeing does not look exploitable.

Did we get the wrong testcase?

I've been testing this on a Mac; maybe Windows gets a different crash and it's just coincidence the same testcase found two different bugs?
Comment 19 Daniel Veditz [:dveditz] 2011-11-23 22:51:02 PST
(In reply to Daniel Veditz from comment #18)
> The fix in that range turns out to be ... (bug 214636)

Right changeset, wrong bug. That changeset was for bug 211636 "nsHTMLTableElement MapAttributesIntoRule is broken". I don't know where I got the '4' from.
Comment 20 Daniel Veditz [:dveditz] 2011-11-29 13:38:57 PST
Tom: according to our current understanding the attached testcase is not an exploitable crash and also happened to be fixed a couple of months before you reported this bug. As such it would not qualify for our bug bounty.

Worryingly, what we see from your testcase doesn't match the symptoms you originally reported. Is the attachment perhaps a minimized testcase that happened to trigger a different bug than an unreported original? If so we'd really like to work on it. If so, however, please file a separate bug to avoid confusion. You can use the "Clone this bug" link near the footer of this page to easily populate a new bug form.
Comment 21 Daniel Veditz [:dveditz] 2011-11-29 18:42:40 PST
For some reason crash-stats doesn't show the recursion in the stack but does show the crash reason is stack overflow and crash is one of the typical ones
bp-6ee566dd-0bcd-4700-bbd0-45d4e2111129
Comment 22 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-12-08 15:55:45 PST
Unable to get this to crash using Nightly and Aurora from 2011-12-08, and Firefox 9.0b5. However, I am seeing 64 instances of this crash signature in the last week on 11.0a1, 10.0a2, and 9.0b4. Can someone please confirm?
Comment 23 Daniel Veditz [:dveditz] 2011-12-15 17:57:38 PST
The ones that are not EXCEPTION_STACK_OVERFLOW are a definitely something else. There are only a handful post-8.0.1 that are stack exhaustion. One I could tell was a different recursive stack. Can't tell with the rest but layout is a big area and it's not hard to believe there are other similar problems lurking. You can (and have) verify that the bug demonstrated by _this_ testcase has been fixed.
Comment 24 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-12-15 18:05:53 PST
Marking this verified based on comment 23.

Note You need to log in before you can comment on or make changes to this bug.