"ASSERTION: disconnected nodes" and "ASSERTION: invalid array index" with selection/range

RESOLVED FIXED in mozilla1.9.1b2

Status

()

defect
P1
critical
RESOLVED FIXED
12 years ago
6 years ago

People

(Reporter: jruderman, Assigned: mats)

Tracking

(Blocks 1 bug, 4 keywords)

Trunk
mozilla1.9.1b2
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 -
wanted1.9.1 +
blocking1.9 -
blocking1.9.0.7 +
wanted1.9.0.x +
wanted1.8.1.x -
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?] post 1.8-branch)

Attachments

(4 attachments, 1 obsolete attachment)

Posted file testcase
Loading the testcase triggers:

###!!! ASSERTION: disconnected nodes: 'parents1.ElementAt(pos1) == parents2.ElementAt(pos2) || aDisconnected', file /Users/jruderman/trunk/mozilla/content/base/src/nsContentUtils.cpp, line 1570

###!!! ASSERTION: invalid array index: 'i < Length()', file ../../dist/include/xpcom/nsTArray.h, line 317

Filing as security-sensitive because the "invalid array index" assertion is happening in nsTArray::ElementAt, which is not bounds-checked at runtime.
Whiteboard: [sg:low?]
Flags: blocking1.9?
Given low severity, this can wait for a 1.9.0.x release.
Given comment 2 moving to wanted list.
Flags: tracking1.9+
Flags: blocking1.9?
Flags: blocking1.9-
Flags: wanted1.9.0.x?
Flags: tracking1.9+
Flags: wanted1.9.0.x? → wanted1.9.0.x+
Flags: blocking1.9.1?
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P3
Target Milestone: --- → mozilla1.9.1
On second thought, not blocking on this. Wanted 1.9.1 though. And this sounds like it could be sg:critical, given the out of bounds array access.
Assignee: nobody → bent.mozilla
Flags: wanted1.9.1+
Flags: blocking1.9.1-
Flags: blocking1.9.1+
Priority: P3 → P1
Whiteboard: [sg:low?] → [sg:critical?]
I had set this as sg:low because it looked like a read (rather than a write) out of bounds.
It's a SEGV crash with a Firefox debug build on Linux (64-bit).
Assignee: bent.mozilla → mats.palmgren
Severity: normal → critical
Keywords: crash
OS: Mac OS X → All
Hardware: PC → All
Posted patch Patch rev. 1 (obsolete) — Splinter Review
The latter part of nsContentUtils::ComparePoints() doesn't make sense for
disconnected nodes, so return early.  The change in nsSelection.cpp is to
allow comparing boundary points for ranges that may be disconnected, which
I think is allowed so it shouldn't assert (but please correct me if I'm wrong).
Attachment #344316 - Flags: superreview?(bzbarsky)
Attachment #344316 - Flags: review?(bzbarsky)
Er....  But shouldn't we do something sane in this case with the return value instead of just using it?
Yeah, we should propagate 'disconnected' to the callers of CompareDOMPoints
and make them deal with it properly.  I'd like to fix the crash here first
though and do that in a followup bug.  Should I leave nsSelection.cpp as is
and just let it assert then?
Yeah, that sounds like a better idea than just hiding the problem.
Posted patch Patch rev. 2Splinter Review
Same patch without the nsSelection.cpp change.
Attachment #344316 - Attachment is obsolete: true
Attachment #344954 - Flags: superreview?(bzbarsky)
Attachment #344954 - Flags: review?(bzbarsky)
Attachment #344316 - Flags: superreview?(bzbarsky)
Attachment #344316 - Flags: review?(bzbarsky)
Comment on attachment 344954 [details] [diff] [review]
Patch rev. 2

Please add a test.
Attachment #344954 - Flags: superreview?(bzbarsky)
Attachment #344954 - Flags: superreview+
Attachment #344954 - Flags: review?(bzbarsky)
Attachment #344954 - Flags: review+
http://hg.mozilla.org/mozilla-central/rev/6a229538f526

I'm holding the crashtest until 1.9.0.x is released with a fix.

-> FIXED
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: mozilla1.9.1 → mozilla1.9.1b2
I filed bug 462897 on the remaining assertion.
Flags: blocking1.9.0.6?
Mats: Does this patch apply to 1.9.0? Can you please request approval if so?
Flags: blocking1.9.0.6?
Flags: blocking1.9.0.7?
Comment on attachment 344954 [details] [diff] [review]
Patch rev. 2

Yes, it applies and fixes the crash. (sorry, I forgot)
Attachment #344954 - Flags: approval1.9.0.7?
Flags: blocking1.9.0.7? → blocking1.9.0.7+
Comment on attachment 344954 [details] [diff] [review]
Patch rev. 2

Approved for 1.9.0.7, a=dveditz for release-drivers.
Attachment #344954 - Flags: approval1.9.0.7? → approval1.9.0.7+
Landed on CVS trunk for 1.9.0.7:
mozilla/content/base/src/nsContentUtils.cpp 	1.311
Keywords: fixed1.9.0.7
The testcase does not crash 1.8 nor does the patched code seem to exist (unconnected ranges don't seem to exist until bug 409380)
Flags: wanted1.8.1.x-
Whiteboard: [sg:critical?] → [sg:critical?] post 1.8-branch
The testcase doesn't crash 1.9.0.6 (non-debug). 

Tomcat, can you run the testcase with a debug 1.9.0.7 nightly? (If you have a debug 1.9.0.6 final, that would be good as well).
verified 1.9.0.7 using the testcase and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.7pre) Gecko/2009021407 Firefox/3.0.7pre 

i don't see the crash on this debug build, but ###!!! ASSERTION: unexpected disconnected nodes: 'aDisconnected', file /work/mozilla/builds/1.9.0/mozilla/content/base/src/nsContentUtils.cpp, line 1573
Group: core-security
crash test added
http://hg.mozilla.org/mozilla-central/rev/1824b971b236
Flags: in-testsuite? → in-testsuite+
Component: DOM: Traversal-Range → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.