Bug 15222

Created attachment 15498 [details]
Render vulnerabilities draft advisory

Created attachment 15499 [details]
MIT-SHM vulnerabilities draft advisory

Created attachment 15500 [details]
Prof of concept and additional information

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.

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

(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?

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.

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.

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

(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

(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).

(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

can somebody attach the zip from idefence to this bug?

(In reply to comment #13)
> can somebody attach the zip from idefence to this bug?
> 

It is there (4th attachment). 

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.

(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.

(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?

(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. 

Thanks for clarification, Matthieu.

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) ?

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

Yes, a shift to May 20 would be very much appreciated. Speaking as X.Org maintainer for SUSE.

(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 #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

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.

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.

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?

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.

so can we agree a new Embargo date?

some holiday Monday in the U.S. coming up I think...

(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.

I can do June 3rd sounds good to me.

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?

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.

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.

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). 

Created attachment 16793 [details]
Draft X.Org advisory 

Attached is a draft for the X.Org advisory. Comments, corrections are welcome.

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.

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/

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. 

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.

Closing, this was fixed almost a year ago.