| 1 | From 6070d07a25b33e9630f8dbbde1d1df6f25164286 Mon Sep 17 00:00:00 2001 |
| 2 | From: Adam Jackson <ajax@redhat.com> |
| 3 | Date: Mon, 10 Nov 2014 12:13:43 -0500 |
| 4 | Subject: [PATCH 27/33] glx: Length checking for RenderLarge requests (v2) |
| 5 | [CVE-2014-8098 3/8] |
| 6 | |
| 7 | This is a half-measure until we start passing request length into the |
| 8 | varsize function, but it's better than the nothing we had before. |
| 9 | |
| 10 | v2: Verify that there's at least a large render header's worth of |
| 11 | dataBytes (Julien Cristau) |
| 12 | |
| 13 | Reviewed-by: Michal Srb <msrb@suse.com> |
| 14 | Reviewed-by: Andy Ritger <aritger@nvidia.com> |
| 15 | Signed-off-by: Adam Jackson <ajax@redhat.com> |
| 16 | Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> |
| 17 | --- |
| 18 | glx/glxcmds.c | 57 ++++++++++++++++++++++++++++++++++----------------------- |
| 19 | 1 file changed, 34 insertions(+), 23 deletions(-) |
| 20 | |
| 21 | --- a/glx/glxcmds.c |
| 22 | +++ b/glx/glxcmds.c |
| 23 | @@ -2099,6 +2099,8 @@ __glXDisp_RenderLarge(__GLXclientState * |
| 24 | |
| 25 | __GLX_DECLARE_SWAP_VARIABLES; |
| 26 | |
| 27 | + REQUEST_AT_LEAST_SIZE(xGLXRenderLargeReq); |
| 28 | + |
| 29 | req = (xGLXRenderLargeReq *) pc; |
| 30 | if (client->swapped) { |
| 31 | __GLX_SWAP_SHORT(&req->length); |
| 32 | @@ -2114,12 +2116,14 @@ __glXDisp_RenderLarge(__GLXclientState * |
| 33 | __glXResetLargeCommandStatus(cl); |
| 34 | return error; |
| 35 | } |
| 36 | + if (safe_pad(req->dataBytes) < 0) |
| 37 | + return BadLength; |
| 38 | dataBytes = req->dataBytes; |
| 39 | |
| 40 | /* |
| 41 | ** Check the request length. |
| 42 | */ |
| 43 | - if ((req->length << 2) != __GLX_PAD(dataBytes) + sz_xGLXRenderLargeReq) { |
| 44 | + if ((req->length << 2) != safe_pad(dataBytes) + sz_xGLXRenderLargeReq) { |
| 45 | client->errorValue = req->length; |
| 46 | /* Reset in case this isn't 1st request. */ |
| 47 | __glXResetLargeCommandStatus(cl); |
| 48 | @@ -2129,7 +2133,7 @@ __glXDisp_RenderLarge(__GLXclientState * |
| 49 | |
| 50 | if (cl->largeCmdRequestsSoFar == 0) { |
| 51 | __GLXrenderSizeData entry; |
| 52 | - int extra; |
| 53 | + int extra = 0; |
| 54 | size_t cmdlen; |
| 55 | int err; |
| 56 | |
| 57 | @@ -2142,13 +2146,17 @@ __glXDisp_RenderLarge(__GLXclientState * |
| 58 | return __glXError(GLXBadLargeRequest); |
| 59 | } |
| 60 | |
| 61 | + if (dataBytes < __GLX_RENDER_LARGE_HDR_SIZE) |
| 62 | + return BadLength; |
| 63 | + |
| 64 | hdr = (__GLXrenderLargeHeader *) pc; |
| 65 | if (client->swapped) { |
| 66 | __GLX_SWAP_INT(&hdr->length); |
| 67 | __GLX_SWAP_INT(&hdr->opcode); |
| 68 | } |
| 69 | - cmdlen = hdr->length; |
| 70 | opcode = hdr->opcode; |
| 71 | + if ((cmdlen = safe_pad(hdr->length)) < 0) |
| 72 | + return BadLength; |
| 73 | |
| 74 | /* |
| 75 | ** Check for core opcodes and grab entry data. |
| 76 | @@ -2170,17 +2178,13 @@ __glXDisp_RenderLarge(__GLXclientState * |
| 77 | if (extra < 0) { |
| 78 | return BadLength; |
| 79 | } |
| 80 | - /* large command's header is 4 bytes longer, so add 4 */ |
| 81 | - if (cmdlen != __GLX_PAD(entry.bytes + 4 + extra)) { |
| 82 | - return BadLength; |
| 83 | - } |
| 84 | } |
| 85 | - else { |
| 86 | - /* constant size command */ |
| 87 | - if (cmdlen != __GLX_PAD(entry.bytes + 4)) { |
| 88 | - return BadLength; |
| 89 | - } |
| 90 | + |
| 91 | + /* the +4 is safe because we know entry.bytes is small */ |
| 92 | + if (cmdlen != safe_pad(safe_add(entry.bytes + 4, extra))) { |
| 93 | + return BadLength; |
| 94 | } |
| 95 | + |
| 96 | /* |
| 97 | ** Make enough space in the buffer, then copy the entire request. |
| 98 | */ |
| 99 | @@ -2207,6 +2211,7 @@ __glXDisp_RenderLarge(__GLXclientState * |
| 100 | ** We are receiving subsequent (i.e. not the first) requests of a |
| 101 | ** multi request command. |
| 102 | */ |
| 103 | + int bytesSoFar; /* including this packet */ |
| 104 | |
| 105 | /* |
| 106 | ** Check the request number and the total request count. |
| 107 | @@ -2225,11 +2230,18 @@ __glXDisp_RenderLarge(__GLXclientState * |
| 108 | /* |
| 109 | ** Check that we didn't get too much data. |
| 110 | */ |
| 111 | - if ((cl->largeCmdBytesSoFar + dataBytes) > cl->largeCmdBytesTotal) { |
| 112 | + if ((bytesSoFar = safe_add(cl->largeCmdBytesSoFar, dataBytes)) < 0) { |
| 113 | client->errorValue = dataBytes; |
| 114 | __glXResetLargeCommandStatus(cl); |
| 115 | return __glXError(GLXBadLargeRequest); |
| 116 | } |
| 117 | + |
| 118 | + if (bytesSoFar > cl->largeCmdBytesTotal) { |
| 119 | + client->errorValue = dataBytes; |
| 120 | + __glXResetLargeCommandStatus(cl); |
| 121 | + return __glXError(GLXBadLargeRequest); |
| 122 | + } |
| 123 | + |
| 124 | memcpy(cl->largeCmdBuf + cl->largeCmdBytesSoFar, pc, dataBytes); |
| 125 | cl->largeCmdBytesSoFar += dataBytes; |
| 126 | cl->largeCmdRequestsSoFar++; |
| 127 | @@ -2241,17 +2253,16 @@ __glXDisp_RenderLarge(__GLXclientState * |
| 128 | ** This is the last request; it must have enough bytes to complete |
| 129 | ** the command. |
| 130 | */ |
| 131 | - /* NOTE: the two pad macros have been added below; they are needed |
| 132 | - ** because the client library pads the total byte count, but not |
| 133 | - ** the per-request byte counts. The Protocol Encoding says the |
| 134 | - ** total byte count should not be padded, so a proposal will be |
| 135 | - ** made to the ARB to relax the padding constraint on the total |
| 136 | - ** byte count, thus preserving backward compatibility. Meanwhile, |
| 137 | - ** the padding done below fixes a bug that did not allow |
| 138 | - ** large commands of odd sizes to be accepted by the server. |
| 139 | + /* NOTE: the pad macro below is needed because the client library |
| 140 | + ** pads the total byte count, but not the per-request byte counts. |
| 141 | + ** The Protocol Encoding says the total byte count should not be |
| 142 | + ** padded, so a proposal will be made to the ARB to relax the |
| 143 | + ** padding constraint on the total byte count, thus preserving |
| 144 | + ** backward compatibility. Meanwhile, the padding done below |
| 145 | + ** fixes a bug that did not allow large commands of odd sizes to |
| 146 | + ** be accepted by the server. |
| 147 | */ |
| 148 | - if (__GLX_PAD(cl->largeCmdBytesSoFar) != |
| 149 | - __GLX_PAD(cl->largeCmdBytesTotal)) { |
| 150 | + if (safe_pad(cl->largeCmdBytesSoFar) != cl->largeCmdBytesTotal) { |
| 151 | client->errorValue = dataBytes; |
| 152 | __glXResetLargeCommandStatus(cl); |
| 153 | return __glXError(GLXBadLargeRequest); |