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

RESOLVED FIXED in mozilla1.9.1b2

Status

()

Core
DOM: Core & HTML
P1
critical
RESOLVED FIXED
10 years ago
5 years ago

People

(Reporter: Jesse Ruderman, Assigned: mats)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla1.9.1b2
assertion, crash, testcase, verified1.9.0.7
Points:
---
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)

(Reporter)

Description

10 years ago
Created attachment 310912 [details]
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.
(Reporter)

Comment 1

10 years ago
Created attachment 310913 [details]
stack traces for the assertions
(Reporter)

Updated

10 years ago
Whiteboard: [sg:low?]
(Reporter)

Updated

10 years ago
Flags: blocking1.9?
Given low severity, this can wait for a 1.9.0.x release.

Comment 3

10 years ago
Given comment 2 moving to wanted list.
Flags: tracking1.9+
Flags: blocking1.9?
Flags: blocking1.9-
Flags: wanted1.9.0.x?

Updated

10 years ago
Flags: tracking1.9+
Flags: wanted1.9.0.x? → wanted1.9.0.x+
(Reporter)

Updated

9 years ago
Flags: blocking1.9.1?

Updated

9 years ago
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?]
(Reporter)

Comment 5

9 years ago
I had set this as sg:low because it looked like a read (rather than a write) out of bounds.
(Assignee)

Comment 6

9 years ago
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
(Assignee)

Comment 7

9 years ago
Created attachment 344316 [details] [diff] [review]
Patch rev. 1

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?
(Assignee)

Comment 9

9 years ago
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.
(Assignee)

Comment 11

9 years ago
Created attachment 344954 [details] [diff] [review]
Patch rev. 2

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+
(Assignee)

Comment 13

9 years ago
Created attachment 345945 [details] [diff] [review]
crashtest.diff
(Assignee)

Comment 14

9 years ago
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
Last Resolved: 9 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: mozilla1.9.1 → mozilla1.9.1b2
(Reporter)

Comment 15

9 years ago
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?
(Assignee)

Comment 17

9 years ago
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+
(Assignee)

Comment 19

9 years ago
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
Keywords: fixed1.9.0.7 → verified1.9.0.7
Group: core-security

Comment 23

9 years ago
crash test added
http://hg.mozilla.org/mozilla-central/rev/1824b971b236
Flags: in-testsuite? → in-testsuite+
Component: DOM: Traversal-Range → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.