Created attachment 473219 [details] testcase (crashes Firefox 3.6.8) wushi reported the following Firefox 3.6. crash to firstname.lastname@example.org: ------- Hi, I think I found another exploitable vuln for firefox 3.6.8, the stack like this: (960.858): Access violation - code c0000005 (first chance) First chance exceptions are reported before any exception handling. This exception may be expected and handled. eax=03c7de48 ebx=03c7de50 ecx=3ffdf793 edx=00000000 esi=03cffffc edi=03d00000 eip=7815023a esp=0012f68c ebp=0012f694 iopl=0 nv up ei pl nz na po nc cs=001b ss=0023 ds=0023 es=0023 fs=003b gs=0000 efl=00010202 *** ERROR: Symbol file could not be found. Defaulted to export symbols for C:\Program Files\Mozilla Firefox\MOZCRT19.dll - MOZCRT19!memmove+0x5a: 7815023a f3a5 rep movs dword ptr es:[edi],dword ptr [esi] *** ERROR: Symbol file could not be found. Defaulted to export symbols for C:\Program Files\Mozilla Firefox\xul.dll - ChildEBP RetAddr WARNING: Stack unwind information not available. Following frames may be wrong. 0012f694 100eab13 MOZCRT19!memmove+0x5a 0012f6b0 1076138d xul!gfxIntSize::gfxIntSize+0x49d7 0012f6d0 107613bf xul!gfxFontTestStore::NewStore+0x6cff 00000000 00000000 xul!gfxFontTestStore::NewStore+0x6d31 When you check the POC, you can found a lot of no-use things, these things just make the file size > 26k , To reproduce this case, maybe you need make your PC slowly and refresh some times ,haha. I can give you the dump file If you need . wushi
Dveditz will try to reproduce.
Got a different stack on a debug Mac 1.9.2 build after reloading a few times (crash in the parser). Haven't seen a problem on trunk or an opt 1.9.2 builds. Exception Type: EXC_BAD_ACCESS (SIGSEGV) Exception Codes: KERN_INVALID_ADDRESS at 0x000000001d8ffffc Crashed Thread: 0 Thread 0 Crashed: 0 __memcpy + 1908 (cpu_capabilities.h:246) 1 nsTArray_base::ShiftData(unsigned int, unsigned int, unsigned int, unsigned int) + 197 (nsTArray.cpp:175) 2 Row** nsTArray<Row*>::ReplaceElementsAt<Row*>(unsigned int, unsigned int, Row* const*, unsigned int) + 129 3 Row** nsTArray<Row*>::InsertElementsAt<Row*>(unsigned int, nsTArray<Row*> const&) + 65 4 nsTreeContentView::InsertRow(int, int, nsIContent*) + 375 (nsTreeContentView.cpp:1447) 5 nsTreeContentView::ContentInserted(nsIDocument*, nsIContent*, nsIContent*, int) + 1342 (nsTreeContentView.cpp:1076) 6 nsTreeContentView::ContentAppended(nsIDocument*, nsIContent*, int) + 105 (nsTreeContentView.cpp:990) 7 nsNodeUtils::ContentAppended(nsIContent*, int) + 324 (nsNodeUtils.cpp:135) 8 nsContentSink::NotifyAppend(nsIContent*, unsigned int) + 146 (nsContentSink.cpp:1381) 9 nsXMLContentSink::HandleEndElement(unsigned short const*, int) + 752 (nsXMLContentSink.cpp:1200) 10 nsXMLContentSink::HandleEndElement(unsigned short const*) + 32 (nsXMLContentSink.cpp:1146) 11 nsExpatDriver::HandleEndElement(unsigned short const*) + 233 (nsExpatDriver.cpp:450) 12 Driver_HandleEndElement(void*, unsigned short const*) + 93 13 doContent + 3677 (xmlparse.c:2550) 14 contentProcessor + 81 (xmlparse.c:2095) 15 MOZ_XML_ParseBuffer + 192 (xmlparse.c:1618) 16 MOZ_XML_Parse + 1218 (xmlparse.c:1589) 17 nsExpatDriver::ParseBuffer(unsigned short const*, unsigned int, int, unsigned int*) + 544 (nsExpatDriver.cpp:1027) 18 nsExpatDriver::ConsumeToken(nsScanner&, int&) + 940 (nsExpatDriver.cpp:1126) 19 nsParser::Tokenize(int) + 349 (nsParser.cpp:3120) 20 nsParser::ResumeParse(int, int, int) + 515 (nsParser.cpp:2336) 21 nsParser::OnDataAvailable(nsIRequest*, nsISupports*, nsIInputStream*, unsigned int, unsigned int) + 776 (nsParser.cpp:2985) 22 nsDocumentOpenInfo::OnDataAvailable(nsIRequest*, nsISupports*, nsIInputStream*, unsigned int, unsigned int) + 99 (nsURILoader.cpp:306) 23 nsStreamListenerTee::OnDataAvailable(nsIRequest*, nsISupports*, nsIInputStream*, unsigned int, unsigned int) + 624 (nsStreamListenerTee.cpp:108) 24 nsHttpChannel::OnDataAvailable(nsIRequest*, nsISupports*, nsIInputStream*, unsigned int, unsigned int) + 802 (nsHttpChannel.cpp:5391) 25 nsInputStreamPump::OnStateTransfer() + 777 (nsInputStreamPump.cpp:510) 26 nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) + 138 (nsInputStreamPump.cpp:400) 27 nsInputStreamReadyEvent::Run() + 100 (nsStreamUtils.cpp:113) 28 nsThread::ProcessNextEvent(int, int*) + 676 (nsThread.cpp:527) 29 NS_ProcessPendingEvents_P(nsIThread*, unsigned int) + 146 30 nsBaseAppShell::NativeEventCallback() + 181 (nsBaseAppShell.cpp:126) 31 nsAppShell::ProcessGeckoEvents(void*) + 518 (nsAppShell.mm:426) 32 CFRunLoopRunSpecific + 3141 33 CFRunLoopRunInMode + 88 34 RunCurrentEventLoopInMode + 283 35 ReceiveNextEventCommon + 175 36 BlockUntilNextEventMatchingListInMode + 106 37 _DPSNextEvent + 657 38 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 128 39 -[NSApplication run] + 795 40 nsAppShell::Run() + 288 (nsAppShell.mm:778) 41 nsAppStartup::Run() + 148 (nsAppStartup.cpp:183) 42 XRE_main + 14942 (nsAppRunner.cpp:3483) 43 main + 709 (nsBrowserApp.cpp:158) 44 _start + 209 45 start + 41
Opt Windows 3.6.10pre, on the other hand, goes down pretty easily. bp-6066425b-a2cd-445c-95c2-ef2412100921 bp-e0059276-fcdb-4860-85c5-0376c2100921 The first crash only took 3 or 4 reloads, the second took at least twenty, plus closing and reopening the testcase in the middle. The stacks look like what I saw on Mac debug. I don't see gfxIntSize anywhere unlike comment 0. Two different bugs? wushi: what fonts are you using on your system? Maybe that's where the difference lies.
jdaggett, can you take a look here?
(In reply to comment #4) > I don't see gfxIntSize anywhere unlike comment 0. Two different bugs? I'm not sure this is actually a crash in gfx code at all, my guess is that this is a bug in content code. The offset from gfxIntSize in the original description is quite large, I think you're seeing a crash in other code. gfxIntSize::gfxIntSize+0x49d7 gfxFontTestStore::NewStore+0x6cff Looking at the source for the 1.9.2 branch, that's testing code that will only ever get called by a test tool that is independent from xul.lib. We should probably #ifdef out this code but that's not really relevant to this bug.
If we ifdef out the test code, try to reproduce, is there any chance that will change the stack a bit and reveal the source of the problem?
Given the offsets in comment 6, it seems like this is totally unrelated to gfxIntSize. Need to get a better stack which contains correct symbols
dveditz will try to repro with a debug build since it was crashing for him.
Have anybody track this one?
What am I supposed to do with a debug build? It repros there as noted in comment 2.
Created attachment 491346 [details] [diff] [review] Likely fix. While I don't fully understand all of the code involved here, what seems to be happening here is that we're notifying for having appended a new child in the sink, the child in question is the <option> element in the testcase, where the relevant parts look like this: <ther:tree insertafter="" > <ther:treechildren > <div id="div2">X <xht:optgroup > <xht:option>1111111111 """"""""""</xht:option> We have a XUL tree, with a treechildren child, containing an HTML div with an optgroup, containing the option that we're adding during the crash. When we're notifying for the option, we try to find the row index (FindContent(), passing in the optgroup), which is not a direct child of the treechildren, nor an actual row, so we get -1 back. Doing nothing in this case seems to fix this, but it's non-trivial to reproduce this here, so I can't claim to know for sure.
Comment on attachment 491346 [details] [diff] [review] Likely fix. This seems ok (though it would be good for Timothy to double-check)... but weren't we going to rip all the HTML stuff out of this code? It's unused (a carryover from XBL form controls), and just causes issues like this one. I don't see an existing bug on this; we should get one filed (and ideally fixed).
Comment on attachment 491346 [details] [diff] [review] Likely fix. I wonder why this is the only place that doesn't check the return value of FindContent.
(In reply to comment #14) > but > weren't we going to rip all the HTML stuff out of this code? It's unused (a > carryover from XBL form controls), and just causes issues like this one. > > I don't see an existing bug on this; we should get one filed (and ideally > fixed). I don't know the history of this, but are we fairly confident no one has used these tags inside a tree?
(In reply to comment #16) > I don't know the history of this, but are we fairly confident no one has used > these tags inside a tree? Since work stopped on XBL form controls I've only seen them in crash tests.
> I wonder why this is the only place that doesn't check the return value of > FindContent. Probably because outside of fuzzers this is dead code? > I don't know the history of this, but are we fairly confident no one has used > these tags inside a tree? Confident enough that I'm totally willing to break any such users, who must have gone to some lengths to thus weirdly mix namespaces.
Ok, so let's get a bug on that, although it's less important now that we've disabled remote xul.
Comment on attachment 491346 [details] [diff] [review] Likely fix. a=LegNeato for 18.104.22.168 and 22.214.171.124. Just a reminder that code freeze is tonight.
(In reply to comment #19) > Ok, so let's get a bug on that, although it's less important now that we've > disabled remote xul. Was this bug filed yet?
(In reply to comment #23) > (In reply to comment #19) > > Ok, so let's get a bug on that, although it's less important now that we've > > disabled remote xul. > > Was this bug filed yet? I filed bug 616746, thanks for the reminder.