Closed Bug 306902 Opened 19 years ago Closed 18 years ago

MathML crash [@ szone_free] [@ nsUnicodeRenderingToolkit::GetTextSegmentBoundingMetrics] [@ SetFontCharsetInfo] [@ OutlineMetrics]

Categories

(Core :: MathML, defect)

PowerPC
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED

People

(Reporter: jruderman, Assigned: jaas)

References

Details

(4 keywords, Whiteboard: [sg:critical?] See bug 366493 for 1.8 branch null deref, bug 365923 for GTK1 crash)

Crash Data

Attachments

(6 files, 2 obsolete files)

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20050901
Firefox/1.6a1

1. Load the testcase.
2. Dismiss the "missing MathML fonts" dialog when it appears.

Result: Firefox crashes while the status bar counter says "1400".

Filing as security-sensitive becasue the testcase includes code from bug 306663.
Attached file testcase (not reduced)
Same crash on Gecko 1.8 branch.
Still crashes with the same signature with an opt trunk build from a few hours ago.
The crash also occurs in a GTK1 debug build, but not GTK2 (neither with MathML).
Summary: MathML/StirDOM crash [@ szone_free] [@ nsUnicodeRenderingToolkit::GetTextSegmentBoundingMetrics] → MathML/StirDOM crash [@ szone_free] [@ nsUnicodeRenderingToolkit::GetTextSegmentBoundingMetrics] [@ SetFontCharsetInfo]
Attached file Stack + data
This fixes my crash.
Just switch to nsFontGTK::SupportsChar(). It already has the null-check.
Whiteboard: [patch] for GTK crash only
Assignee: rbs → bugmail
I still see the Mac crash (comment 0) with yesterday's trunk nightly.  It happens while the status bar counter says 2400 now, though.
Attached file simple testcase for the Mac crash (obsolete) —
You might have to reload the testcase to trigger the crash.

The GTK patch should probably be moved into another bug.
Keywords: testcase
Summary: MathML/StirDOM crash [@ szone_free] [@ nsUnicodeRenderingToolkit::GetTextSegmentBoundingMetrics] [@ SetFontCharsetInfo] → MathML/StirDOM crash [@ szone_free] [@ nsUnicodeRenderingToolkit::GetTextSegmentBoundingMetrics] [@ SetFontCharsetInfo] [@ OutlineMetrics]
I can not reproduce this on windows. The stack jesse showed me seemed to indicate that some mac api was unable to deal with measuring too long words.

What surpriced me a lot though is that it made a difference if the node was a cdata node or a text node. That should never make a difference in which code paths we're executing.
Sorry, I was wrong; it crashes with a normal text node too.  I'm not sure how I convinced myself that it had to be a CDATA node.
Same testcase without CDATA markers.
Attachment #209675 - Attachment is obsolete: true
Do we need to get someone from Apple to look at the simple testcase and crash?
With the switch to Thebes/Cairo, we won't be using this Apple API at all, so maybe this crash will just go away (at least on trunk).
That won't happen on branch though, and given that this crashes due to a long string i get sort of worried about it being a buffer overrun.
It looks like the patch at bug 121540 solves this. But this is really odd, because  bounding metrics measurement is one of the parts left almost untouched by that.
Josh, can you take this bug?
Assignee: bugmail → nobody
Assignee: nobody → joshmoz
Whiteboard: [patch] for GTK crash only → [sg:investigate][patch] for GTK crash only
Depends on: atsui
OS: Mac OS X 10.2 → Mac OS X 10.4
Whiteboard: [sg:investigate][patch] for GTK crash only → [sg:critical?] [patch for GTK crash only]
Fixed by the switch to cairo on Mac, as expected:
* Crashes a 2006-11-21 nightly.
* Does not crash a 2006-11-25 nightly.
* Does not crash fresh opt and debug builds.

