Exploitable - User Mode Write AV starting at USP10!LoadCmapFontGlyphs+0x0000000000000059

RESOLVED FIXED

Status

()

Core
Graphics
P1
critical
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: Tomcat, Assigned: jtd)

Tracking

({crash, testcase, verified1.9.1})

1.9.1 Branch
x86
Windows XP
crash, testcase, verified1.9.1
Points:
---
Bug Flags:
blocking1.9.2 +
wanted1.9.0.x -

Firefox Tracking Flags

(status1.9.2 beta1-fixed, blocking1.9.1 .4+, status1.9.1 .4-fixed)

Details

(Whiteboard: [sg:vector-critical][MS ref: 9458mh --keep private until their patch, ETA ~May 2010], URL)

Attachments

(6 attachments, 1 obsolete attachment)

(Reporter)

Description

8 years ago
Created attachment 400764 [details]
more detailed information (stack) from windbg/!exploitable

1.9.1 Nightly Debug Build
steps to reproduce:
-> Load http://www.laserwords.com/commercial.php in a debugger
--> !exploitable report a problem

93c.c18): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
eax=ffff0104 ebx=0519e7c8 ecx=00000000 edx=00baad04 esi=05196578 edi=ffff0105
eip=74da6e19 esp=00126448 ebp=00126458 iopl=0         nv up ei ng nz na pe nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00010286
USP10!LoadCmapFontGlyphs+0x59:
74da6e19 66893c53        mov     word ptr [ebx+edx*2],di  ds:0023:068f41d0=????
0:000> !exploitable -v
HostMachine\HostUser
Executing Processor Architecture is x86
Debuggee is in User Mode
Debuggee is a live user mode debugging session on the local machine
Event Type: Exception
Exception Faulting Address: 0x68f41d0
First Chance Exception Type: STATUS_ACCESS_VIOLATION (0xC0000005)
Exception Sub-Type: Write Access Violation

Exception Hash (Major/Minor): 0x287e0601.0x7074110b

