99174 - Buffer overflow in X widgets

Status: VERIFIED FIXED

(Core :: Security, defect)

Description

[Mitchell Stoltz (not reading bugmail)]

This problem seems to be in X and not in Mozilla though I have not debugged much
but may be exploitable from Mozilla.
There is buffer overflow in X which also affects Mozilla/Communicator and other
programs.
I suppose this is exploitable, i.e. probably it may allow execution of code or
at least of XLib
requests.
Tested on Linux kernel 2.4 XFree86 Version 4.0.3 / X Window System
(protocol Version 11, revision 0, vendor release 6400)
Have not seen security bulletin by the X folks about this though they may have
fixed it silently.
Windoze seems not affected.
The problem is when Mozilla/Communicator create native X widgets (or whatever
the correct term is).
For Mozilla and Communicotor the overflow occurs in:
----------------------------------------
<input type="text" value="$A" name="x">
----------------------------------------
where $A is about 300K.
Output:
Gdk-ERROR **: BadLength (poly request too large or internal Xlib length erro
  serial 13629 error_code 16 request_code 74 minor_code 0
Gdk-ERROR **: BadLength (poly request too large or internal Xlib length erro
  serial 13630 error_code 16 request_code 116 minor_code 0
The interesting part is that "request_code 116" may be controlled by the content
of the overflow.
Mozilla seems to exit thru exit() though have not debugged too much.
Communicator freezes.

For Communicator the overflow is:
-----------
<script>
prompt("msg","$A","$A",1); // < this is another overflow
</script>
-----------
where $A is about 300K
(guess Mozilla does not support correctly prompt() - [does not allow default
text] that's why this does not work on Mozilla).
Communicator freezes and when it is killed with kill(1) it exits with "Illegal
instruction" and the stack is screwed up.

I suppose almost every application that creates a "textbox" is affected.
A tcl script which creates "textbox" is also affected by this.
Konqueror 2 is affected by both bugs and Konqueror and tcl kill the whole X with
SEGV.

Usage of the programs:
For mozilla and comm:
./xov1.pl > xov1.html
For comm:
./xov.pl > xov2.html

Open the resulting html with moz or comm or even konqueror.
For tcl:
./t1.pl > ./t1.tcl
chmod +x ./t1.tcl
./t1.tcl
[the tcl may need to be run several times or increase the length of the overflow]

Georgi Guninski

Comment 1

[Mitchell Stoltz (not reading bugmail)]

Attachment: Demonstration 1 - WARNING: THIS LINK WILL CAUSE A CRASH ON LINUX/UNIX

Comment 2

[Mitchell Stoltz (not reading bugmail)]

Attachment: Demonstration 2 - THIS WILL ALSO CRASH

Comment 3

[Mitchell Stoltz (not reading bugmail)]

I've confirmed that the first one, at least, causes a crash on the current
trunk. The console shows a Gdk-Error as Georgi describes. We need to (a) confirm
that the bug is in X, not in Mozilla, and (b) either inform X about it or add a
bounds check in Mozilla, or preferably both.

Comment 4

[Mitchell Stoltz (not reading bugmail)]

Reassigning to Blizzard.

Chris, this could be serious, could you take a look?

Comment 5

[Christopher Blizzard (:blizzard)]

Looking.

Comment 6

[Christopher Blizzard (:blizzard)]

This crash is really down deep in the bowels of Xlib.  The array of text looks
good going into the function so I'm concerned it's an Xlib bug which is bad. 
I'm going to build X rpms with debugging so I can debug this more.

Comment 7

[Mitchell Stoltz (not reading bugmail)]

Georgi also thinks the bug is in XLib, not Mozilla. He attempted to contact the
X people but got no response. If necessary, if X doesn't address this problem
soon, can we put a bounds check in our code to prevent this?

Comment 8

[Mitchell Stoltz (not reading bugmail)]

Chris, any more progress on this? Georgi thinks this may be serious.

Comment 9

[Christopher Blizzard (:blizzard)]

No, I haven't been able to make progress on this.  I will (hopefully) be able to
get access to what I need on Wednesday the 31st.  This bug is at the top of my list.

Comment 10

[Christopher Blizzard (:blizzard)]

Here's a status update of where I am with this.  I spent a couple of hours on
this yesterday looking into how dangerous this is.  It appears after looking
that the client side is quite safe.  It looks like we are just hitting a buffer
limitation on the server side and the server side is returning the error to the
client, just as it should.

I'm checking the server side now to make sure that we aren't running into any
possible buffer overruns and to try and figure out why we get a random
request_code back in the struct.

Comment 11

[Christopher Blizzard (:blizzard)]

Attachment: patch

Comment 12

[Christopher Blizzard (:blizzard)]

This is a patch that clamps the draw requests to 32k or less.  The comment says:

+    // We clamp the sizes down to 32768 which is the maximum width of
+    // a window.  Even if a font was 1 pixel high and started at the
+    // left, the maximum size of a draw request could only be 32k.

Comment 13

[Christopher Blizzard (:blizzard)]

Another note from Keith:

>#define TEXT_LEN 260077

Some Xlib functions will split data into separate requests, but PolyText 
can't easily do so as the arguments to the function don't include any 
notion of the per-glyph escapement.  Further, PolyText splits text into 
254-char strings, so the maximum number of glyphs indices encodable at 
8-bits per glyph is:

	(max_request_len - SIZEOF(xPolyTextReq)) / (254 + SIZEOF(xTExtElt)) * 254

	= floor ((256 * 1024 - 16) / (254 + 2)) * 254 = 1023 * 254 = 259842

Applications are nominally expected to verify that their requests are
within this limit.

Comment 14

[Christopher Blizzard (:blizzard)]

So can I get some reviews?  My fix is probably the best we are going to be able
to do.

Comment 15

[Brendan Eich [:brendan]]

Comment on attachment 56284 [details] [diff] [review]
patch

sr=[e-mail]

Comment 16

[Alec Flett]

Comment on attachment 56284 [details] [diff] [review]
patch

yikes. r=alecf

Comment 17

[Christopher Blizzard (:blizzard)]

This has been checked in.  It's before the 0.9.6 release.

Comment 18

[Mitchell Stoltz (not reading bugmail)]

Is this fixed? Is there a reason why it wasn't marked Fixed? Does it need to be
checked into a branch?

Comment 19

[Christopher Blizzard (:blizzard)]

Yes, this has been fixed.

By the way, this was determined to be an exploitable X server bug and it's been
fixed in the most recent X release.

Comment 20

[Christopher Blizzard (:blizzard)]

Fixed.

Comment 21

[georgi - hopefully not receiving bugspam]

Blizzard,

Could you please give me a link where it is claimed this is exploitable?
I know it is fixed in recent X but didn't know it is exploitable (more than DoS).

Very similar issue which I sent to Mitchell this week still works - crashes at
least X 4.1.0-3 (the one that ships with RedHat 7.2).
In some circumstances it does not crash X but corrupts at least some visible
memory. In other cases it locks the computer completely and reset is needed.

I suggest fixing the new issue also.

Comment 22

[Christopher Blizzard (:blizzard)]

I don't know of a link.  You would have to talk to Keith Packard about it.

As for the second bug, I haven't seen it.

Comment 23

[bsharma]

Verified on 2002-02-21-Trunk.

Both attached test cases work fine.

Regarding other crash please provide more details to reproduce. 
Marking this verified.

Comment 24

[bsharma]

Added paw and jimmylee to the cc list.

Comment 25

[Scott MacGregor]

I just checked this into the AOLTW branch.

Comment 26

[bsharma]

Added AOLTWOK in the Status Whiteboard.

Verified on WinNT and MacOSX.