From d264da92f7f8129b8aad4f0114a6467fc38fc896 Mon Sep 17 00:00:00 2001 From: Nathan Kidd Date: Sun, 21 Dec 2014 01:10:03 -0500 Subject: [PATCH] hw/xfree86: unvalidated lengths This addresses: CVE-2017-12180 in XFree86-VidModeExtension CVE-2017-12181 in XFree86-DGA CVE-2017-12182 in XFree86-DRI Reviewed-by: Jeremy Huddleston Sequoia Reviewed-by: Julien Cristau Signed-off-by: Nathan Kidd Signed-off-by: Julien Cristau (cherry picked from commit 1b1d4c04695dced2463404174b50b3581dbd857b) --- Xext/vidmode.c | 129 +++++++++++++++++++----------------- hw/xfree86/common/xf86DGA.c | 81 ++++++++++++---------- hw/xfree86/dri/xf86dri.c | 1 + 3 files changed, 117 insertions(+), 94 deletions(-) diff --git a/Xext/vidmode.c b/Xext/vidmode.c index ea3ad1320..76055c89a 100644 --- a/Xext/vidmode.c +++ b/Xext/vidmode.c @@ -454,6 +454,20 @@ ProcVidModeAddModeLine(ClientPtr client) DEBUG_P("XF86VidModeAddModeline"); ver = ClientMajorVersion(client); + + if (ver < 2) { + REQUEST_AT_LEAST_SIZE(xXF86OldVidModeAddModeLineReq); + len = + client->req_len - + bytes_to_int32(sizeof(xXF86OldVidModeAddModeLineReq)); + } + else { + REQUEST_AT_LEAST_SIZE(xXF86VidModeAddModeLineReq); + len = + client->req_len - + bytes_to_int32(sizeof(xXF86VidModeAddModeLineReq)); + } + if (ver < 2) { /* convert from old format */ stuff = &newstuff; @@ -501,18 +515,6 @@ ProcVidModeAddModeLine(ClientPtr client) stuff->after_vsyncend, stuff->after_vtotal, (unsigned long) stuff->after_flags); - if (ver < 2) { - REQUEST_AT_LEAST_SIZE(xXF86OldVidModeAddModeLineReq); - len = - client->req_len - - bytes_to_int32(sizeof(xXF86OldVidModeAddModeLineReq)); - } - else { - REQUEST_AT_LEAST_SIZE(xXF86VidModeAddModeLineReq); - len = - client->req_len - - bytes_to_int32(sizeof(xXF86VidModeAddModeLineReq)); - } if (len != stuff->privsize) return BadLength; @@ -622,6 +624,20 @@ ProcVidModeDeleteModeLine(ClientPtr client) DEBUG_P("XF86VidModeDeleteModeline"); ver = ClientMajorVersion(client); + + if (ver < 2) { + REQUEST_AT_LEAST_SIZE(xXF86OldVidModeDeleteModeLineReq); + len = + client->req_len - + bytes_to_int32(sizeof(xXF86OldVidModeDeleteModeLineReq)); + } + else { + REQUEST_AT_LEAST_SIZE(xXF86VidModeDeleteModeLineReq); + len = + client->req_len - + bytes_to_int32(sizeof(xXF86VidModeDeleteModeLineReq)); + } + if (ver < 2) { /* convert from old format */ stuff = &newstuff; @@ -649,18 +665,6 @@ ProcVidModeDeleteModeLine(ClientPtr client) stuff->vdisplay, stuff->vsyncstart, stuff->vsyncend, stuff->vtotal, (unsigned long) stuff->flags); - if (ver < 2) { - REQUEST_AT_LEAST_SIZE(xXF86OldVidModeDeleteModeLineReq); - len = - client->req_len - - bytes_to_int32(sizeof(xXF86OldVidModeDeleteModeLineReq)); - } - else { - REQUEST_AT_LEAST_SIZE(xXF86VidModeDeleteModeLineReq); - len = - client->req_len - - bytes_to_int32(sizeof(xXF86VidModeDeleteModeLineReq)); - } if (len != stuff->privsize) { DebugF("req_len = %ld, sizeof(Req) = %d, privsize = %ld, " "len = %d, length = %d\n", @@ -744,6 +748,20 @@ ProcVidModeModModeLine(ClientPtr client) DEBUG_P("XF86VidModeModModeline"); ver = ClientMajorVersion(client); + + if (ver < 2) { + REQUEST_AT_LEAST_SIZE(xXF86OldVidModeModModeLineReq); + len = + client->req_len - + bytes_to_int32(sizeof(xXF86OldVidModeModModeLineReq)); + } + else { + REQUEST_AT_LEAST_SIZE(xXF86VidModeModModeLineReq); + len = + client->req_len - + bytes_to_int32(sizeof(xXF86VidModeModModeLineReq)); + } + if (ver < 2) { /* convert from old format */ stuff = &newstuff; @@ -768,18 +786,6 @@ ProcVidModeModModeLine(ClientPtr client) stuff->vdisplay, stuff->vsyncstart, stuff->vsyncend, stuff->vtotal, (unsigned long) stuff->flags); - if (ver < 2) { - REQUEST_AT_LEAST_SIZE(xXF86OldVidModeModModeLineReq); - len = - client->req_len - - bytes_to_int32(sizeof(xXF86OldVidModeModModeLineReq)); - } - else { - REQUEST_AT_LEAST_SIZE(xXF86VidModeModModeLineReq); - len = - client->req_len - - bytes_to_int32(sizeof(xXF86VidModeModModeLineReq)); - } if (len != stuff->privsize) return BadLength; @@ -877,6 +883,19 @@ ProcVidModeValidateModeLine(ClientPtr client) DEBUG_P("XF86VidModeValidateModeline"); ver = ClientMajorVersion(client); + + if (ver < 2) { + REQUEST_AT_LEAST_SIZE(xXF86OldVidModeValidateModeLineReq); + len = client->req_len - + bytes_to_int32(sizeof(xXF86OldVidModeValidateModeLineReq)); + } + else { + REQUEST_AT_LEAST_SIZE(xXF86VidModeValidateModeLineReq); + len = + client->req_len - + bytes_to_int32(sizeof(xXF86VidModeValidateModeLineReq)); + } + if (ver < 2) { /* convert from old format */ stuff = &newstuff; @@ -905,17 +924,6 @@ ProcVidModeValidateModeLine(ClientPtr client) stuff->vdisplay, stuff->vsyncstart, stuff->vsyncend, stuff->vtotal, (unsigned long) stuff->flags); - if (ver < 2) { - REQUEST_AT_LEAST_SIZE(xXF86OldVidModeValidateModeLineReq); - len = client->req_len - - bytes_to_int32(sizeof(xXF86OldVidModeValidateModeLineReq)); - } - else { - REQUEST_AT_LEAST_SIZE(xXF86VidModeValidateModeLineReq); - len = - client->req_len - - bytes_to_int32(sizeof(xXF86VidModeValidateModeLineReq)); - } if (len != stuff->privsize) return BadLength; @@ -1027,6 +1035,20 @@ ProcVidModeSwitchToMode(ClientPtr client) DEBUG_P("XF86VidModeSwitchToMode"); ver = ClientMajorVersion(client); + + if (ver < 2) { + REQUEST_AT_LEAST_SIZE(xXF86OldVidModeSwitchToModeReq); + len = + client->req_len - + bytes_to_int32(sizeof(xXF86OldVidModeSwitchToModeReq)); + } + else { + REQUEST_AT_LEAST_SIZE(xXF86VidModeSwitchToModeReq); + len = + client->req_len - + bytes_to_int32(sizeof(xXF86VidModeSwitchToModeReq)); + } + if (ver < 2) { /* convert from old format */ stuff = &newstuff; @@ -1055,18 +1077,6 @@ ProcVidModeSwitchToMode(ClientPtr client) stuff->vdisplay, stuff->vsyncstart, stuff->vsyncend, stuff->vtotal, (unsigned long) stuff->flags); - if (ver < 2) { - REQUEST_AT_LEAST_SIZE(xXF86OldVidModeSwitchToModeReq); - len = - client->req_len - - bytes_to_int32(sizeof(xXF86OldVidModeSwitchToModeReq)); - } - else { - REQUEST_AT_LEAST_SIZE(xXF86VidModeSwitchToModeReq); - len = - client->req_len - - bytes_to_int32(sizeof(xXF86VidModeSwitchToModeReq)); - } if (len != stuff->privsize) return BadLength; @@ -1457,6 +1467,7 @@ ProcVidModeSetGammaRamp(ClientPtr client) VidModePtr pVidMode; REQUEST(xXF86VidModeSetGammaRampReq); + REQUEST_AT_LEAST_SIZE(xXF86VidModeSetGammaRampReq); if (stuff->screen >= screenInfo.numScreens) return BadValue; diff --git a/hw/xfree86/common/xf86DGA.c b/hw/xfree86/common/xf86DGA.c index c689dcb73..039f38dfa 100644 --- a/hw/xfree86/common/xf86DGA.c +++ b/hw/xfree86/common/xf86DGA.c @@ -1272,13 +1272,14 @@ ProcXDGAOpenFramebuffer(ClientPtr client) char *deviceName; int nameSize; + REQUEST_SIZE_MATCH(xXDGAOpenFramebufferReq); + if (stuff->screen >= screenInfo.numScreens) return BadValue; if (!DGAAvailable(stuff->screen)) return DGAErrorBase + XF86DGANoDirectVideoMode; - REQUEST_SIZE_MATCH(xXDGAOpenFramebufferReq); rep.type = X_Reply; rep.length = 0; rep.sequenceNumber = client->sequence; @@ -1305,14 +1306,14 @@ ProcXDGACloseFramebuffer(ClientPtr client) { REQUEST(xXDGACloseFramebufferReq); + REQUEST_SIZE_MATCH(xXDGACloseFramebufferReq); + if (stuff->screen >= screenInfo.numScreens) return BadValue; if (!DGAAvailable(stuff->screen)) return DGAErrorBase + XF86DGANoDirectVideoMode; - REQUEST_SIZE_MATCH(xXDGACloseFramebufferReq); - DGACloseFramebuffer(stuff->screen); return Success; @@ -1328,10 +1329,11 @@ ProcXDGAQueryModes(ClientPtr client) xXDGAModeInfo info; XDGAModePtr mode; + REQUEST_SIZE_MATCH(xXDGAQueryModesReq); + if (stuff->screen >= screenInfo.numScreens) return BadValue; - REQUEST_SIZE_MATCH(xXDGAQueryModesReq); rep.type = X_Reply; rep.length = 0; rep.number = 0; @@ -1443,11 +1445,12 @@ ProcXDGASetMode(ClientPtr client) ClientPtr owner; int size; + REQUEST_SIZE_MATCH(xXDGASetModeReq); + if (stuff->screen >= screenInfo.numScreens) return BadValue; owner = DGA_GETCLIENT(stuff->screen); - REQUEST_SIZE_MATCH(xXDGASetModeReq); rep.type = X_Reply; rep.length = 0; rep.offset = 0; @@ -1533,14 +1536,14 @@ ProcXDGASetViewport(ClientPtr client) { REQUEST(xXDGASetViewportReq); + REQUEST_SIZE_MATCH(xXDGASetViewportReq); + if (stuff->screen >= screenInfo.numScreens) return BadValue; if (DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; - REQUEST_SIZE_MATCH(xXDGASetViewportReq); - DGASetViewport(stuff->screen, stuff->x, stuff->y, stuff->flags); return Success; @@ -1554,14 +1557,14 @@ ProcXDGAInstallColormap(ClientPtr client) REQUEST(xXDGAInstallColormapReq); + REQUEST_SIZE_MATCH(xXDGAInstallColormapReq); + if (stuff->screen >= screenInfo.numScreens) return BadValue; if (DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; - REQUEST_SIZE_MATCH(xXDGAInstallColormapReq); - rc = dixLookupResourceByType((void **) &cmap, stuff->cmap, RT_COLORMAP, client, DixInstallAccess); if (rc != Success) @@ -1575,14 +1578,14 @@ ProcXDGASelectInput(ClientPtr client) { REQUEST(xXDGASelectInputReq); + REQUEST_SIZE_MATCH(xXDGASelectInputReq); + if (stuff->screen >= screenInfo.numScreens) return BadValue; if (DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; - REQUEST_SIZE_MATCH(xXDGASelectInputReq); - if (DGA_GETCLIENT(stuff->screen) == client) DGASelectInput(stuff->screen, client, stuff->mask); @@ -1594,14 +1597,14 @@ ProcXDGAFillRectangle(ClientPtr client) { REQUEST(xXDGAFillRectangleReq); + REQUEST_SIZE_MATCH(xXDGAFillRectangleReq); + if (stuff->screen >= screenInfo.numScreens) return BadValue; if (DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; - REQUEST_SIZE_MATCH(xXDGAFillRectangleReq); - if (Success != DGAFillRect(stuff->screen, stuff->x, stuff->y, stuff->width, stuff->height, stuff->color)) return BadMatch; @@ -1614,14 +1617,14 @@ ProcXDGACopyArea(ClientPtr client) { REQUEST(xXDGACopyAreaReq); + REQUEST_SIZE_MATCH(xXDGACopyAreaReq); + if (stuff->screen >= screenInfo.numScreens) return BadValue; if (DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; - REQUEST_SIZE_MATCH(xXDGACopyAreaReq); - if (Success != DGABlitRect(stuff->screen, stuff->srcx, stuff->srcy, stuff->width, stuff->height, stuff->dstx, stuff->dsty)) @@ -1635,14 +1638,14 @@ ProcXDGACopyTransparentArea(ClientPtr client) { REQUEST(xXDGACopyTransparentAreaReq); + REQUEST_SIZE_MATCH(xXDGACopyTransparentAreaReq); + if (stuff->screen >= screenInfo.numScreens) return BadValue; if (DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; - REQUEST_SIZE_MATCH(xXDGACopyTransparentAreaReq); - if (Success != DGABlitTransRect(stuff->screen, stuff->srcx, stuff->srcy, stuff->width, stuff->height, stuff->dstx, stuff->dsty, stuff->key)) @@ -1657,13 +1660,14 @@ ProcXDGAGetViewportStatus(ClientPtr client) REQUEST(xXDGAGetViewportStatusReq); xXDGAGetViewportStatusReply rep; + REQUEST_SIZE_MATCH(xXDGAGetViewportStatusReq); + if (stuff->screen >= screenInfo.numScreens) return BadValue; if (DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; - REQUEST_SIZE_MATCH(xXDGAGetViewportStatusReq); rep.type = X_Reply; rep.length = 0; rep.sequenceNumber = client->sequence; @@ -1680,13 +1684,14 @@ ProcXDGASync(ClientPtr client) REQUEST(xXDGASyncReq); xXDGASyncReply rep; + REQUEST_SIZE_MATCH(xXDGASyncReq); + if (stuff->screen >= screenInfo.numScreens) return BadValue; if (DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; - REQUEST_SIZE_MATCH(xXDGASyncReq); rep.type = X_Reply; rep.length = 0; rep.sequenceNumber = client->sequence; @@ -1725,13 +1730,14 @@ ProcXDGAChangePixmapMode(ClientPtr client) xXDGAChangePixmapModeReply rep; int x, y; + REQUEST_SIZE_MATCH(xXDGAChangePixmapModeReq); + if (stuff->screen >= screenInfo.numScreens) return BadValue; if (DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; - REQUEST_SIZE_MATCH(xXDGAChangePixmapModeReq); rep.type = X_Reply; rep.length = 0; rep.sequenceNumber = client->sequence; @@ -1755,14 +1761,14 @@ ProcXDGACreateColormap(ClientPtr client) REQUEST(xXDGACreateColormapReq); int result; + REQUEST_SIZE_MATCH(xXDGACreateColormapReq); + if (stuff->screen >= screenInfo.numScreens) return BadValue; if (DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; - REQUEST_SIZE_MATCH(xXDGACreateColormapReq); - if (!stuff->mode) return BadValue; @@ -1791,10 +1797,11 @@ ProcXF86DGAGetVideoLL(ClientPtr client) int num, offset, flags; char *name; + REQUEST_SIZE_MATCH(xXF86DGAGetVideoLLReq); + if (stuff->screen >= screenInfo.numScreens) return BadValue; - REQUEST_SIZE_MATCH(xXF86DGAGetVideoLLReq); rep.type = X_Reply; rep.length = 0; rep.sequenceNumber = client->sequence; @@ -1831,9 +1838,10 @@ ProcXF86DGADirectVideo(ClientPtr client) REQUEST(xXF86DGADirectVideoReq); + REQUEST_SIZE_MATCH(xXF86DGADirectVideoReq); + if (stuff->screen >= screenInfo.numScreens) return BadValue; - REQUEST_SIZE_MATCH(xXF86DGADirectVideoReq); if (!DGAAvailable(stuff->screen)) return DGAErrorBase + XF86DGANoDirectVideoMode; @@ -1889,10 +1897,11 @@ ProcXF86DGAGetViewPortSize(ClientPtr client) REQUEST(xXF86DGAGetViewPortSizeReq); xXF86DGAGetViewPortSizeReply rep; + REQUEST_SIZE_MATCH(xXF86DGAGetViewPortSizeReq); + if (stuff->screen >= screenInfo.numScreens) return BadValue; - REQUEST_SIZE_MATCH(xXF86DGAGetViewPortSizeReq); rep.type = X_Reply; rep.length = 0; rep.sequenceNumber = client->sequence; @@ -1917,14 +1926,14 @@ ProcXF86DGASetViewPort(ClientPtr client) { REQUEST(xXF86DGASetViewPortReq); + REQUEST_SIZE_MATCH(xXF86DGASetViewPortReq); + if (stuff->screen >= screenInfo.numScreens) return BadValue; if (DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; - REQUEST_SIZE_MATCH(xXF86DGASetViewPortReq); - if (!DGAAvailable(stuff->screen)) return DGAErrorBase + XF86DGANoDirectVideoMode; @@ -1944,10 +1953,11 @@ ProcXF86DGAGetVidPage(ClientPtr client) REQUEST(xXF86DGAGetVidPageReq); xXF86DGAGetVidPageReply rep; + REQUEST_SIZE_MATCH(xXF86DGAGetVidPageReq); + if (stuff->screen >= screenInfo.numScreens) return BadValue; - REQUEST_SIZE_MATCH(xXF86DGAGetVidPageReq); rep.type = X_Reply; rep.length = 0; rep.sequenceNumber = client->sequence; @@ -1962,11 +1972,11 @@ ProcXF86DGASetVidPage(ClientPtr client) { REQUEST(xXF86DGASetVidPageReq); + REQUEST_SIZE_MATCH(xXF86DGASetVidPageReq); + if (stuff->screen >= screenInfo.numScreens) return BadValue; - REQUEST_SIZE_MATCH(xXF86DGASetVidPageReq); - /* silently fail */ return Success; @@ -1980,14 +1990,14 @@ ProcXF86DGAInstallColormap(ClientPtr client) REQUEST(xXF86DGAInstallColormapReq); + REQUEST_SIZE_MATCH(xXF86DGAInstallColormapReq); + if (stuff->screen >= screenInfo.numScreens) return BadValue; if (DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; - REQUEST_SIZE_MATCH(xXF86DGAInstallColormapReq); - if (!DGAActive(stuff->screen)) return DGAErrorBase + XF86DGADirectNotActivated; @@ -2008,10 +2018,11 @@ ProcXF86DGAQueryDirectVideo(ClientPtr client) REQUEST(xXF86DGAQueryDirectVideoReq); xXF86DGAQueryDirectVideoReply rep; + REQUEST_SIZE_MATCH(xXF86DGAQueryDirectVideoReq); + if (stuff->screen >= screenInfo.numScreens) return BadValue; - REQUEST_SIZE_MATCH(xXF86DGAQueryDirectVideoReq); rep.type = X_Reply; rep.length = 0; rep.sequenceNumber = client->sequence; @@ -2030,14 +2041,14 @@ ProcXF86DGAViewPortChanged(ClientPtr client) REQUEST(xXF86DGAViewPortChangedReq); xXF86DGAViewPortChangedReply rep; + REQUEST_SIZE_MATCH(xXF86DGAViewPortChangedReq); + if (stuff->screen >= screenInfo.numScreens) return BadValue; if (DGA_GETCLIENT(stuff->screen) != client) return DGAErrorBase + XF86DGADirectNotActivated; - REQUEST_SIZE_MATCH(xXF86DGAViewPortChangedReq); - if (!DGAActive(stuff->screen)) return DGAErrorBase + XF86DGADirectNotActivated; diff --git a/hw/xfree86/dri/xf86dri.c b/hw/xfree86/dri/xf86dri.c index 68f8b7e72..65f368efd 100644 --- a/hw/xfree86/dri/xf86dri.c +++ b/hw/xfree86/dri/xf86dri.c @@ -570,6 +570,7 @@ static int SProcXF86DRIQueryDirectRenderingCapable(register ClientPtr client) { REQUEST(xXF86DRIQueryDirectRenderingCapableReq); + REQUEST_SIZE_MATCH(xXF86DRIQueryDirectRenderingCapableReq); swaps(&stuff->length); swapl(&stuff->screen); return ProcXF86DRIQueryDirectRenderingCapable(client);