Stack Trace:
USP10!LoadCmapFontGlyphs+0x59
USP10!FillCMAP+0x36
USP10!LoadFont+0x1de
USP10!FindOrCreateFaceCache+0x9c
USP10!FindOrCreateSizeCacheWithoutRealizationID+0xc7
USP10!FindOrCreateSizeCacheUsingRealizationID+0x5d
USP10!UpdateCache+0x2c
USP10!ScriptCheckCache+0x67
USP10!ScriptShape+0x88
thebes!UniscribeItem::ShapeUniscribe+0x11a
thebes!UniscribeItem::Shape+0x3a
thebes!gfxWindowsFontGroup::InitTextRunUniscribe+0x19b
thebes!gfxWindowsFontGroup::MakeTextRun+0x169
thebes!TextRunWordCache::MakeTextRun+0x7bb
thebes!gfxTextRunWordCache::MakeTextRun+0x2f
gklayout!MakeTextRun+0x77
gklayout!BuildTextRunsScanner::BuildTextRunForFrames+0xcef
gklayout!BuildTextRunsScanner::FlushFrames+0x1f6
gklayout!BuildTextRuns+0x656
gklayout!nsTextFrame::EnsureTextRun+0xb2
gklayout!nsTextFrame::Reflow+0x420
gklayout!nsLineLayout::ReflowFrame+0x3ab
gklayout!nsBlockFrame::ReflowInlineFrame+0x5e
gklayout!nsBlockFrame::DoReflowInlineFrames+0x210
gklayout!nsBlockFrame::ReflowInlineFrames+0xf2
gklayout!nsBlockFrame::ReflowLine+0x2c2
gklayout!nsBlockFrame::ReflowDirtyLines+0x561
gklayout!nsBlockFrame::Reflow+0x251
gklayout!nsBlockReflowContext::ReflowBlock+0x1a3
gklayout!nsBlockFrame::ReflowBlockFrame+0x6b3
gklayout!nsBlockFrame::ReflowLine+0xd2
gklayout!nsBlockFrame::ReflowDirtyLines+0x561
gklayout!nsBlockFrame::Reflow+0x251
gklayout!nsBlockReflowContext::ReflowBlock+0x1a3
gklayout!nsBlockFrame::ReflowBlockFrame+0x6b3
gklayout!nsBlockFrame::ReflowLine+0xd2
gklayout!nsBlockFrame::ReflowDirtyLines+0x561
gklayout!nsBlockFrame::Reflow+0x251
gklayout!nsBlockReflowContext::ReflowBlock+0x1a3
gklayout!nsBlockFrame::ReflowBlockFrame+0x6b3
gklayout!nsBlockFrame::ReflowLine+0xd2
gklayout!nsBlockFrame::ReflowDirtyLines+0x561
gklayout!nsBlockFrame::Reflow+0x251
gklayout!nsBlockReflowContext::ReflowBlock+0x1a3
gklayout!nsBlockFrame::ReflowBlockFrame+0x6b3
gklayout!nsBlockFrame::ReflowLine+0xd2
gklayout!nsBlockFrame::ReflowDirtyLines+0x561
gklayout!nsBlockFrame::Reflow+0x251
gklayout!nsBlockReflowContext::ReflowBlock+0x1a3
gklayout!nsBlockFrame::ReflowBlockFrame+0x6b3
gklayout!nsBlockFrame::ReflowLine+0xd2
gklayout!nsBlockFrame::ReflowDirtyLines+0x561
gklayout!nsBlockFrame::Reflow+0x251
gklayout!nsBlockReflowContext::ReflowBlock+0x1a3
gklayout!nsBlockFrame::ReflowBlockFrame+0x6b3
gklayout!nsBlockFrame::ReflowLine+0xd2
gklayout!nsBlockFrame::ReflowDirtyLines+0x561
gklayout!nsBlockFrame::Reflow+0x251
gklayout!nsBlockReflowContext::ReflowBlock+0x1a3
gklayout!nsBlockFrame::ReflowBlockFrame+0x6b3
gklayout!nsBlockFrame::ReflowLine+0xd2
gklayout!nsBlockFrame::ReflowDirtyLines+0x561
gklayout!nsBlockFrame::Reflow+0x251
gklayout!nsContainerFrame::ReflowChild+0xe9
Instruction Address: 0x0000000074da6e19

Description: User Mode Write AV
Short Description: WriteAV
Exploitability Classification: EXPLOITABLE
Recommended Bug Title: Exploitable - User Mode Write AV starting at USP10!LoadCmapFontGlyphs+0x0000000000000059 (Hash=0x287e0601.0x7074110b)

User mode write access violations that are not near NULL are exploitable.
(Reporter)

Comment 1

8 years ago
oh and also crash the debug build when loading the url

(9e0.6c4): Access violation - code c0000005 (!!! second chance !!!)
eax=ffff0104 ebx=051bef50 ecx=00000000 edx=00ae0004 esi=051be1f0 edi=ffff0105
eip=74da6e19 esp=00126448 ebp=00126458 iopl=0         nv up ei ng nz na pe nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00000286
USP10!ScriptApplyDigitSubstitution+0x21a7:
74da6e19 66893c53        mov     word ptr [ebx+edx*2],di  ds:0023:0677ef58=????
0:000> cdb: Reading initial command '!load winext\msec.dll;.logappend;!exploitab
le;k;q'
Opened log file 'dbgeng.log'
Exploitability Classification: EXPLOITABLE
Recommended Bug Title: Exploitable - User Mode Write AV starting at USP10!Script
ApplyDigitSubstitution+0x00000000000021a7 (Hash=0x5819333f.0x704a675e)