Mats, please move your GTK patch to another bug if it is still needed.
Status: NEW → RESOLVED
Closed: 18 years ago
Depends on: 323934
Resolution: --- → FIXED
Whiteboard: [sg:critical?] [patch for GTK crash only] → [sg:critical?] Stir DOM testcase
(In reply to comment #20)
> Mats, please move your GTK patch to another bug if it is still needed.

It still crashes. I'm pretty sure it's gtk1 or xlib widget only.
It's a SEGV crash from a read. I'm not sure how much of a priority that is.

The bug seems to have changed from the last time (attachment 197271 [details])
The crash now occurs when calling CCMAP_HAS_CHAR_EXT(m,c) when 'c' is a
surrogate character and 'm' is 'gDoubleByteSpecialCharsCCMap' which comes
from a generated file ("dbyte_special_chars.ccmap"):
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/gfx/src/gtk/nsFontMetricsGTK.cpp&rev=1.289&root=/cvsroot&mark=872-873#865
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/gfx/src/x11shared/dbyte_special_chars.ccmap&rev=1.5&root=/cvsroot

It could mean that the ccmap generator has a bug or CCMAP_HAS_CHAR_EXT
has bug, both are used on other (modern) platforms too I think.

Or maybe CCMAP_HAS_CHAR_EXT is just used inappropriately here:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/gfx/src/gtk/nsFontMetricsGTK.cpp&rev=1.289&root=/cvsroot&mark=4112,4122#4089
(We crash on line 4122, with 'c' == 120793, which was assigned on line 4112
from aString[i]=55349 and aString[i+1]=57305.)

Some more debug data, for posterity:
gDoubleByteSpecialCharsCCMap c=120793 (0x1d7d9) (c)&0xffff0000 is true (0x10000)
extraSurrogateLength=1 , aString[i]=55349 aString[i+1]=57305
ALU_SIZE=32 CCMAP_SIZE(gDoubleByteSpecialCharsCCMap)=16361
(CCMAP_FLAG(m) & CCMAP_SURROGATE_FLAG) is true
CCMAP_PLANE(c) = 1
CCMAP_TO_PAGE((m),(c)) = 32
CCMAP_ALU_INDEX(c & 0xffff) = 6
(CCMAP_TO_ALU(m, c & 0xffff) = 0
CCMAP_BIT_INDEX(c & 0xffff) = 25
(CCMAP_TO_ALU(m, c & 0xffff) >> CCMAP_BIT_INDEX(c & 0xffff) = 0
CCMAP_FOR_PLANE_EXT(gDoubleByteSpecialCharsCCMap, CCMAP_PLANE(c)) = 0xf6e1c24 (-824388892 bytes off gDoubleByteSpecialCharsCCMap)
Program received signal SIGSEGV, Segmentation fault.

Maybe Talkback can enlighten us if this a problem on other platforms as well?
(analyzing consumers of SupportsChar() and CCMAP_HAS_CHAR_EXT)
Filed bug 365923 on the remaining problem described in comment 21.
The original unreduced testcase still crashes both FF1.5.0.10pre and FF2.0.0.2pre. Filed bug 366493 since the crash is nothing like the one fixed in this bug.
Whiteboard: [sg:critical?] Stir DOM testcase → [sg:critical?] Stir DOM testcase. See bug 366493 for 1.8 branch
The reduced testcase still crashes Fx 2.0.0.3 (and I'd guess 1.5.0.11). I know that you filed bug 366493 on the original testcase, but should this bug be nominated as well for blocking?
Does the crash look like 366493? Does it match the apple stack traces in this bug? If neither we probably need a new bug.
Summary: MathML/StirDOM crash [@ szone_free] [@ nsUnicodeRenderingToolkit::GetTextSegmentBoundingMetrics] [@ SetFontCharsetInfo] [@ OutlineMetrics] → MathML crash [@ szone_free] [@ nsUnicodeRenderingToolkit::GetTextSegmentBoundingMetrics] [@ SetFontCharsetInfo] [@ OutlineMetrics]
Whiteboard: [sg:critical?] Stir DOM testcase. See bug 366493 for 1.8 branch → [sg:critical?] See bug 366493 for 1.8 branch null deref
Attached file Crash log from Fx 2.0.0.3 (obsolete) —
Here's the log I got using 2.0.0.3 with the simplified testcase. They look very similar to me as the initial crash log above...
Whoops. Wrong log. Here's the real one...
Attachment #259624 - Attachment is obsolete: true
Whiteboard: [sg:critical?] See bug 366493 for 1.8 branch null deref → [sg:critical?] See bug 366493 for 1.8 branch null deref, bug 365923 for GTK1 crash
Comment on attachment 194712 [details]
testcase (not reduced)

Sorry for marking this private prematurely. I was going to open the bug and must have noticed the spin-off bugs and stopped half-way
(In reply to comment #20)
> Fixed by the switch to cairo on Mac, as expected:

Which is great for the trunk, but leaves an sg:critical bug on the branch. Still need a patch.
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Flags: blocking1.8.1.4?
Flags: blocking1.8.0.12?
Flags: blocking1.8.1.4?
Flags: blocking1.8.1.4+
Flags: blocking1.8.0.12?
Flags: blocking1.8.0.12+
I don't know much of anything about our font code, and unfortunately the people who wrote that code are no longer around. I'll take a look or try to find someone better suited for this bug.
Won't this potentially exploitable crash affect branch-based Camino as well? Maybe someone on the Camino team has the font expertise we need. Guess it's not likely to make this set of releases, though -- moving out.
Flags: blocking1.8.1.5+
Flags: blocking1.8.1.4+
Flags: blocking1.8.0.13+
Flags: blocking1.8.0.12+
Attached patch fixSplinter Review
No domain knowledge needed, as it turns out. This is just a simple smasher from what looks like a copy-and-paste bug; Rect is 8 bytes, Fixed is 4, so the rects array was half the length it needed to be.
Attachment #262211 - Flags: review?(joshmoz)
Comment on attachment 262211 [details] [diff] [review]
fix

Great!
Attachment #262211 - Flags: review?(joshmoz) → review+
Flags: blocking1.8.1.4?
Attachment #262211 - Flags: superreview?(vladimir)
Attachment #262211 - Flags: superreview?(vladimir) → superreview+
Comment on attachment 262211 [details] [diff] [review]
fix

Requesting approval on this patch for the 1.8.0 and 1.8 branches.
Attachment #262211 - Flags: approval1.8.1.4?
Attachment #262211 - Flags: approval1.8.0.12?
Comment on attachment 262211 [details] [diff] [review]
fix

approved for 1.8.0.12 and 1.8.1.4, a=dveditz
Attachment #262211 - Flags: approval1.8.1.4?
Attachment #262211 - Flags: approval1.8.1.4+
Attachment #262211 - Flags: approval1.8.0.12?
Attachment #262211 - Flags: approval1.8.0.12+
Flags: blocking1.8.1.5+
Flags: blocking1.8.1.4?
Flags: blocking1.8.1.4+
Flags: blocking1.8.0.13+
Flags: blocking1.8.0.12+
Landed on MOZILLA_1_8_BRANCH and MOZILLA_1_8_0_BRANCH.
Stuart - can you please land this on trunk even though it isn't used there?
Landed on trunk.
verified on the 1.8 branch using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.4pre) Gecko/2007050804 BonEcho/2.0.0.4pre. I don't crash with the simple testcases. I do see what dveditz notes in Comment 23, and my stack appears to match what appears in Bug 366493. Adding branch verified keyword.
verified on the 1.8.0 branch using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.12pre) Gecko/20070508 Firefox/1.5.0.12pre. I don't crash with
the simple testcases. I do see what dveditz notes in Comment 23, and my stack
appears to match what appears in Bug 366493. Adding branch verified keyword.
Group: security
Flags: in-testsuite?
v.fixed with latest trunk nightly
Status: RESOLVED → VERIFIED
Crashtest is in: gfx/src/mac/crashtests/306902-1.xml
Flags: in-testsuite? → in-testsuite+
Jesse, was your crashtest supposed to land in gfx/src/mac on trunk? Just wondering because I thought gfx/src/mac was going away (bug 376791).
That's where the patch was.  Want me to move the test to gfx/src/thebes/ or something?
I can't judge if it still makes sense to keep this test but if so then gfx/src/thebes/ is probably a good location.
Test moved.
Crash Signature: [@ szone_free] [@ nsUnicodeRenderingToolkit::GetTextSegmentBoundingMetrics] [@ SetFontCharsetInfo] [@ OutlineMetrics]
You need to log in before you can comment on or make changes to this bug.