608336 - (CVE-2010-3769) CRITICAL BUG when calling document.write()

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Dirk Heinrich]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12 (.NET CLR 3.5.30729)

crashing after executing the following code from my test website with firefox 3.6.12:

<script language="JavaScript">
var m = "6";
for (var i = 0; i < 26; i++) {
	m = m + m;
}
document.write(m);
</script>

Reproducible: Always

Steps to Reproduce:
1. create a file called crash.htm with the following content
<script language="JavaScript">
var m = "6";
for (var i = 0; i < 26; i++) {
	m = m + m;
}
document.write(m);
</script>
2. doppelclick the file

Actual Results:  
0034a874 7684fdff 04000000 00000005 03d6e948 USP10!DoubleWideCharMappedString::DoubleWideCharMappedString+0x33
0034a954 76850f69 04000000 00000005 00000000 USP10!ScriptTokenize+0x4f
0034a990 76846248 0034af78 04000000 0034aa54 USP10!ScriptItemizeCommon+0x59
0034a9b4 555dc1a2 0034af78 04000000 00000005 USP10!ScriptItemize+0x38
WARNING: Stack unwind information not available. Following frames may be wrong.
0034aa6c 58348741 583e7ff8 00000000 0034ab28 xul!gfxTextRun::CopyGlyphDataFrom+0x5902
00000000 00000000 00000000 00000000 00000000 MOZCRT19!expand+0xa41

Comment 1

[Benjamin Smedberg]

Do you have a crash report ID, did the Firefox crash reporter come up?

Comment 2

[Dirk Heinrich]

the crash reporter came up, i have sent a crash report to mozilla (dirk4000)

Comment 3

[Benjamin Smedberg]

Dirk, can you past the crash report link from about:crashes?

Comment 4

[Dirk Heinrich]

http://crash-stats.mozilla.com/report/index/bp-aaec46cb-b0d8-4c68-bb2c-8823c2101029
http://crash-stats.mozilla.com/report/index/bp-50c618df-e95f-4a50-8e7f-4a0f92101029

Comment 5

[Dirk Heinrich]

http://crash-stats.mozilla.com/report/index/bp-15b2343c-edf8-453e-8377-e9d8c2101029

Comment 6

[Benjamin Smedberg]

Crash #1

0 	usp10.dll 	DoubleWideCharMappedString::DoubleWideCharMappedString 	
1 	usp10.dll 	ScriptTokenize 	
2 	usp10.dll 	ScriptItemizeCommon 	
3 	usp10.dll 	ScriptItemize 	
4 	xul.dll 	Uniscribe::Itemize 	gfx/thebes/src/gfxWindowsFonts.cpp:2413
5 	xul.dll 	gfxWindowsFontGroup::InitTextRunUniscribe 	gfx/thebes/src/gfxWindowsFonts.cpp:2685

Not clear whether this is exploitable. Graphics gurus, can you tell if we have an unchecked allocation or are just passing null into uniscribe?

Crash #2

0 	kernel32.dll 	RaiseException 	
1 	mozcrt19.dll 	_CxxThrowException 	throw.cpp:159
2 	mozcrt19.dll 	operator new 	obj-firefox/memory/jemalloc/crtsrc/new.cpp:57
3 	xul.dll 	gfxWindowsFontGroup::MakeTextRun 	gfx/thebes/src/gfxWindowsFonts.cpp:1515
4 	xul.dll 	TextRunWordCache::MakeTextRun 	gfx/thebes/src/gfxTextRunWordCache.cpp:811

Definitely not exploitable.

Comment 7

[Bas Schouten (:bas.schouten)]

DirectWrite seems to survive this code. I'm having a look at the GDI case now.

Comment 8

[Bas Schouten (:bas.schouten)]

In a minefield nightly on Windows 7 this crashes on a NULL pointer dereference for me in a different part of the font code after MakeTextRun, I think that MakeTextRun is handling an error here where in the Windows XP situation it's not. Minefield is using over 1 GB of memory when this crash occurs so I'm guessing it's OOM related.

Comment 10

[Bas Schouten (:bas.schouten)]

I've reproduced the first crash mentioned by bsmedberg inside a debugger. With the following exception address:

firefox.exe: 0xC0000005: Access violation reading location 0x00481000.

After some digging around in annoying optimized code I've isolated what the Uniscribe structure appears to looks like:

-		(Uniscribe*)0x0047A67C	0x0047a67c {mContext={...} mDC=0x810110bc mString=0x0047aba8 "" ...}	Uniscribe *
-		mContext	{mRawPtr=0x04e01660 }	nsRefPtr<gfxContext>
+		mRawPtr	0x04e01660 {mRefCnt={...} mCairo=0x04a48000 mSurface={...} ...}	gfxContext *
+		mDC	0x810110bc {unused=??? }	HDC__ *
-		mString	0x0047aba8 ""	const wchar_t *
			0x0000	const wchar_t
		mLength	0x04000000	const unsigned int
		mIsRTL	0x00000000	const int
+		mControl	{uDefaultLanguage=0x00000000 fContextDigits=0x00000000 fInvertPreBoundDir=0x00000000 ...}	tag_SCRIPT_CONTROL
+		mState	{uBidiLevel=0x0000 fOverrideDirection=0x0001 fInhibitSymSwap=0x0000 ...}	tag_SCRIPT_STATE
-		mItems	{...}	nsTArray<tag_SCRIPT_ITEM>
-		nsTArray_base	{sEmptyHdr={...} mHdr=0x04a28680 }	nsTArray_base
+		sEmptyHdr	{mLength=0x00000000 mCapacity=0x00000000 mIsAutoArray=0x00000000 }	nsTArray_base::Header
-		mHdr	0x04a28680 {mLength=0x00000006 mCapacity=0x00000006 mIsAutoArray=0x00000000 }	nsTArray_base::Header *
		mLength	0x00000006	unsigned int
		mCapacity	0x00000006	unsigned int
		mIsAutoArray	0x00000000	unsigned int
		mNumItems	0x77a32210	int

mLength seems to make sense being 2^26 as one would expect. mString is a little more confusing as it does not seem to point to any repeatable pattern. As a matter of fact when reading from the address mString points to, 0x0047aba8, indeed the first inaccessible address after a contiguous set of readable memory is 0x00481000. I have not figured out yet why we get an mString that is not an  allocated block of mLength * sizeof(PRUnichar).

Comment 11

[Bas Schouten (:bas.schouten)]

It seems for some reason my stack is messed up above the InitTextRun stack frame making it very difficult to determine what lead to this strange situation.

Comment 12

[Bas Schouten (:bas.schouten)]

I believe the following code is where things might be going wrong:

http://hg.mozilla.org/releases/mozilla-1.9.2/annotate/8fe44c79dfd1/gfx/thebes/src/gfxWindowsFonts.cpp#l1534

I've been looking at the code for AppendASCIItoUTF16 and it does not appear to be infallible. This means that if it hits an OOM we will silently fail to append, our current, empty, stack based string will be passed in -however- we will pass in the length of our original C string as length! I believe this is what's happening. It would seem to be confirmed by the fact mString points to 0x0000.

Comment 13

[Bas Schouten (:bas.schouten)]

Attachment: Pass proper length to InitTextRun*

This patch fixes the crash the read access violation. (the OOM situation is a different matter, so is the other potentially security sensitive issue I mentioned)

This seems like the right solution for the mozilla-1.9.2 branch, it's also what we do on current trunk in gfxFontGroup::MakeTextRun

Comment 14

[Dirk Heinrich]

crash also happens on MacOS 10/Leopard with FireFox 4.0 b6

http://crash-stats.mozilla.com/report/index/bp-d68924fc-9563-4a31-ab73-7024c2101030

Comment 15

[Bas Schouten (:bas.schouten)]

I believe Comment 9 may very well have been a false alarm. After further
examining the assembly code I believe the Visual Studio debugger is wrongly
determining the mTextRun pointer so I do not believe it was actually pointing
to the aforementioned memory containing 0x36 in a repeatable pattern.

Sadly the optimizations in the code make it extremely hard to determine what's
actually going on here so it is still hard to say if it is exploitable. The
full dump with heap is obviously too big to attach here.

Comment 16

[Bas Schouten (:bas.schouten)]

I've determined where the actual BreakSink we're working with is in memory, the text run member's vtable on there looks just fine:

