499862 - "ASSERTION: invalid array index" with overflow:scroll, float:left, text-transform, changing <style>

Status: RESOLVED FIXED

(Core :: Layout, defect, P2)

Description

[Jesse Ruderman]

Attachment: testcase

###!!! ASSERTION: Should have been cleared: 'mBreakSinks.IsEmpty()', file layout/generic/nsTextFrameThebes.cpp, line 648

###!!! ASSERTION: Should have Reset() before destruction!: 'mCurrentWord.Length() == 0', file content/base/src/nsLineBreaker.cpp, line 51

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

###!!! ASSERTION: Hmm, something went wrong, aOffset should have been found: 'mGlyphRuns[start].mCharacterOffset <= aOffset', file gfx/thebes/src/gfxFont.cpp, line 2189

Security-sensitive because the "invalid array index" assertion is in an unchecked array access function.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

We need to flush out break sinks etc even if there are no mapped flows. In this testcase, the textrun finishes after the T (since it's in a float by itself) so we exit too early from FlushFrames. Flushing out the break sinks is needed so that we tell the text-transform textrun where the capitalized characters are.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: simpler testcase

This testcase is simpler but still fires the first assertion.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/mozilla-central/rev/9c24556c14c3

I checked in my simple testcase, which doesn't lead directly to a security issue.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 428646 [details] [diff] [review]
fix

Patch should apply to all 1.9.x branches.

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 428646 [details] [diff] [review]
fix

a=beltzner for all branches

Comment 6

[timeless]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/60dce6cf3aaa
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/5d3418e69ab0

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Checked into 1.9.0.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I backed this out of 1.9.0 because it caused crashes there. Is it worth figuring those out, or should we just leave this unfixed on 1.9.0?

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 428646 [details] [diff] [review]
fix

I'm fine to leave these unfixed, yeah. It does mean we can't open up this bug for a while longer, though.

Comment 10

[Al Billings [:abillings - ex-MoCo]]

Verified for 1.9.1 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.9pre) Gecko/20100311 Shiretoko/3.5.9pre (.NET CLR 3.5.30729) using testcase.

Verified for 1.9.2 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.3pre) Gecko/20100315 Namoroka/3.6.3pre (.NET CLR 3.5.30729). (This was built just after the release branch was cut but before any checkins were made so it is the same as 1.9.2.2.)