Closed
Bug 507452
Opened 15 years ago
Closed 3 years ago
Crash (too much recursion in BindToTree) with feed full of "<p/><p/>"
Categories
(Core :: DOM: HTML Parser, defect)
Tracking
()
RESOLVED
WORKSFORME
Tracking | Status | |
---|---|---|
blocking2.0 | --- | - |
People
(Reporter: cbook, Unassigned)
References
()
Details
(Keywords: crash, testcase, Whiteboard: [sg:dos])
Attachments
(2 files)
Version=3.5.3pre BuildID=20090730054515
Steps to reproduce:
-> Load http://inside.nike.com/blogs/nikefootball-en_IN/feeds/posts
(934.f10): Stack overflow - code c00000fd (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
eax=0000000d ebx=7ffdb000 ecx=08250690 edx=082506a4 esi=00d05340 edi=011ef6f0
eip=0045111a esp=00033000 ebp=00033008 iopl=0 nv up ei pl nz na po nc
cs=001b ss=0023 ds=0023 es=0023 fs=003b gs=0000 efl=00010202
nspr4!_MD_CURRENT_THREAD+0xa:
0045111a ff155c864600 call dword ptr [nspr4!_imp__TlsGetValue (0046865c)] ds:0023:0046865c={kernel32!TlsGetValue (7c8097e0)}
Event Type: Exception
Exception Faulting Address: 0x45111a
First Chance Exception Type: STATUS_STACK_OVERFLOW (0xC00000FD)
Faulting Instruction:0045111a call dword ptr [nspr4!_imp__tlsgetvalue (0046865c)]
Basic Block:
0045111a call dword ptr [nspr4!_imp__tlsgetvalue (0046865c)]
Exception Hash (Major/Minor): 0x2c027849.0x143d2732
Stack Trace:
nspr4!_MD_CURRENT_THREAD+0xa
nspr4!PR_GetCurrentThread+0x16
nspr4!PR_GetThreadPrivate+0xb
xpcom_core!NS_LogAddRef_P+0x1b
gklayout!nsGenericElement::AddRef+0xa1
gklayout!nsHTMLParagraphElement::AddRef+0xd
gklayout!nsGenericElement::QueryInterface+0x3b0
gklayout!nsHTMLParagraphElement::QueryInterface+0x65
xpcom_core!nsQueryInterface::operator()+0x2d
gklayout!nsCOMPtr<nsIContent>::assign_from_qi+0x19
gklayout!nsCOMPtr<nsIContent>::nsCOMPtr<nsIContent>+0x34
gklayout!nsCOMPtr<nsIContent>::Assert_NoQueryNeeded+0x2e
gklayout!nsCOMPtr<nsIContent>::nsCOMPtr<nsIContent>+0x48
gklayout!nsGenericElement::BindToTree+0x66c
gklayout!nsStyledElement::BindToTree+0x21
gklayout!nsMappedAttributeElement::BindToTree+0x21
gklayout!nsGenericHTMLElement::BindToTree+0x21
gklayout!nsGenericElement::BindToTree+0x697
gklayout!nsStyledElement::BindToTree+0x21
gklayout!nsMappedAttributeElement::BindToTree+0x21
gklayout!nsGenericHTMLElement::BindToTree+0x21
gklayout!nsGenericElement::BindToTree+0x697
gklayout!nsStyledElement::BindToTree+0x21
gklayout!nsMappedAttributeElement::BindToTree+0x21
gklayout!nsGenericHTMLElement::BindToTree+0x21
gklayout!nsGenericElement::BindToTree+0x697
gklayout!nsStyledElement::BindToTree+0x21
gklayout!nsMappedAttributeElement::BindToTree+0x21
gklayout!nsGenericHTMLElement::BindToTree+0x21
gklayout!nsGenericElement::BindToTree+0x697
gklayout!nsStyledElement::BindToTree+0x21
gklayout!nsMappedAttributeElement::BindToTree+0x21
gklayout!nsGenericHTMLElement::BindToTree+0x21
gklayout!nsGenericElement::BindToTree+0x697
gklayout!nsStyledElement::BindToTree+0x21
gklayout!nsMappedAttributeElement::BindToTree+0x21
gklayout!nsGenericHTMLElement::BindToTree+0x21
gklayout!nsGenericElement::BindToTree+0x697
gklayout!nsStyledElement::BindToTree+0x21
gklayout!nsMappedAttributeElement::BindToTree+0x21
gklayout!nsGenericHTMLElement::BindToTree+0x21
gklayout!nsGenericElement::BindToTree+0x697
gklayout!nsStyledElement::BindToTree+0x21
gklayout!nsMappedAttributeElement::BindToTree+0x21
gklayout!nsGenericHTMLElement::BindToTree+0x21
gklayout!nsGenericElement::BindToTree+0x697
gklayout!nsStyledElement::BindToTree+0x21
gklayout!nsMappedAttributeElement::BindToTree+0x21
gklayout!nsGenericHTMLElement::BindToTree+0x21
gklayout!nsGenericElement::BindToTree+0x697
gklayout!nsStyledElement::BindToTree+0x21
gklayout!nsMappedAttributeElement::BindToTree+0x21
gklayout!nsGenericHTMLElement::BindToTree+0x21
gklayout!nsGenericElement::BindToTree+0x697
gklayout!nsStyledElement::BindToTree+0x21
gklayout!nsMappedAttributeElement::BindToTree+0x21
gklayout!nsGenericHTMLElement::BindToTree+0x21
gklayout!nsGenericElement::BindToTree+0x697
gklayout!nsStyledElement::BindToTree+0x21
gklayout!nsMappedAttributeElement::BindToTree+0x21
gklayout!nsGenericHTMLElement::BindToTree+0x21
gklayout!nsGenericElement::BindToTree+0x697
gklayout!nsStyledElement::BindToTree+0x21
gklayout!nsMappedAttributeElement::BindToTree+0x21
Instruction Address: 0x000000000045111a
Description: Stack Overflow
Short Description: StackOverflow
Exploitability Classification: UNKNOWN
Recommended Bug Title: Stack Overflow starting at nspr4!_MD_CURRENT_THREAD+0x000000000000000a (Hash=0x2c027849.0x143d2732)
FAULTING_IP:
gklayout!nsStyledElement::BindToTree+21 [c:\work\mozilla\builds\1.9.1\mozilla\content\base\src\nsstyledelement.cpp @ 157]
02e80411 8945fc mov dword ptr [ebp-4],eax
EXCEPTION_RECORD: ffffffff -- (.exr ffffffffffffffff)
ExceptionAddress: 0045111a (nspr4!_MD_CURRENT_THREAD+0x0000000a)
ExceptionCode: c00000fd (Stack overflow)
ExceptionFlags: 00000000
NumberParameters: 2
Parameter[0]: 00000001
Parameter[1]: 00032ffc
FAULTING_THREAD: 00000f10
BUGCHECK_STR: c00000fd
DEFAULT_BUCKET_ID: STATUS_STACKOVERFLOW
PROCESS_NAME: firefox.exe
ERROR_CODE: (NTSTATUS) 0xc00000fd - A new guard page for the stack cannot be created.
RECURRING_STACK: From frames 0xe to 0x11
LAST_CONTROL_TRANSFER: from 004495a6 to 0045111a
FOLLOWUP_IP:
gklayout!nsStyledElement::BindToTree+21 [c:\work\mozilla\builds\1.9.1\mozilla\content\base\src\nsstyledelement.cpp @ 157]
02e80411 8945fc mov dword ptr [ebp-4],eax
FAULTING_SOURCE_CODE:
153: PRBool aCompileEventHandlers)
154: {
155: nsresult rv = nsStyledElementBase::BindToTree(aDocument, aParent,
156: aBindingParent,
> 157: aCompileEventHandlers);
158: NS_ENSURE_SUCCESS(rv, rv);
159:
160: // XXXbz if we already have a style attr parsed, this won't do
161: // anything... need to fix that.
162: ReparseStyleAttribute(PR_FALSE);
SYMBOL_STACK_INDEX: e
SYMBOL_NAME: gklayout!nsStyledElement::BindToTree+21
FOLLOWUP_NAME: MachineOwner
MODULE_NAME: gklayout
IMAGE_NAME: gklayout.dll
DEBUG_FLR_IMAGE_TIMESTAMP: 4a719fa1
STACK_COMMAND: ~0s ; kb
FAILURE_BUCKET_ID: c00000fd_gklayout!nsStyledElement::BindToTree+21
BUCKET_ID: c00000fd_gklayout!nsStyledElement::BindToTree+21
Comment 1•15 years ago
|
||
crashes on Mac too 94fde5f8-9ad6-4b33-86af-dd0082090730
OS: Windows XP → All
Comment 2•15 years ago
|
||
Comment 3•15 years ago
|
||
Stack exhaustion is usually not a security hole. Any particular reason this bug is marked as security-sensitive?
Comment 4•15 years ago
|
||
Related to bug 474181, or is it just a coincidence that they both crash with too-much-recursion in BindToTree?
Group: core-security
Summary: Stack Overflow starting at nspr4!_MD_CURRENT_THREAD+0x000000000000000a → Crash (too much recursion in BindToTree) with feed full of "<p/><p/>"
Comment 5•15 years ago
|
||
Coincidence; I get a crash in a different place with this bug.
That said, this is just a dup of our various other "deep dom tree will run out of stack" bugs.
Comment 6•15 years ago
|
||
Like bug 323394? I could believe that, but:
1) What's going on at the parsing level? Our HTML parser would normally treat these paragraphs as siblings and limit the depth. Our XML parser would normally treat these paragraphs as closed. So how are we ending up with a deep tree?
2) We should inform Nike that their feed crashes Firefox.
Comment 7•15 years ago
|
||
> Like bug 323394?
Yes.
> 1) What's going on at the parsing level?
Good question. Far as I can see, parser returns a document fragment with 30000-some sibling <p> nodes (<p> closes <p> in HTML parser, so all of them are empty here too).
I'll try to do some digging on Monday into why the end up as descendants of each other.
Comment 8•15 years ago
|
||
OK. So I was totally wrong. This particular testcase causes the parser to produce lots of nested <p>.
Why?
First, the caller (nsScriptableUnescapeHTML::ParseFragment) uses eViewFragment as the mode. This means the parser should suspend containment checking, and in particular means that <p> does not close <p>.
Second, in CNavDTD::WillHandleStartTag we do hit the stackDepth > MAX_REFLOW_DEPTH case, but since <p> has kHandleStrayTag set, we don't close it out. Usually this is ok, since <p> can't contain blocks and non-blocks we close out earlier in the same function, so we can't get into trouble here... unless we have a bunch of nested <p>.
Over to HTML parser, since it seems like eViewFragment ought not to lead to bustage like this; maybe we should not do the kHandleStrayTag check in eViewFragment mode?
Component: General → HTML: Parser
QA Contact: general → parser
Comment 9•15 years ago
|
||
Note that I hit a similar crash with the HTML5 parser enabled; I don't know whether we hit the same codepath in that case or not.
Comment 10•15 years ago
|
||
Note that we have exactly three consumers who can trigger eViewFragment as the mode (using eDTDMode_fragment):
1) HTML paste code.
2) Copy code converting HTML to plaintext via nsHTMLFormatConverter.
3) Feed parsing code.
Of these, only #3 allows random people to control the string coming in; perhaps it should stop using the fragment parsing mode?
Updated•15 years ago
|
blocking1.9.1: ? → ---
Whiteboard: [sg:dos]
Updated•14 years ago
|
blocking2.0: --- → ?
Comment 11•14 years ago
|
||
Is this still an issue? Either way, doesn't sound critical to Firefox 4, but if this is more common than it sounds, please say so here and renominate.
blocking2.0: ? → -
Comment 12•13 years ago
|
||
Although the crash is a duplicate of bug 485941 we can leave this one open to figure out why we think the paragraph elements are nested in the first place.
Depends on: CVE-2009-1232
Depends on: 482909
Comment 13•3 years ago
|
||
Closing this as Resolved > Worksforme as there are no more crashes on the latest versions Firefox Nightly 91.0a1, beta 90.0b7 or release 89.0 using the provided link and test case.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•