-		(BuildTextRunsScanner::BreakSink*)0x127615c0	0x127615c0 {mTextRun=0x41000000 mContext=0x04d15ff0 mOffsetIntoTextRun=0x02090000 ...}	BuildTextRunsScanner::BreakSink *
+		nsILineBreakSink	{...}	nsILineBreakSink
-		mTextRun	0x41000000 {mCharacterGlyphs=0x41000060 mDetailedGlyphs={...} mGlyphRuns={...} ...}	gfxTextRun *
-		__vfptr	0x10903fbc const gfxTextRun::`vftable'	*
		[0x0]	0x10006450 gfxTextRun::`vector deleting destructor'(unsigned int)	*
		[0x1]	0x0ffeb0b0 gfxTextRun::SetPotentialLineBreaks(unsigned int, unsigned int, unsigned char *, gfxContext *)	*
		[0x2]	0x100bbe77 gfxTextRun::SetLineBreaks(unsigned int, unsigned int, int, int, double *, gfxContext *)	*
		[0x3]	0x105bd8fe gfxTextRun::Clone(const gfxTextRunFactory::Parameters *, const void *, unsigned int, gfxFontGroup *, unsigned int)	*
		[0x4]	0x1008e550 gfxTextRun::CopyGlyphDataFrom(gfxTextRun *, unsigned int, unsigned int, unsigned int, int)	*
+		mCharacterGlyphs	0x41000060 {mValue=0x81e00019 }	gfxTextRun::CompressedGlyph *
+		mDetailedGlyphs	{mRawPtr=0x00000000 }	nsAutoArrayPtr<nsAutoArrayPtr<gfxTextRun::DetailedGlyph> >
+		mGlyphRuns	{mAutoBuf=0x41000014 "" }	nsAutoTArray<gfxTextRun::GlyphRun,1>
+		mText	{mSingle=0x51000064 "666666666666666666666666 m	gfxTextRun::<unnamed-tag>
		mUserData	0x126cc000	void *
+		mFontGroup	0x04c28820 {mGenericFamily={...} mFontEntries={...} mFontNeedsBold={...} ...}	gfxFontGroup *
+		mSkipChars	{mList={...} mShortcuts={...} mListLength=0x00000000 ...}	gfxSkipChars
+		mExpirationState	{mGeneration=0x00000000 mIndexInGeneration=0x00000000 }	nsExpirationState
		mAppUnitsPerDevUnit	0x0000003c	unsigned int
		mFlags	0x01440120	unsigned int
		mCharacterCount	0x04000001	unsigned int
		mHashCode	0x00000000	unsigned int
		mUserFontSetGeneration	0x0000000000000000	unsigned __int64
+		mContext	0x04d15ff0 {mRefCnt={...} mCairo=0x04c56000 mSurface={...} ...}	gfxContext *
		mOffsetIntoTextRun	0x02090000	unsigned int
		mChangedBreaks	0x00	unsigned char
		mExistingTextRun	0x00	unsigned char

The actual crash location turns out to be obscured due to inlining of the function, but is:

2919 gfxTextRun::SetPotentialLineBreaks(PRUint32 aStart, PRUint32 aLength,
2920                                    PRPackedBool *aBreakBefore,
2921                                    gfxContext *aRefContext)

The line it crashes in is:

PRBool canBreak = aBreakBefore[i];

where i == 0xff8

In other words aBreakBefore is too small. This brought me to nsLineBreaker::FlushCurrentWord where it seems we make the breakState array that ends up in aBreakBefore the size of 'length' and we properly check for success. length here appears to be 0x02000000, however note that mOffsetIntoTextRun is actually 0x02090000.

I do not know the root cause of this crash yet and it seems to be pretty tricky to reproduce, I'll be away the coming week though so this is all I've got for now. Someone more familiar with the code might find enough information in here to determine the cause hopefully.

Comment 17

[Bas Schouten (:bas.schouten)]

I'm going away for the week. This testcase crashes on trunk too, I doubt that crash is exploitable (it seems like just a form of OOM), but it should probably be looked at.

Comment 18

[Reed Loden [:reed]]

Is 1.9.1 affected?

Comment 19

[Daniel Veditz [:dveditz]]

Comment on attachment 487129 [details] [diff] [review]
Pass proper length to InitTextRun*

Approved for 1.9.2.13, a=dveditz for release-drivers

Is this the trunk patch as well?

This looks like the correct patch for 1.9.1, please land there as well:
Approved for 1.9.1.16, a=dveditz for release-drivers

Comment 20

[Reed Loden [:reed]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/73cc9a1257c0
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/38c1615a5480

Comment 21

[Jesse Ruderman]

Comment 13 says that the thing we fixed here on 1.9.2 is already fixed on trunk.

Based on comment 17, it sounds like trunk tends to OOM.  We already have plenty of bugs on "OOM after gigantic document.write() crashes in various ways, most of which are not exploitable, but we're not really safe until we fix bug 611123".  But if we were going to fix something else on trunk, it would need a new bug anyway.