1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
|
From 873f86cb8b8fb5a1efabd8b39a50242ef093136f Mon Sep 17 00:00:00 2001
From: Alan Coopersmith <alan.coopersmith@oracle.com>
Date: Sun, 10 Mar 2013 13:30:55 -0700
Subject: [PATCH 13/16] Avoid integer overflow in XGetDeviceProperties()
[CVE-2013-1984 7/8]
If the number of items as reported by the Xserver is too large, it
could overflow the calculation for the size of the buffer to copy the
reply into, causing memory corruption.
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 17071c1c608247800b2ca03a35b1fcc9c4cabe6c)
(cherry picked from commit 7be9dcc311e29ed326e53117904648cb51ee21df)
---
src/XGetDProp.c | 61 +++++++++++++++++++++++++++++++++---------------------
1 files changed, 37 insertions(+), 24 deletions(-)
diff --git a/src/XGetDProp.c b/src/XGetDProp.c
index f9e8f0c..3691122 100644
--- a/src/XGetDProp.c
+++ b/src/XGetDProp.c
@@ -38,6 +38,7 @@ in this Software without prior written authorization from the author.
#include <X11/extensions/XInput.h>
#include <X11/extensions/extutil.h>
#include "XIint.h"
+#include <limits.h>
int
XGetDeviceProperty(Display* dpy, XDevice* dev,
@@ -48,7 +49,8 @@ XGetDeviceProperty(Display* dpy, XDevice* dev,
{
xGetDevicePropertyReq *req;
xGetDevicePropertyReply rep;
- long nbytes, rbytes;
+ unsigned long nbytes, rbytes;
+ int ret = Success;
XExtDisplayInfo *info = XInput_find_display(dpy);
@@ -81,30 +83,43 @@ XGetDeviceProperty(Display* dpy, XDevice* dev,
* data, but this last byte is null terminated and convenient for
* returning string properties, so the client doesn't then have to
* recopy the string to make it null terminated.
+ *
+ * Maximum item limits are set to both prevent integer overflow when
+ * calculating the amount of memory to malloc, and to limit how much
+ * memory will be used if a server provides an insanely high count.
*/
switch (rep.format) {
case 8:
- nbytes = rep.nItems;
- rbytes = rep.nItems + 1;
- if (rbytes > 0 &&
- (*prop = (unsigned char *) Xmalloc ((unsigned)rbytes)))
- _XReadPad (dpy, (char *) *prop, nbytes);
+ if (rep.nItems < INT_MAX) {
+ nbytes = rep.nItems;
+ rbytes = rep.nItems + 1;
+ if ((*prop = Xmalloc (rbytes)))
+ _XReadPad (dpy, (char *) *prop, nbytes);
+ else
+ ret = BadAlloc;
+ }
break;
case 16:
- nbytes = rep.nItems << 1;
- rbytes = rep.nItems * sizeof (short) + 1;
- if (rbytes > 0 &&
- (*prop = (unsigned char *) Xmalloc ((unsigned)rbytes)))
- _XRead16Pad (dpy, (short *) *prop, nbytes);
+ if (rep.nItems < (INT_MAX / sizeof (short))) {
+ nbytes = rep.nItems << 1;
+ rbytes = rep.nItems * sizeof (short) + 1;
+ if ((*prop = Xmalloc (rbytes)))
+ _XRead16Pad (dpy, (short *) *prop, nbytes);
+ else
+ ret = BadAlloc;
+ }
break;
case 32:
- nbytes = rep.nItems << 2;
- rbytes = rep.nItems * sizeof (long) + 1;
- if (rbytes > 0 &&
- (*prop = (unsigned char *) Xmalloc ((unsigned)rbytes)))
- _XRead32 (dpy, (long *) *prop, nbytes);
+ if (rep.nItems < (INT_MAX / sizeof (long))) {
+ nbytes = rep.nItems << 2;
+ rbytes = rep.nItems * sizeof (long) + 1;
+ if ((*prop = Xmalloc (rbytes)))
+ _XRead32 (dpy, (long *) *prop, nbytes);
+ else
+ ret = BadAlloc;
+ }
break;
default:
@@ -112,16 +127,13 @@ XGetDeviceProperty(Display* dpy, XDevice* dev,
* This part of the code should never be reached. If it is,
* the server sent back a property with an invalid format.
*/
- _XEatDataWords(dpy, rep.length);
- UnlockDisplay(dpy);
- SyncHandle();
- return(BadImplementation);
+ ret = BadImplementation;
}
if (! *prop) {
_XEatDataWords(dpy, rep.length);
- UnlockDisplay(dpy);
- SyncHandle();
- return(BadAlloc);
+ if (ret == Success)
+ ret = BadAlloc;
+ goto out;
}
(*prop)[rbytes - 1] = '\0';
}
@@ -130,9 +142,10 @@ XGetDeviceProperty(Display* dpy, XDevice* dev,
*actual_format = rep.format;
*nitems = rep.nItems;
*bytes_after = rep.bytesAfter;
+ out:
UnlockDisplay (dpy);
SyncHandle ();
- return Success;
+ return ret;
}
--
1.7.7.1
|
