Re: [Dri-devel] Re: [Dri-devel][PATCH] Segfault in DRI Xserver extension

Jens Owen Sun, 15 Dec 2002 19:24:01 -0800

Felix,

Thanks for finding and fixing this vulnerability in the DRI extension. I'm copying the xpert list on this fix, because it looks like their may be other extensions with the same vulnerability. For example:

xc/programs/Xserver/Xext/panoramiX.c: line 991

Felix Kühling wrote:

On Thu, 12 Dec 2002 22:26:23 +0000
Keith Whitwell <[EMAIL PROTECTED]> wrote:
Felix Kühling wrote:
Hi,

while I was messing around with my query programme I found this: if I
specify an invalid screen as argument to XF86DRIGetClientDriverName the
Xserver segfaults. I had a quick look at xc/xc/Xserver/GL/dri/xf86dri.c.
stuff->screen is used as array index without checking. I'm not sure
though, where would be the right place to fix it.

Other functions in xf86dri.c must be affacted, too. They use
stuff->screen in the same way.
Those functions should validate any information they receive over the wire, as soon as is feasible.
The problem is that the request records are all different. So we can't
check stuff->screen in ProcXF86DRIDispatch. We have to do it in each
request separately.

The attached patch does just that. While I grepped around to see how
other extensions check this I found one more potential problem in
programs/Xserver/GL/glx/glxcmds.c and two suspicious casts of screen
from unsigned to int (screen is always unsigned in the request records).
They are fixed in the attached patch as well.

Regards,
Felix

__\|/__ ___ ___ ___
__Tschüß_______\_6 6_/___/__ \___/__ \___/___\___You can do anything,___
_____Felix_______\Ä/\ \_____\ \_____\ \______U___just not everything____
[EMAIL PROTECTED] >o<__/ \___/ \___/ at the same time!

------------------------------------------------------------------------