User mode write access violations that are not near NULL are exploitable.
ChildEBP RetAddr
WARNING: Stack unwind information not available. Following frames may be wrong.
00126458 74da7de3 USP10!ScriptApplyDigitSubstitution+0x21a7
00126480 74da833e USP10!ScriptApplyDigitSubstitution+0x3171
001266f0 74da506a USP10!ScriptApplyDigitSubstitution+0x36cc
00126710 74da581a USP10!ScriptApplyDigitSubstitution+0x3f8
00126810 74da5a8e USP10!ScriptApplyDigitSubstitution+0xba8
00126834 74da5ea3 USP10!ScriptApplyDigitSubstitution+0xe1c
0012684c 74da5f8b USP10!ScriptApplyDigitSubstitution+0x1231
0012686c 74da2ea2 USP10!ScriptApplyDigitSubstitution+0x1319
0012688c 01ef309a USP10!ScriptShape+0x88
001268d4 01ef2f6a thebes!UniscribeItem::ShapeUniscribe+0x11a
001268e0 01ef2d3b thebes!UniscribeItem::Shape+0x3a
00126944 01ef10c9 thebes!gfxWindowsFontGroup::InitTextRunUniscribe+0x19b
00126a10 01edfadb thebes!gfxWindowsFontGroup::MakeTextRun+0x169
00126ff8 01ee06ff thebes!TextRunWordCache::MakeTextRun+0x7bb
00127014 02b9ccd7 thebes!gfxTextRunWordCache::MakeTextRun+0x2f
00127044 02b9c79f gklayout!MakeTextRun+0x77
00128494 02b9b106 gklayout!BuildTextRunsScanner::BuildTextRunForFrames+0xcef
001294bc 02b9e006 gklayout!BuildTextRunsScanner::FlushFrames+0x1f6
00129858 02b9d6e2 gklayout!BuildTextRuns+0x656
001298c8 02ba80a0 gklayout!nsTextFrame::EnsureTextRun+0xb2
quit:
status1.9.1: --- → ?
Whiteboard: crash
(Reporter)

Comment 2

8 years ago
and also crashes opt builds on windows -> http://crash-stats.mozilla.com/report/index/1e098a17-86b0-4f0a-ade3-6fab02090915?p=1

Comment 3

8 years ago
usp10.dll is ms' uniscribe unicode service. dveditz, can you ping them?
Tomcat:
- Does this crash 1.9.2 or trunk?

This might be crashing on a bad font -- your font or one downloaded? We need to capture the problem page/font before we can send info to MS.

