"ASSERTION: detailed glyph record missing!" uppercasing ß, followed by zwsp

RESOLVED FIXED

Status

()

Core
Graphics
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: Jesse Ruderman, Assigned: jfkthame)

Tracking

(Blocks: 1 bug, {assertion, regression, testcase})

Trunk
x86
Mac OS X
assertion, regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 final+)

Details

Attachments

(4 attachments)

(Reporter)

Description

7 years ago
Created attachment 511506 [details]
testcase

###!!! ASSERTION: detailed glyph record missing!: 'mLastUsed != nsTArray<DGRec>::NoIndex', file gfxFont.h, line 2105

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

Seems to be a regression from bug 631035.  Security-sensitive because it's using nsTArray's ElementAt (not SafeElementAt).

It seems to be important that the second character is ß (which uppercases to SS) and the third character is non-rendered (U+200B ZWSP or U+2061 FUNCTION APPLICATION).  I don't know what the deal with the first character is; on my machine it's a hexbox.
(Reporter)

Updated

7 years ago
Group: core-security
(Reporter)

Comment 1

7 years ago
Created attachment 511507 [details]
stack trace
(Reporter)

Updated

7 years ago
blocking2.0: --- → ?
(Assignee)

Comment 2

7 years ago
Yes, this is certainly a regression from bug 631035; I'll dig into it ASAP. Thanks for the testcase!

This probably accounts for the crashes reported in bug 633453.
Assignee: nobody → jfkthame
(Assignee)

Comment 3

7 years ago
Created attachment 511675 [details] [diff] [review]
patch, ensure GetDetailedGlyphs is not called when glyph count is zero

The DetailedGlyphStore is not intended to be called for character indexes that don't have any detailed glyphs; callers are expected to check this before trying to retrieve the DetailedGlyphs pointer. I missed one in MergeCharactersInTextRun().

So the real change here is just to check GetGlyphCount() in MergeCharactersInTextRun(). I've checked that all other current callers of GetDetailedGlyphs look correct, but I've also added extra comments and assertion checks at GetDetailedGlyphs, to help us catch any future issues quickly.

We should add this example to crashtests, too.
Attachment #511675 - Flags: review?(roc)
(Assignee)

Comment 4

7 years ago
Created attachment 511677 [details] [diff] [review]
patch, add the testcase to gfx/tests/crashtests
Attachment #511677 - Flags: review?(roc)
(Assignee)

Comment 5

7 years ago
Moving this to Graphics, as it's a gfx bug rather than layout.

(In reply to comment #2)
> This probably accounts for the crashes reported in bug 633453.
FTR, that turned out to be a separate error, though they're both regressions from 631035.
Component: Layout: Text → Graphics
QA Contact: layout.fonts-and-text → thebes
blocking2.0: ? → final+
(Assignee)

Comment 6

7 years ago
http://hg.mozilla.org/mozilla-central/rev/fcf6c9b3bd7d (patch)
http://hg.mozilla.org/mozilla-central/rev/f36e81d4d60d (crashtest)
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Group: core-security
You need to log in before you can comment on or make changes to this bug.