Index: dri/xf86dri.c
===================================================================
RCS file: /cvsroot/dri/xc/xc/programs/Xserver/GL/dri/xf86dri.c,v
retrieving revision 1.10
diff -u -r1.10 xf86dri.c
--- dri/xf86dri.c 29 Oct 2002 20:28:57 -0000 1.10
+++ dri/xf86dri.c 13 Dec 2002 15:51:57 -0000
@@ -155,6 +155,11 @@
REQUEST(xXF86DRIQueryDirectRenderingCapableReq);
REQUEST_SIZE_MATCH(xXF86DRIQueryDirectRenderingCapableReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
+
rep.type = X_Reply;
rep.length = 0;
rep.sequenceNumber = client->sequence;
@@ -184,6 +189,10 @@
REQUEST(xXF86DRIOpenConnectionReq);
REQUEST_SIZE_MATCH(xXF86DRIOpenConnectionReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
if (!DRIOpenConnection( screenInfo.screens[stuff->screen], &hSAREA,
@@ -221,6 +230,10 @@
REQUEST(xXF86DRIAuthConnectionReq);
REQUEST_SIZE_MATCH(xXF86DRIAuthConnectionReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
rep.type = X_Reply;
rep.length = 0;
@@ -242,6 +255,10 @@
{
REQUEST(xXF86DRICloseConnectionReq);
REQUEST_SIZE_MATCH(xXF86DRICloseConnectionReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
DRICloseConnection( screenInfo.screens[stuff->screen]);
@@ -258,6 +275,10 @@
REQUEST(xXF86DRIGetClientDriverNameReq);
REQUEST_SIZE_MATCH(xXF86DRIGetClientDriverNameReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
DRIGetClientDriverName( screenInfo.screens[stuff->screen],
(int *)&rep.ddxDriverMajorVersion,
@@ -295,6 +316,11 @@
REQUEST(xXF86DRICreateContextReq);
REQUEST_SIZE_MATCH(xXF86DRICreateContextReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
+
rep.type = X_Reply;
rep.length = 0;
rep.sequenceNumber = client->sequence;
@@ -329,6 +355,10 @@
{
REQUEST(xXF86DRIDestroyContextReq);
REQUEST_SIZE_MATCH(xXF86DRIDestroyContextReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
if (!DRIDestroyContext( screenInfo.screens[stuff->screen],
stuff->context)) {
@@ -348,6 +378,11 @@
REQUEST(xXF86DRICreateDrawableReq);
REQUEST_SIZE_MATCH(xXF86DRICreateDrawableReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
+
rep.type = X_Reply;
rep.length = 0;
rep.sequenceNumber = client->sequence;
@@ -378,6 +413,10 @@
REQUEST(xXF86DRIDestroyDrawableReq);
DrawablePtr pDrawable;
REQUEST_SIZE_MATCH(xXF86DRIDestroyDrawableReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
if (!(pDrawable = (DrawablePtr)SecurityLookupDrawable(
(Drawable)stuff->drawable,
@@ -409,6 +448,11 @@
REQUEST(xXF86DRIGetDrawableInfoReq);
REQUEST_SIZE_MATCH(xXF86DRIGetDrawableInfoReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
+
rep.type = X_Reply;
rep.length = 0;
rep.sequenceNumber = client->sequence;
@@ -483,6 +527,11 @@
REQUEST(xXF86DRIGetDeviceInfoReq);
REQUEST_SIZE_MATCH(xXF86DRIGetDeviceInfoReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
+
rep.type = X_Reply;
rep.length = 0;
rep.sequenceNumber = client->sequence;
@@ -528,6 +577,11 @@
DrawablePtr pDrawable;
REQUEST_SIZE_MATCH(xXF86DRIOpenFullScreenReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
+
rep.type = X_Reply;
rep.length = 0;
rep.sequenceNumber = client->sequence;
@@ -554,6 +608,11 @@
DrawablePtr pDrawable;
REQUEST_SIZE_MATCH(xXF86DRICloseFullScreenReq);
+ if (stuff->screen >= screenInfo.numScreens) {
+ client->errorValue = stuff->screen;
+ return BadValue;
+ }
+
rep.type = X_Reply;
rep.length = 0;
rep.sequenceNumber = client->sequence;
Index: glx/glxcmds.c
===================================================================
RCS file: /cvsroot/dri/xc/xc/programs/Xserver/GL/glx/glxcmds.c,v
retrieving revision 1.8
diff -u -r1.8 glxcmds.c
--- glx/glxcmds.c 25 Nov 2002 19:58:38 -0000 1.8
+++ glx/glxcmds.c 13 Dec 2002 15:52:01 -0000
@@ -761,7 +761,7 @@
int i, p;
screen = req->screen;
- if (screen > screenInfo.numScreens) {
+ if (screen >= screenInfo.numScreens) {
/* The client library must send a valid screen number. */
client->errorValue = screen;
return BadValue;
@@ -1466,7 +1466,7 @@
ClientPtr client = cl->client;
xGLXQueryExtensionsStringReq *req = (xGLXQueryExtensionsStringReq *) pc;
xGLXQueryExtensionsStringReply reply;
- GLint screen;
+ GLuint screen;
size_t n, length;
const char *ptr;
char *buf;
@@ -1475,7 +1475,7 @@
/*
** Check if screen exists.
*/
- if ((screen < 0) || (screen >= screenInfo.numScreens)) {
+ if (screen >= screenInfo.numScreens) {
client->errorValue = screen;
return BadValue;
}
@@ -1511,7 +1511,7 @@
xGLXQueryServerStringReq *req = (xGLXQueryServerStringReq *) pc;
xGLXQueryServerStringReply reply;
int name;
- GLint screen;
+ GLuint screen;
size_t n, length;
const char *ptr;
char *buf;
@@ -1521,7 +1521,7 @@
/*
** Check if screen exists.
*/
- if ((screen < 0) || (screen >= screenInfo.numScreens)) {
+ if (screen >= screenInfo.numScreens) {
client->errorValue = screen;
return BadValue;
}



--
                               /\
         Jens Owen            /  \/\ _
  [EMAIL PROTECTED]  /    \ \ \   Steamboat Springs, Colorado



-------------------------------------------------------
This sf.net email is sponsored by:
With Great Power, Comes Great Responsibility
Learn to use your power at OSDN's High Performance Computing Channel
http://hpc.devchannel.org/
_______________________________________________
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel

Re: [Dri-devel] Re: [Dri-devel][PATCH] Segfault in DRI Xserver extension

Reply via email to