Crash [@ nsINode::CompareDocPosition(nsINode*) ] with too many attr children

RESOLVED FIXED in mozilla8

Status

()

Core
HTML: Parser
--
critical
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: bc, Assigned: mats)

Tracking

(Blocks: 1 bug, {crash, reproducible})

Trunk
mozilla8
crash, reproducible
Points:
---

Firefox Tracking Flags

(firefox7 affected, firefox8 affected)

Details

(Whiteboard: [inbound][sg:dos] null-pointer crash, crash signature, URL)

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
Created attachment 546706 [details]
aurora stack

1. http://dopey.de/flash/
2. Crash Aurora/7, Nightly/8 on Mac.

rapidly issues oodles of warnings then crash with Aurora. Nightly takes a bit longer. Haven't checked Beta/6.

aurora bp-8698ca1b-054a-41c5-8d01-4f6572110718

WARNING: NS_ENSURE_TRUE(childCount < ATTRCHILD_ARRAY_MAX_CHILD_COUNT) failed: file /work/mozilla/builds/aurora/mozilla/content/base/src/nsAttrAndChildArray.cpp, line 170
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file /work/mozilla/builds/aurora/mozilla/content/base/src/nsGenericElement.cpp, line 3473

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
0x054254eb in nsINode::CompareDocPosition (this=0x0, aOtherNode=0x26151260) at /work/mozilla/builds/aurora/mozilla/content/base/src/nsGenericElement.cpp:774
774	  if (node2->IsNodeOfType(nsINode::eATTRIBUTE)) {

fwiw, this is another of these sites with broken php scripts that eat memory and crash the browser.

Automation did see a different stack with aurora:
NS_DebugBreak_P | nsCOMPtr<nsIParser>::operator-> | nsScriptElement::MaybeProcessScript nsHTMLScriptElement::BindToTree nsINode::doInsertChildAt nsGenericElement::InsertChildAt nsINode::AppendChildTo  

The original Socorro signature was mozalloc_abort(char const* const) | mozalloc_handle_oom() | nsTArray_base<nsTArrayDefaultAllocator>::EnsureCapacity(unsigned int, unsigned int) | nsTArray<nsHtml5TreeOperation, nsTArrayDefaultAllocator>::MoveElementsFrom<nsHtml5TreeOperation, nsTArrayD...
Jonas, can you take a look?
Severity: normal → critical
Assignee: nobody → jonas
(Assignee)

Comment 2

6 years ago
PostPendingAppendNotification is called before AppendChildTo:
http://mxr.mozilla.org/mozilla-central/source/parser/html/nsHtml5TreeOperation.cpp#213
which results in a new nsHtml5PendingNotification object:
http://mxr.mozilla.org/mozilla-central/source/parser/html/nsHtml5PendingNotification.h#50
with mChildCount = aParent->GetChildCount().

The bug occurs when the child count has reached the maximum value,
then AppendChildTo() fails and mChildCount has a value that isn't
a valid index so the mParent->GetChildAt(mChildCount) at line 60
returns null leading to the null-pointer crash.
OS: Mac OS X → All
Hardware: x86 → All
Whiteboard: [sg:dos] null-pointer crash
(Assignee)

Comment 3

6 years ago
Created attachment 547002 [details] [diff] [review]
Like so?

Notifying only if AppendChildTo succeeds seems like the way to go.
Attachment #547002 - Flags: review?(hsivonen)
Comment on attachment 547002 [details] [diff] [review]
Like so?

Looks reasonable in order to fix the crash.

I wonder if we should try, as a more elaborate follow-up, to halt the parser on unload the document upon failures that leave the DOM in a state that doesn't correspond to what the parser saw.
Attachment #547002 - Flags: review?(hsivonen) → review+
(Assignee)

Comment 5

6 years ago
Filed bug 672928 to follow-up Henri's review comment.

http://hg.mozilla.org/integration/mozilla-inbound/rev/8d56702130fb
Assignee: jonas → matspal
Component: DOM → HTML: Parser
QA Contact: general → parser
Whiteboard: [sg:dos] null-pointer crash → [inbound][sg:dos] null-pointer crash
Target Milestone: --- → mozilla8
http://hg.mozilla.org/mozilla-central/rev/8d56702130fb
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.