Commit | Line | Data |
---|---|---|
7217e0ca ML |
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 | ||
4db25562 JB |
21 | --- a/glx/glxcmds.c |
22 | +++ b/glx/glxcmds.c | |
23 | @@ -2099,6 +2099,8 @@ __glXDisp_RenderLarge(__GLXclientState * | |
7217e0ca ML |
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); | |
4db25562 | 32 | @@ -2114,12 +2116,14 @@ __glXDisp_RenderLarge(__GLXclientState * |
7217e0ca ML |
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); | |
4db25562 | 48 | @@ -2129,7 +2133,7 @@ __glXDisp_RenderLarge(__GLXclientState * |
7217e0ca ML |
49 | |
50 | if (cl->largeCmdRequestsSoFar == 0) { | |
51 | __GLXrenderSizeData entry; | |
52 | - int extra; | |
53 | + int extra = 0; | |
54 | size_t cmdlen; | |
55 | int err; | |
56 | ||
4db25562 | 57 | @@ -2142,13 +2146,17 @@ __glXDisp_RenderLarge(__GLXclientState * |
7217e0ca ML |
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. | |
4db25562 | 76 | @@ -2170,17 +2178,13 @@ __glXDisp_RenderLarge(__GLXclientState * |
7217e0ca ML |
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 | */ | |
4db25562 | 99 | @@ -2207,6 +2211,7 @@ __glXDisp_RenderLarge(__GLXclientState * |
7217e0ca ML |
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. | |
4db25562 | 107 | @@ -2225,11 +2230,18 @@ __glXDisp_RenderLarge(__GLXclientState * |
7217e0ca ML |
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++; | |
4db25562 | 127 | @@ -2241,17 +2253,16 @@ __glXDisp_RenderLarge(__GLXclientState * |
7217e0ca ML |
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); |