(Does this crash IE? they support some downloaded fonts too, though if it's not that they may not be calling Uniscribe the way we do.)
blocking1.9.1: --- → ?
status1.9.1: ? → wanted
Keywords: crash
Whiteboard: crash → [sg:critical]

Comment 5

8 years ago
we see this signature show up in both fx3.x and 3.5x, but looks like it might
be only on Windows NT 5.1.2600 Service Pack 2, and 3.

here are crash reports for this signature for sept.

   1 Firefox 3.0.13 Windows NT 5.1.2600 Service Pack 2
http://crash-stats.mozilla.com/report/index/3778fb4f-7255-4e74-90d6-e27942090906
   1 Firefox 3.0.13 Windows NT 5.1.2600 Service Pack 2
http://crash-stats.mozilla.com/report/index/3a6a3941-52f9-4ea5-8794-4a2242090911
   1 Firefox 3.0.13 Windows NT 5.1.2600 Service Pack 2
http://crash-stats.mozilla.com/report/index/aa929652-4243-4d54-b9a4-6a19d2090911
   1 Firefox 3.0.13 Windows NT 5.1.2600 Service Pack 3
http://crash-stats.mozilla.com/report/index/03fa0452-7e2d-4edf-8db5-2f6472090831
   1 Firefox 3.0.13 Windows NT 5.1.2600 Service Pack 3
http://crash-stats.mozilla.com/report/index/7b959cf7-a376-4857-8b06-c29532090831
   1 Firefox 3.0.13 Windows NT 5.1.2600 Service Pack 3
http://crash-stats.mozilla.com/report/index/9fdb46f5-2472-48e8-ba6f-ca3962090902
   1 Firefox 3.0.7 Windows NT 5.1.2600 Service Pack 2
http://crash-stats.mozilla.com/report/index/e5cf1e54-10c5-499d-9e3e-850872090909
   1 Firefox 3.5.2 Windows NT 5.1.2600 Service Pack 2
http://crash-stats.mozilla.com/report/index/106d3ffa-110a-498b-bccd-19b362090909
   1 Firefox 3.5.2 Windows NT 5.1.2600 Service Pack 2
http://crash-stats.mozilla.com/report/index/2587fa38-3e09-403d-a875-390842090901
   1 Firefox 3.5.2 Windows NT 5.1.2600 Service Pack 2
http://crash-stats.mozilla.com/report/index/c10b00e3-345b-485e-a7cf-9e3f82090909
   1 Firefox 3.5.2 Windows NT 5.1.2600 Service Pack 2
http://crash-stats.mozilla.com/report/index/cb44d5bc-93ae-4208-abed-eb9c52090901
   1 Firefox 3.5.2 Windows NT 5.1.2600 Service Pack 3
http://crash-stats.mozilla.com/report/index/1c838e56-45b7-46e0-94fa-1c8122090905
   1 Firefox 3.5.2 Windows NT 5.1.2600 Service Pack 3
http://crash-stats.mozilla.com/report/index/39440237-edbf-47d5-be6d-9a2e92090901
   1 Firefox 3.5.2 Windows NT 5.1.2600 Service Pack 3
http://crash-stats.mozilla.com/report/index/80af3bbb-2f35-4185-a3d1-847a72090831
   1 Firefox 3.5.2 Windows NT 5.1.2600 Service Pack 3
http://crash-stats.mozilla.com/report/index/a878ac6f-8ba7-4865-a705-9ad6d2090903
   1 Firefox 3.5.2 Windows NT 5.1.2600 Service Pack 3
http://crash-stats.mozilla.com/report/index/c4bff7aa-af7c-4dfc-9f7d-cda9e2090903

I think bsterne is working on getting us a regular contact for windows crashes.
(Reporter)

Comment 6

8 years ago
(In reply to comment #4)
> Tomcat:
> - Does this crash 1.9.2 or trunk?
 
it crashes 1.9.2 -> http://crash-stats.mozilla.com/report/index/b03cd9fb-15b8-4dc7-8820-4dec12090915?p=1

was not able to crash on trunk so far on opt builds, but will build a new trunk debug build to check this. 

> This might be crashing on a bad font -- your font or one downloaded? We need to > capture the problem page/font before we can send info to MS.
> 

The VM where this Problem is reproducible is a "standard" en-US Win-XP with latest updates etc, no additional fonts - just the standard installation.

> (Does this crash IE? they support some downloaded fonts too, though if it's not
> that they may not be calling Uniscribe the way we do.)

No, does not crash with IE 8 (and latest updates).
(Reporter)

Updated

8 years ago
Flags: blocking1.9.2?
(Reporter)

Comment 7

8 years ago
Created attachment 400797 [details]
local copy of the page

zipped copy of this page, but was not able to reproduce this crash so far locally on this copy
I haven't looked yet, but that might point at downloadable fonts -- we have a same-origin restriction on those, and as a new feature probably aren't captured by the "save as webpage complete" feature.
Yeah, we needs to look through the source/stylesheets to see if there are DL fonts in use.
Created attachment 400829 [details]
The font (crashes mac, too)

There's the sncb.ttf font from lw.css -- it crashes my Mac "quick look viewer" too just accessing it in the finder.
(Assignee)

Comment 11

8 years ago
Created attachment 400960 [details]
testcase, self-contained using font data url

Attached is a testcase that causes the bug on trunk debug/opt.  Basically it loads and unloads the funky font via a data url to render random text to stress Uniscribe's handling of the font.

Looks like this is caused by a funky format-12 cmap, dumping the ttf out using ttx shows some funky codepoints:

      <map code="0x2265" name="greaterequal"/><!-- GREATER-THAN OR EQUAL TO -->
      <map code="0x25ca" name="lozenge"/><!-- LOZENGE -->
      <map code="0xf8ff" name="uniF8FF"/><!-- Private Use -->
      <map code="0xfb01" name="fi"/><!-- LATIN SMALL LIGATURE FI -->
      <map code="0xfb02" name="fl"/><!-- LATIN SMALL LIGATURE FL -->
      <map code="0xffff0104L" name=".notdef#2"/><!-- ???? -->
      <map code="0xffff0105L" name=".notdef#3"/><!-- ???? -->

My guess is Uniscribe is not sanity checking these code points.  I don't crash on the Mac but there are lots of funky error messages in the console:

Sep 16 15:56:45 john-daggetts-mac-pro [0x0-0x988988].org.mozilla.firefox[0]: firefox-bin(78139,0xa04a3720) malloc: *** mmap(size=67108864) failed (error code=12)
Sep 16 15:56:45 john-daggetts-mac-pro [0x0-0x988988].org.mozilla.firefox[0]: *** error: can't allocate region
Sep 16 15:56:45 john-daggetts-mac-pro [0x0-0x988988].org.mozilla.firefox[0]: *** set a breakpoint in malloc_error_break to debug

Need to make sure this is not us stomping on memory (for example, in our cmap handling code).  We read in cmaps on Mac/Windows so we should be able to exclude fonts with funky cmaps like this.

Note that the site in question uses a funky DL font but many of the Sept crash reports are from 3.0 which did not support DL fonts so it's a general font system problem.  Obviously DL fonts makes it more exploitable.

I'll see if I can set up an EOT testcase for testing with IE.
Assignee: nobody → jdaggett
Status: NEW → ASSIGNED
(Assignee)

Comment 12

8 years ago
Created attachment 400971 [details] [diff] [review]
patch, v.0.2, validate codepoint ranges in cmaps (mac/windows)

When reading in the cmap, check that codepoints in the cmap are within the valid range of Unicode codepoints (i.e. <= 0x10FFFF).  Also, distinguish from fonts that lack format 4 or format 12 cmaps from fonts that appear to have malformed cmaps.  On Windows, only do glyph-by-glyph fallback for non-malformed cmaps.

One other note, running the test on Windows 7 I don't see any crashes so I'm guessing this has been fixed in the latest versions of Uniscribe.

Win XP SP3 w/ updates: usp10 version 1.420.2600.5512
Windows 7 RC: usp10 version 1.626.7100.0
Attachment #400971 - Flags: review?
(Assignee)

Comment 13

8 years ago
Not sure about the Linux case, that will depend on Pango since we don't use our own cmap-handling code under Linux.  Karl, could you confirm?
(Assignee)

Updated

8 years ago
Attachment #400971 - Flags: review? → review?(jfkthame)
Comment on attachment 400971 [details] [diff] [review]
patch, v.0.2, validate codepoint ranges in cmaps (mac/windows)

> /**
>+ * gfx errors
>+ */
>+
>+/* nsIDeviceContext.h defines a set of printer errors  */
>+#define NS_ERROR_GFX_GENERAL_BASE (50) 
>+
>+/* Font cmap is strangely structured - avoid this font! */
>+#define NS_ERROR_GFX_CMAP_MALFORMED          \
>+  NS_ERROR_GENERATE_FAILURE(NS_ERROR_MODULE_GFX,NS_ERROR_GFX_GENERAL_BASE+1)

I wish there was a more central place to do this, like maybe an nsGfxErrors.h header in gfx/public. I guess it's not crucial for now, and this is really only used internally, but in principle I think the error codes should be collected somewhere easier to find.

>     const PRUint8 *groups = aBuf + OffsetGroups;
>     for (PRUint32 i = 0; i < numGroups; i++, groups += SizeOfGroup) {
>         const PRUint32 startCharCode = ReadLongAt(groups, GroupOffsetStartCode);
>         const PRUint32 endCharCode = ReadLongAt(groups, GroupOffsetEndCode);
>+        NS_ENSURE_TRUE(startCharCode <= CMAP_MAX_CODEPOINT && endCharCode <= CMAP_MAX_CODEPOINT, NS_ERROR_GFX_CMAP_MALFORMED);
>         aCharacterMap.SetRange(startCharCode, endCharCode);
>     }

Suggest making this check a little more thorough: check that startCharCode <= endCharCode, and check that startCharCode > the previous endCharCode. If either of these things fail, the cmap table is invalid.

Also please break the really long line.

>@@ -308,17 +309,17 @@ gfxFontUtils::ReadCMAPTableFormat4(PRUin
>             for (PRUint32 c = startCount; c <= endCount; ++c) {
>                 if (c == 0xFFFF)
>                     break;

Sanity-check these start/end values before the loop (actually, before the preceding "if (idRangeOffset == 0)" test). If startCount > endCount, or if startCount <= previous value of endCount, we should reject the cmap as invalid (not just skip the loop and proceed to the next segment).

>+                NS_ENSURE_TRUE((PRUint8*)gdata > aBuf && (PRUint8*)gdata < aBuf + aLength, NS_ERROR_GFX_CMAP_MALFORMED);

Overlong line.

>-        // ReadCMAP may change the values of mUnicodeFont and mSymbolFont
>-        if (NS_FAILED(::ReadCMAP(hdc, fe))) {
>+        rv = ::ReadCMAP(hdc, fe);
>+
>+        // ReadCMAP can fail but only handle failure cases when the font
>+        // did *not* have a cmap that appears to be malformed.  Uniscribe
>+        // can crash with corrupt cmaps.
>+        if (NS_FAILED(rv) && rv != NS_ERROR_GFX_CMAP_MALFORMED) {

What is the intended behavior here when ReadCMAP returns NS_ERROR_GFX_CMAP_MALFORMED?

AFAICS, we'd still end up using the font, just without a complete bitmap of "supported" characters. I think the proper response would be to delete the new FontEntry and return NULL from FontEntry::CreateFontEntry() instead -- if the font has a malformed cmap, we should reject it altogether rather than risk using it.
Created attachment 400990 [details]
ftdump -v (cmaps from FreeType)

I haven't been able to produce a crash on Linux with a 64-bit trunk debug build or a 32-bit firefox-3.5.3pre nightly from 2009-08-12, either on attachment 400960 [details] or on http://www.laserwords.com/commercial.php.

Yes, gfxPangoFonts.cpp (used in Linux builds) doesn't use the code modified by attachment 400971 [details] [diff] [review].  cmaps are read by FreeType (through fontconfig).

Here I've attached the FreeType's interpretation of the cmaps from ftdump -v.
(Reporter)

Updated

8 years ago
Keywords: testcase
Looks like FreeType is ignoring the format 12 cmap table (platform 3, encoding 10) completely; it's not mentioned in that dump at all. Presumably it determines that it is invalid and (silently?) discards it.
(Reporter)

Comment 17

8 years ago
(In reply to comment #12)
> One other note, running the test on Windows 7 I don't see any crashes so I'm
> guessing this has been fixed in the latest versions of Uniscribe.
> 
> Win XP SP3 w/ updates: usp10 version 1.420.2600.5512
> Windows 7 RC: usp10 version 1.626.7100.0

for reference i did also tests on vista business sp2 (and latest updates) and vista is also affected (http://crash-stats.mozilla.com/report/index/229eb045-cecd-4d03-85ca-077ab2090916?p=1)

usp10 version : 1.626.6002.18005
blocking1.9.1: ? → .4+
(Assignee)

Comment 18

8 years ago
Created attachment 401178 [details] [diff] [review]
patch, v.0.3d, update based on review comments

On Windows, fonts with bad cmaps will cause FontEntry creation to fail.  On the Mac, font entry creation will fail for downloadable fonts.  For platform fonts, the cmap read happens lazily so it's more work to clean it out so for now I just reset the character map so that the font will never be used.  Since this is a patch we need to push back into 1.9.1, I wanted to limit the scope somewhat.

Note: I also moved around the bitset include to where it should be, we should replace that eventually but not for this bug.
Attachment #400971 - Attachment is obsolete: true
Attachment #401178 - Flags: review?(jfkthame)
Attachment #400971 - Flags: review?(jfkthame)
(Assignee)

Comment 19

8 years ago
Try server builds here:

https://build.mozilla.org/tryserver-builds/jdaggett@mozilla.com-cmapcheck3/
(Reporter)

Comment 20

8 years ago
(In reply to comment #19)
> Try server builds here:
> 
> https://build.mozilla.org/tryserver-builds/jdaggett@mozilla.com-cmapcheck3/

Hi John, this tryserver builds worked fine on windows - was not able to crash on xp or vista
Attachment #401178 - Flags: review?(jfkthame) → review+
Comment on attachment 401178 [details] [diff] [review]
patch, v.0.3d, update based on review comments

>+    PRUint32 prevEndCharCode = 0;
>     for (PRUint32 i = 0; i < numGroups; i++, groups += SizeOfGroup) {
>         const PRUint32 startCharCode = ReadLongAt(groups, GroupOffsetStartCode);
>         const PRUint32 endCharCode = ReadLongAt(groups, GroupOffsetEndCode);
>+        NS_ENSURE_TRUE((prevEndCharCode < startCharCode || startCharCode == 0) &&
>+                       startCharCode <= endCharCode &&
>+                       endCharCode <= CMAP_MAX_CODEPOINT, 
>+                       NS_ERROR_GFX_CMAP_MALFORMED);

One small point: this isn't quite the right test, as it would allow a non-initial segment to reset startCharCode to zero, which implies an out-of-order segment, without detecting this as an error. Making the first part of the test

   (prevEndCharCode < startCharCode || i == 0)

would fix this, I think.

>+    PRUint16 prevEndCount = 0;
>     for (PRUint16 i = 0; i < segCount; i++) {
>         const PRUint16 endCount = ReadShortAt16(endCounts, i);
>         const PRUint16 startCount = ReadShortAt16(startCounts, i);
>         const PRUint16 idRangeOffset = ReadShortAt16(idRangeOffsets, i);
>+        
>+        // sanity-check range
>+        NS_ENSURE_TRUE((startCount > prevEndCount || startCount == 0) && 
>+                       startCount <= endCount,
>+                       NS_ERROR_GFX_CMAP_MALFORMED);

Same applies here.

Marking this as r+, but still recommend tightening up those validity checks before landing.
(Assignee)

Comment 22

8 years ago
Pushed to trunk (with suggested modification)
http://hg.mozilla.org/mozilla-central/rev/1ee2863e89e2

Sounds like corrupt format-12 cmaps don't affect FreeType, so I'm assuming this problem doesn't affect Linux.
(Assignee)

Comment 23

8 years ago
Comment on attachment 401178 [details] [diff] [review]
patch, v.0.3d, update based on review comments

Need to push this to 1.9.2/1.9.1
Attachment #401178 - Flags: approval1.9.2?
(Assignee)

Updated

8 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Attachment #401178 - Flags: approval1.9.1.4+
Comment on attachment 401178 [details] [diff] [review]
patch, v.0.3d, update based on review comments

Approved for 1.9.1.4, a=dveditz for release-drivers
(Assignee)

Comment 25

8 years ago
Pushed to 1.9.1 branch
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b3a6533db3da
status1.9.1: wanted → .4-fixed
Flags: blocking1.9.2? → blocking1.9.2+
Priority: -- → P1
Comment on attachment 401178 [details] [diff] [review]
patch, v.0.3d, update based on review comments

Marking approval, though it doesn't need it now that it's a blocker.
Attachment #401178 - Flags: approval1.9.2? → approval1.9.2+
(Assignee)

Comment 27

8 years ago
Pushed to 1.9.2 branch
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/ad95be4fe45d
status1.9.2: --- → beta1-fixed
Verified fixed for 1.9.1 using standalone testcase from comment 11 and my own debug build (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.4pre) Gecko/20090929 Shiretoko/3.5.4pre (.NET CLR 3.5.30729)).
Keywords: verified1.9.1
Flags: wanted1.9.0.x-
(Assignee)

Updated

8 years ago
Blocks: 526869
No longer blocks: 526869
Depends on: 526869
Group: core-security
Re-hiding bug, apparently MS is fixing the underlying problem on their end and considers it critical.
Group: core-security
Whiteboard: [sg:critical] → [sg:vector-critical] wait for MS
Whiteboard: [sg:vector-critical] wait for MS → [sg:vector-critical][MS ref: 9458mh --keep private until their patch, ETA ~May 2010]
(Reporter)

Comment 30

7 years ago
(In reply to comment #29)
> Re-hiding bug, apparently MS is fixing the underlying problem on their end and
> considers it critical.

its now http://www.microsoft.com/technet/security/bulletin/MS10-063.mspx
Group: core-security
You need to log in before you can comment on or make changes to this bug.