Bug 9267 - multiple integer overflows in XRender and Dbe extensions
Summary: multiple integer overflows in XRender and Dbe extensions
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Server/General (show other bugs)
Version: 7.1 (2006.05)
Hardware: All All
: high critical
Assignee: X.Org Security
QA Contact:
URL:
Whiteboard:
Keywords: security
Depends on:
Blocks:
 
Reported: 2006-12-06 13:36 UTC by Matthieu Herrb
Modified: 2007-12-10 21:34 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
1st iDefense draft advisory (6.68 KB, text/plain)
2006-12-06 13:37 UTC, Matthieu Herrb
no flags Details
Proof of concept for 1st advisory (6.46 KB, application/zip)
2006-12-06 13:38 UTC, Matthieu Herrb
no flags Details
2nd draft advisory (5.81 KB, text/plain)
2006-12-06 13:38 UTC, Matthieu Herrb
no flags Details
poc for 2nd advisory (4.04 KB, application/zip)
2006-12-06 13:39 UTC, Matthieu Herrb
no flags Details
3rd draft advisory (5.99 KB, text/plain)
2006-12-06 13:40 UTC, Matthieu Herrb
no flags Details
Poc for 3rd advisory (3.97 KB, application/zip)
2006-12-06 13:40 UTC, Matthieu Herrb
no flags Details
Proposed patch (1.57 KB, patch)
2006-12-07 13:03 UTC, Matthieu Herrb
no flags Details | Splinter Review
Updated patch for git master (5.31 KB, patch)
2007-01-06 01:17 UTC, Matthieu Herrb
no flags Details | Splinter Review
patch for xserver 1.2 branch (5.20 KB, patch)
2007-01-06 01:19 UTC, Matthieu Herrb
no flags Details | Splinter Review

Description Matthieu Herrb 2006-12-06 13:36:04 UTC
iDefense has reported several vulnerabilities in XRender and DBE extensions,
based on integer overflows.
Comment 1 Matthieu Herrb 2006-12-06 13:37:26 UTC
Created attachment 7986 [details]
1st iDefense draft advisory
Comment 2 Matthieu Herrb 2006-12-06 13:38:23 UTC
Created attachment 7987 [details]
Proof of concept for 1st advisory

POC for 1st advisory
Comment 3 Matthieu Herrb 2006-12-06 13:38:57 UTC
Created attachment 7988 [details]
2nd draft advisory
Comment 4 Matthieu Herrb 2006-12-06 13:39:40 UTC
Created attachment 7989 [details]
poc for 2nd advisory
Comment 5 Matthieu Herrb 2006-12-06 13:40:12 UTC
Created attachment 7990 [details]
3rd draft advisory
Comment 6 Matthieu Herrb 2006-12-06 13:40:54 UTC
Created attachment 7991 [details]
Poc for 3rd advisory
Comment 7 Matthieu Herrb 2006-12-07 09:03:24 UTC
Here are CVE ids allocated by Josh Bresser:

CVE-2006-6101 iDefense X.org ProcRenderAddGlyphs
CVE-2006-6102 iDefense X.org ProcDbeGetVisualInfo
CVE-2006-6103 iDefense X.org ProcDbeSwapBuffers

Comment 8 Matthieu Herrb 2006-12-07 13:03:21 UTC
Created attachment 8007 [details] [review]
Proposed patch

Any suggestion for a disclosure date? before or after 7.2 release?
Comment 9 Stefan Dirsch 2006-12-12 22:20:26 UTC
(In reply to comment #8)
> Created an attachment (id=8007) [edit]
> Proposed patch
> 
> Any suggestion for a disclosure date? before or after 7.2 release?

Any update on this one? SUSE/Novell would be ready for a release with the 
proposed patch.
Comment 10 Alan Coopersmith 2006-12-13 10:59:07 UTC
From Sun's point of view, we'd prefer one week prior notice to disclosure and
to not announce between Dec. 23 & Jan 1.   Other than that, we're open to 
whenever the community and other vendors are ready.
Comment 11 Alan Coopersmith 2006-12-15 17:08:32 UTC
I haven't seen any proposed dates, so I'll nominate one:
Tuesday, January 9, 2007

Lets everyone get back from the holidays for a week to get everything ready 
to go.  This work for everyone?


Also, I had to make one small adjustment to the proposed patches on older
Solaris releases - I changed the include blocks to:

 +#if HAVE_STDINT_H
 +#include <stdint.h>
 +#elif !defined(UINT32_MAX)
 +#define UINT32_MAX 0xffffffffU
 +#endif

On releases older than Solaris 10, UINT32_MAX was defined in <inttypes.h> based
on a draft of the C99 spec before C99 switched to <stdint.h>, so they don't have
stdint.h, but give a redefined macro warning if you try to define UINT32_MAX
anyway.
Comment 12 Matthieu Herrb 2006-12-17 02:21:57 UTC
ok for me. 14:00 UTC ?

I was waiting for feedback from ajax or daniel on the 7.2 release date, but they
seem to be buzy enough with the release process.

I'll send that to iDefense and vendor-sec tomorrow. 
Comment 13 Adam Jackson 2006-12-18 14:22:19 UTC
Jan 9 is fine for Red Hat.

7.2 _should_ be baked before then, and we'll just have to do a 0-day release. 
These things happen.
Comment 14 Matthieu Herrb 2006-12-20 00:50:15 UTC
I'm appending comments from iDefense to my patch. I'm not sure what the impact
of turning off alloca is, and other areas may have problems too since the 
3 places where iDefense spotted the integer overflows are not the only places
where ALLOCATE_LOCAL is passed a size that comes from the client.


Matthieu,

These patches do not properly fix the vulnerabilities.  In addition to
the integer overflow vulnerabilities, there are also vulnerabilities
present when ALLOCATE_LOCAL is #define'd to use alloca().   This is the
default on all of the platforms I verified these vulnerabilities on.
The following behavior:

alloca(some_user_controlled_value);

is inherently insecure.  GNU versions of alloca() merely subtract the
size argument from the current stack pointer after rounding the size up
to a 16 byte bounds.  This lets the stack pointer be shifted down to
point to other segments.

A possible "fix" would be to check if the parameter being passed to
alloca() is greater than some value.  This is still not a valid fix.
AFAIK, it is not possible know the bounds of the stack segment, or if
there is another segment mapped directly below it.  An attacker could
grow the stack low enough, perhaps using some recursive function or by
padding the environment, so that it lays directly above another mapped
segment.  If the current stack pointer is only a few hundred bytes above
the bottom of the stack, then even a small value passed to alloca() can
shift the pointer into a different segment.  Simply put there is not a
portable and secure way to use the alloca() function.  In order to fix
these vulnerabilities I would recommend using malloc().

Sean Larsson
Comment 15 Matthieu Herrb 2007-01-06 01:17:11 UTC
Created attachment 8309 [details] [review]
Updated patch for git master

No one commented on Alan's proposition to turn off alloca globally in the X
server.
I'm a bit uncomfortable with doing this without torough testing of possible
performance impacts, so I'm proposing a patch that just turns the involved
alloca's into heap allocations.
This one is for git's master branch
Comment 16 Matthieu Herrb 2007-01-06 01:19:59 UTC
Created attachment 8310 [details] [review]
patch for xserver 1.2 branch

And here's a patch for the 1.2 branch.
Comment 17 Matthieu Herrb 2007-01-09 06:27:16 UTC
Patches committed.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.