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)

1.9.1 Branch
x86
All
defect
Not set
critical

Tracking

()

RESOLVED WORKSFORME
Tracking Status
blocking2.0 --- -

People

(Reporter: cbook, Unassigned)

References

()

Details

(Keywords: crash, testcase, Whiteboard: [sg:dos])

Attachments

(2 files)

Attached file stack (deleted) —
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
crashes on Mac too 94fde5f8-9ad6-4b33-86af-dd0082090730
OS: Windows XP → All
Severity: normal → critical
Keywords: crash
Attached file testcase (deleted) —
Keywords: testcase
Stack exhaustion is usually not a security hole. Any particular reason this bug is marked as security-sensitive?
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/>"
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.
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.
> 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.
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
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.
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?
blocking1.9.1: ? → ---
Whiteboard: [sg:dos]
blocking2.0: --- → ?
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: ? → -
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

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.

Attachment

General

Created:
Updated:
Size: