Bug 15222 - Multiple X server vulnerabilities reported by iDefense
Multiple X server vulnerabilities reported by iDefense
Status: RESOLVED FIXED
Product: xorg
Classification: Unclassified
Component: Security
7.2 (2007.02)
All All
: medium normal
Assigned To: X.Org Security
X.Org Security
: security
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-03-27 03:35 UTC by Matthieu Herrb
Modified: 2009-05-21 02:26 UTC (History)
2 users (show)

See Also:


Attachments
Record vulnerabilites draft advisory (3.76 KB, text/plain)
2008-03-27 03:35 UTC, Matthieu Herrb
no flags Details
Render vulnerabilities draft advisory (3.78 KB, text/plain)
2008-03-27 03:36 UTC, Matthieu Herrb
no flags Details
MIT-SHM vulnerabilities draft advisory (3.49 KB, text/plain)
2008-03-27 03:38 UTC, Matthieu Herrb
no flags Details
Prof of concept and additional information (18.26 KB, application/octet-stream)
2008-03-27 03:44 UTC, Matthieu Herrb
no flags Details
1st try at patching the issues (against server-1.4-branch) (5.98 KB, patch)
2008-04-01 14:42 UTC, Matthieu Herrb
no flags Details | Splinter Review
new version of the patch (6.75 KB, patch)
2008-05-04 02:46 UTC, Matthieu Herrb
no flags Details | Splinter Review
patch against current X.org master. (6.67 KB, patch)
2008-05-11 19:01 UTC, Dave Airlie
no flags Details | Splinter Review
patch against xserver master for dbe. (412 bytes, patch)
2008-05-11 20:18 UTC, Dave Airlie
no flags Details | Splinter Review
Another possible patch (1.76 KB, patch)
2008-05-11 23:40 UTC, Paul Anderson
no flags Details | Splinter Review
even better dbe fixing - against master (3.59 KB, patch)
2008-05-13 17:23 UTC, Dave Airlie
no flags Details | Splinter Review
Draft X.Org advisory (4.00 KB, text/plain)
2008-05-28 12:59 UTC, Matthieu Herrb
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Matthieu Herrb 2008-03-27 03:35:48 UTC
Created attachment 15497 [details]
Record vulnerabilites draft advisory

iDefense has reported several vulnerabilities in the X server, in Record, Render and SHM extensions. 

Attached are the 3 draft advisories that were sent to us.
Comment 1 Matthieu Herrb 2008-03-27 03:36:58 UTC
Created attachment 15498 [details]
Render vulnerabilities draft advisory
Comment 2 Matthieu Herrb 2008-03-27 03:38:00 UTC
Created attachment 15499 [details]
MIT-SHM vulnerabilities draft advisory
Comment 3 Matthieu Herrb 2008-03-27 03:44:17 UTC
Created attachment 15500 [details]
Prof of concept and additional information
Comment 4 Matthieu Herrb 2008-03-27 03:58:31 UTC
I've started looking at the issues. 

When is xserver 1.5 release planned? we should try to coordinate the disclosure of these issues with its release if possible.
Comment 5 Alan Coopersmith 2008-03-27 08:29:05 UTC
The current X11R7.4/server-1.5 plan put forth by ajax is:
 April 4: .903, freeze, approved fixes only
 April 18: .991, hard freeze, showstoppers only
 April 25: xserver 1.5 and 7.4 katamari
Comment 6 Matthieu Herrb 2008-03-27 14:14:54 UTC
(In reply to comment #5)
> The current X11R7.4/server-1.5 plan put forth by ajax is:
>  April 4: .903, freeze, approved fixes only
>  April 18: .991, hard freeze, showstoppers only
>  April 25: xserver 1.5 and 7.4 katamari

Hmm fridays are generally considered bad as security release dates. If we get patches ready next monday, would April 22 1400 UTC be an acceptable disclosure date for all of you, before I submit that to vendor-sec and back to iDefense?
Comment 7 Matthieu Herrb 2008-04-01 14:42:58 UTC
Created attachment 15614 [details] [review]
1st try at patching the issues (against server-1.4-branch)

This is my first try at patching some of the issues in server-1.4-branch. 

I'm a bit helpless with two of the issues: the DBE extension DoS and the MIT-SHM memoryy reading one. I would appreciate if someone had clues on how to validate the input in those cases.
Comment 8 ajax at nwnk dot net 2008-04-03 11:36:54 UTC
I don't see a DBE DoS described in any of the attached vulnerability reports.  Is this available out of band somewhere?

Embargo date is currently April 22.
Comment 9 Josh Bressers 2008-04-03 12:27:25 UTC
Here are some CVE ids for these issues:

CVE-2008-1377 iDefense X.org Record extension Input Validation flaw
CVE-2008-1378 iDefense X.org Render extension Integer overflows
CVE-2008-1379 iDefense X.org MIT-SHM extension Input Validation flaw
Comment 10 Paul Anderson 2008-04-03 16:46:49 UTC
(In reply to comment #7)
> Created an attachment (id=15614) [details]
> 1st try at patching the issues (against server-1.4-branch)
> This is my first try at patching some of the issues in server-1.4-branch. 

I have a question about the fix to SecurityGenerateAuthorization.  

Part of the patch for this fix is:

+    if (values_offset > stuff->length - (sz_xSecurityQueryVersionReq >> 2))
+       return BadLength;

Should sz_xSecurityQueryVersionReq be sz_xSecurityGenerateAuthorizationReq?

-paul
Comment 11 Alan Coopersmith 2008-04-03 21:28:44 UTC
(In reply to comment #8)
> I don't see a DBE DoS described in any of the attached vulnerability reports.

There's a DBE-DoS.c in the zip file from iDefense - I assume that's what he's
referring to, though I haven't had a chance to examine it myself.
 
> Embargo date is currently April 22.

That seems awfully soon if we don't have fixes yet, and are going to lose most
of a week to the X.Org Developer's Conference between then and now (especially
for those traveling far).
Comment 12 Matthieu Herrb 2008-04-08 12:33:32 UTC
(In reply to comment #10)
> (In reply to comment #7)
> > Created an attachment (id=15614) [details] [details]
> > 1st try at patching the issues (against server-1.4-branch)
> > This is my first try at patching some of the issues in server-1.4-branch. 
> 
> I have a question about the fix to SecurityGenerateAuthorization.  
> 
> Part of the patch for this fix is:
> 
> +    if (values_offset > stuff->length - (sz_xSecurityQueryVersionReq >> 2))
> +       return BadLength;
> 
> Should sz_xSecurityQueryVersionReq be sz_xSecurityGenerateAuthorizationReq?
> 
> -paul
> 

Sure
Comment 13 Dave Airlie 2008-04-13 21:51:19 UTC
can somebody attach the zip from idefence to this bug?
Comment 14 Matthieu Herrb 2008-04-13 22:57:41 UTC
(In reply to comment #13)
> can somebody attach the zip from idefence to this bug?
> 

It is there (4th attachment). 
Comment 15 Josh Bressers 2008-04-15 10:37:35 UTC
So what's the plan for an embargo here?  Given how close we are to the 22nd now, what if we move it to May 6.  That should give everyone plenty of time.
Comment 16 Matthieu Herrb 2008-04-15 12:29:06 UTC
(In reply to comment #15)
> So what's the plan for an embargo here?  Given how close we are to the 22nd
> now, what if we move it to May 6.  That should give everyone plenty of time.
> 

I'm fine with that. The original plan was to be able to ship xserver 1.5 with the patches, but since the X.Org 7.4 release schedule seems to be slipping, lets take more time. 

There are still 2 bugs for which we don't have patches yet.
Comment 17 Stefan Dirsch 2008-04-15 18:29:12 UTC
(In reply to comment #16)
> I'm fine with that. The original plan was to be able to ship xserver 1.5
> with the patches, but since the X.Org 7.4 release schedule seems to be
> slipping, lets take more time. 
> 
> There are still 2 bugs for which we don't have patches yet.
You mean for X.Org 7.4 or for this security issue?

Comment 18 Matthieu Herrb 2008-04-16 13:03:01 UTC
(In reply to comment #17)
> (In reply to comment #16)
> > I'm fine with that. The original plan was to be able to ship xserver 1.5
> > with the patches, but since the X.Org 7.4 release schedule seems to be
> > slipping, lets take more time. 
> > 
> > There are still 2 bugs for which we don't have patches yet.
> You mean for X.Org 7.4 or for this security issue?
> 

for this security issue. The MIT-SHM one and the DBE DoS. 
Comment 19 Stefan Dirsch 2008-04-19 03:09:24 UTC
Thanks for clarification, Matthieu.
Comment 20 Matthieu Herrb 2008-05-04 02:46:20 UTC
Created attachment 16341 [details] [review]
new version of the patch

Here's a new version of the patch. 
It takes into account a problem with some height==0 cases that I found to happen on my desktop system. (Are other testing/reviewing the patches ?) and adds a patch for the SHM memory disclosure issue.

The DBE issue is still open, but that may be ok, since iDefense doesn't consider it as having a security impact. 

Are you all ready for disclosure on next tuesday (May 6) or should we shift more (I propose May 20, since May 13 is very impractical for me, as I'm drowning in other deadlines for May 5) ?
Comment 21 Julien Cristau 2008-05-05 13:00:20 UTC
On Sun, May  4, 2008 at 02:46:21 -0700, bugzilla-daemon@freedesktop.org wrote:

> --- Comment #20 from Matthieu Herrb <matthieu.herrb@laas.fr>  2008-05-04 02:46:20 PST ---
> Are you all ready for disclosure on next tuesday (May 6) or should we shift
> more (I propose May 20, since May 13 is very impractical for me, as I'm
> drowning in other deadlines for May 5) ?
> 
Delaying would be better for me, as i'm out of town until the 7th.

Cheers,
Julien
Comment 22 Stefan Dirsch 2008-05-06 00:52:05 UTC
Yes, a shift to May 20 would be very much appreciated. Speaking as X.Org maintainer for SUSE.
Comment 23 Matthieu Herrb 2008-05-06 01:05:00 UTC
(In reply to comment #22)
> Yes, a shift to May 20 would be very much appreciated. Speaking as X.Org
> maintainer for SUSE.
> 

I've already informed iDefense that we're not releasing this this today. I'd like to see some feedback (regression testing, and checks that the vulnerabilites are indeed fixed) on the patches I prepared before confirming May 20 as the disclosure date.
Comment 24 Julien Cristau 2008-05-07 19:36:31 UTC
> --- Comment #20 from Matthieu Herrb <matthieu.herrb@laas.fr>  2008-05-04 02:46:20 PST ---
> Here's a new version of the patch. 
> It takes into account a problem with some height==0 cases that I found to
> happen on my desktop system. (Are other testing/reviewing the patches ?) and
> adds a patch for the SHM memory disclosure issue.
> 
I think render/glyph.c should include <stdint.h> (for UINT32_MAX).  In
1.4 it gets it through <pixman.h>, but that's pretty fragile and breaks
on older servers who don't use pixman.

Cheers,
Julien
Comment 25 Dave Airlie 2008-05-11 19:01:14 UTC
Created attachment 16473 [details] [review]
patch against current X.org master.

I've ported the patch to the current X.org master and for 1.5

The glyph code has changed quite a lot, so the fix is now in render/render.c

I've run the RENDER-heap-overflow against the updated fix and it return BadLength.

I'll try and do some more testing today.
Comment 26 Dave Airlie 2008-05-11 20:18:21 UTC
Created attachment 16477 [details] [review]
patch against xserver master for dbe.

This is a fix for the DBE issue against X server master. resetting the private to NULL stops the issue from happening.

this isn't backwards portable as all the dix privates was re-written but I doubt its too hard to fix in previous releases.
Comment 27 Paul Anderson 2008-05-11 23:40:53 UTC
Created attachment 16478 [details] [review]
Another possible patch

The previous patch will fix the crash, but I think it will result in a potential resource leak because we've already called AddResource().  

This patch is another possibility.  I haven't done as much testing as I'd like, but I wanted to post it for feedback.  

This patch delays the call to AddResource() until we've past the calls that could generate failures.  

Another question for the group - do you think the calls at the end of the routine to increment nBufferIDs and set the swap action should always happen (as they are now) or only happen in the "Success" case?
Comment 28 Dave Airlie 2008-05-13 17:23:44 UTC
Created attachment 16513 [details] [review]
even better dbe fixing - against master

Paul,

I've enhanced you're patch and I think this is the "correct" solution now :)

its against master, so the dixPrivate lines have to change for old servers.
Comment 29 Dave Airlie 2008-05-13 18:57:40 UTC
so can we agree a new Embargo date?

some holiday Monday in the U.S. coming up I think...
Comment 30 Alan Coopersmith 2008-05-15 07:39:57 UTC
(In reply to comment #29)
> so can we agree a new Embargo date?
> some holiday Monday in the U.S. coming up I think...

Monday, May 26 is Memorial Day holiday in the U.S.    Looks like it's also
a Bank Holiday in the U.K.

Tuesday June 3 would give us just over 2 weeks from today to prepare.
Comment 31 Dave Airlie 2008-05-15 16:23:03 UTC
I can do June 3rd sounds good to me.
Comment 32 Dave Airlie 2008-05-25 16:47:16 UTC
So the DBE problem doesn't seem at first look to be an actual exploitable hole, just a DoS if you have access to the users screen, in which case xkill is probably worse.

However I don't see anything about the SECURITY memory corruption, do we need another CVE?
Comment 33 Josh Bressers 2008-05-27 14:04:42 UTC
I'm under the impression the Record and Security extension flaws are of the same type (unchecked input validation), and since they're both covered in a single iDefense advisory, we can use CVE-2008-1377 for them.

If I'm mistaken (which is quite possible), please let me know and I can assign a new CVE id.
Comment 34 Josh Bressers 2008-05-27 17:57:09 UTC
After speaking with airlied, I am splitting the Render advisory into 3 seperate CVE ids.  The three functions affect various versions of X in different ways.

CVE-2008-2360 Render Integer overflows AllocateGlyph()
CVE-2008-2361 Render Integer overflows ProcRenderCreateCursor()
CVE-2008-2362 Render Integer overflows SProcRenderCreateLinearGradient()

DO NOT USE CVE-2008-1378, that ID is going to be rejected.
Comment 35 Matthieu Herrb 2008-05-28 12:38:04 UTC
Should we include the DBE stuff in our advisory and patch, or just commit it without mentionning security explicitely. (iDefense seem to share the analysis that it has no security impact). 
Comment 36 Matthieu Herrb 2008-05-28 12:59:22 UTC
Created attachment 16793 [details]
Draft X.Org advisory 

Attached is a draft for the X.Org advisory. Comments, corrections are welcome.
Comment 37 Dave Airlie 2008-05-28 13:56:52 UTC
I think we should just commit the DBE stuff at the same time but not mention it. I don't intend on patching our security releases with the DBE patch and I'd prefer not to confuse people.
Comment 38 Julien Cristau 2008-05-28 14:48:24 UTC
On Wed, May 28, 2008 at 12:59:22 -0700, bugzilla-daemon@freedesktop.org wrote:

> --- Comment #36 from Matthieu Herrb <matthieu.herrb@laas.fr>  2008-05-28 12:59:22 PST ---
> Attached is a draft for the X.Org advisory. Comments, corrections are welcome.
> 
two minor spelling fixes:
s/succesfully/successfully/
s/PRocRenderCreateCursor/ProcRenderCreateCursor/
Comment 39 Matthieu Herrb 2008-05-29 14:38:10 UTC
As per Alan Coopersmith request, the new disclosure date is set to june 11, 14:00 UTC. 

I've notified iDefense of the new date and the new CVE Ids. 
Comment 40 Matthieu Herrb 2008-06-11 09:00:40 UTC
The patches have been commited to HEAD and server. 

Only the dbe problem that was decided as non security related stays open, because airlied latest patch doesn't apply to the HEAD of master I have, and I can't currently test that my resolution of the conflicts even compile.
Comment 41 Julien Cristau 2009-05-21 02:26:03 UTC
Closing, this was fixed almost a year ago.