Closed Bug 410228 Opened 12 years ago Closed 12 years ago

"ASSERTION: Bad offset calculations" with zwnj followed by text-transformed szlig

Categories

(Core :: Layout: Text and Fonts, defect, P2, critical)

x86
macOS
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: jruderman, Assigned: roc)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, crash, testcase, Whiteboard: [sg:critical])

Attachments

(3 files)

Loading 'testcase 1' triggers:

firefox-bin(1338,0xa000d000) malloc: ***  Deallocation of a pointer not malloced: 0x10001; This could be a double free(), or free() called with the middle of an allocated block; Try setting environment variable MallocHelp to see tools to help debug

###!!! ASSERTION: Bad offset calculations: 'offset == aDest->GetLength()', file /Users/jruderman/trunk/mozilla/layout/generic/nsTextRunTransformations.cpp, line 257

This can lead to crashes that look exploitable.
Flags: blocking1.9?
Just to be clear, I'm not sure 'testcase 1' can trigger a crash by itself.  I turned the assertion into an abort locally while I was making the testcase.
Whiteboard: [sg:critical]
The two 'S' characters are displayed using different fonts even though they're part of the same text node.
Assignee: nobody → roc
WFM, on Linux and MacOSX 10.5.1 (Intel) with a fresh profile.
Flags: blocking1.9? → blocking1.9+
Priority: -- → P2
testcase 2 looks like some kind of font-selection oddness.
InitTextRun with textrun dumping enabled:

InitTextRun 3695fc10 fontgroup 36960970 (arial,serif) len 3 TEXTRUN "‌SS" ENDTEXTRUN
InitTextRun font: ArialMT
InitTextRun 3695fc10 fontgroup 36960970 font 3f499450 match Times-Roman (1-2)
InitTextRun 3695fc10 fontgroup 36960970 font 34c7bf00 match ArialMT (3-1)

So the first zwnj is matching against Times because Arial doesn't contain a zwnj entry in its cmap table.  Any character following a zwnj will default to the font for the previous character, in this case Times.

http://mxr.mozilla.org/seamonkey/source/gfx/thebes/src/gfxAtsuiFonts.cpp#756

Note that this is specific to the version of Arial in use:

v.2.6 (10.4) no zwnj
v.3.05 (MS Office) has zwnj
v.5.01 (10.5) has zwnj

So you'll only see this funkiness on 10.4 systems without MS Office installed.

For the actual testcase, the assertion fires before font matching is done so I don't think that's the source of this problem:

###!!! ASSERTION: Bad offset calculations: 'offset == aDest->GetLength()', file /builds/anontrunk/mozilla/layout/generic/nsTextRunTransformations.cpp, line 257
InitTextRun 34c2fe00 fontgroup 3694f3c0 (arial,serif) len 2 TEXTRUN "Ā" ENDTEXTRUN
InitTextRun font: ArialMT
InitTextRun 34c2fe00 fontgroup 3694f3c0 font 3695a5d0 match Times-Roman (1-2)

Is there a real-world situation where a zwnj appears at the beginning of a text run?

This error gets dumped to the console:

firefox-bin(3932,0xa000d000) malloc: ***  Deallocation of a pointer not malloced: 0x2; This could be a double free(), or free() called with the middle of an allocated block; Try setting environment variable MallocHelp to see tools to help debug


I understand the first testcase and nearly have a fix. The code in nsTextRunTransformations is incorrectly handling the case where there's a font change in the middle of the uppercased szlig.
> So the first zwnj is matching against Times because Arial doesn't contain a
> zwnj entry in its cmap table.  Any character following a zwnj will default to
> the font for the previous character, in this case Times.

Is this the right thing to do?  It seems strange to me, since zwnj isn't displayed.

> Is there a real-world situation where a zwnj appears at the beginning of a 
> text run?

I don't know.  This bug was found through fuzzing :)
This code was taken from the Windows font matching code so the best person to ask about this is Stuart, the code is the same on Windows so if you find a font that doesn't define zwnj in its cmap you'll see the same behavior.  Verdana should work.  In general though, the idea is that you don't want to change fonts when joiners/non-joiners are involved.  Not 100% correct according to the CSS spec but I'm 100% certain that the folks writing the CSS spec never thought about this situation.
(In reply to comment #7)
> Is this the right thing to do?  It seems strange to me, since zwnj isn't
> displayed.

The idea is that any sequence of characters "xyz", where y is either zwj or zwnj, should be displayed in the same font. Otherwise there are lots of shaping errors in complex text.
This just does what my existing comment claimed the code already did. There is no point in trying to get this "right", that would be pretty hard and this testcase exists only in Jesse's demonic little world
Attachment #296112 - Flags: review?(smontagu)
Whiteboard: [sg:critical] → [sg:critical][needs review]
(In reply to comment #5)
> Is there a real-world situation where a zwnj appears at the beginning of a text
> run?

I doubt it, but there are certainly real-world situations where a zwj appears at the beginning of a text run, which causes exactly the same bug.
Attachment #296112 - Flags: review?(smontagu) → review+
> The idea is that any sequence of characters "xyz", where y is either zwj or
> zwnj, should be displayed in the same font. Otherwise there are lots of shaping
> errors in complex text.

I don't understand why this is true for zwnj, but I guess it makes sense for zwj.
One example where it's true for zwnj is Malayalam, where zwnj is used to distinguish letters with visible viramas from chillu forms.
checked in
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [sg:critical][needs review] → [sg:critical]
checked in crashtest.
Flags: in-testsuite+
verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2008011009 Firefox/3.0b3pre ID:2008011009 and the testcases from jesse
Status: RESOLVED → VERIFIED
This bug doesn't affect the 1.8 branch.
Group: security
Flags: wanted1.8.1.x-
You need to log in before you can comment on or make changes to this bug.