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
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
|
From eae57493feec958bcf733ad0d334715107029f8b Mon Sep 17 00:00:00 2001
From: Alan Coopersmith <alan.coopersmith@oracle.com>
Date: Sat, 9 Mar 2013 11:29:21 -0800
Subject: [PATCH 1/2] Unchecked return values of XGetWindowProperty
[CVE-2013-2005]
Multiple functions in Selection.c assumed that XGetWindowProperty() would
always set the pointer to the property, but before libX11 1.6, it could
fail to do so in some cases, leading to libXt freeing or operating on an
uninitialized pointer value, so libXt should always initialize the pointers
and check for failure itself.
Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
---
src/Selection.c | 84 ++++++++++++++++++++++++++++++++-------------------------
1 file changed, 47 insertions(+), 37 deletions(-)
diff --git a/src/Selection.c b/src/Selection.c
index f35cb44..4f59d70 100644
--- a/src/Selection.c
+++ b/src/Selection.c
@@ -839,14 +839,16 @@ static void HandleSelectionEvents(
IndirectPair *p;
int format;
unsigned long bytesafter, length;
- unsigned char *value;
+ unsigned char *value = NULL;
ev.property = event->xselectionrequest.property;
StartProtectedSection(ev.display, ev.requestor);
- (void) XGetWindowProperty(ev.display, ev.requestor,
+ if (XGetWindowProperty(ev.display, ev.requestor,
event->xselectionrequest.property, 0L, 1000000,
False,(Atom)AnyPropertyType, &target, &format, &length,
- &bytesafter, &value);
- count = BYTELENGTH(length, format) / sizeof(IndirectPair);
+ &bytesafter, &value) == Success)
+ count = BYTELENGTH(length, format) / sizeof(IndirectPair);
+ else
+ count = 0;
for (p = (IndirectPair *)value; count; p++, count--) {
EndProtectedSection(ctx->dpy);
if (!GetConversion(ctx, (XSelectionRequestEvent*)event,
@@ -1053,9 +1055,10 @@ static Boolean IsINCRtype(
if (prop == None) return False;
- (void)XGetWindowProperty(XtDisplay(info->widget), window, prop, 0L, 0L,
- False, info->ctx->prop_list->incr_atom,
- &type, &format, &length, &bytesafter, &value);
+ if (XGetWindowProperty(XtDisplay(info->widget), window, prop, 0L, 0L,
+ False, info->ctx->prop_list->incr_atom, &type,
+ &format, &length, &bytesafter, &value) != Success)
+ return False;
return (type == info->ctx->prop_list->incr_atom);
}
@@ -1069,7 +1072,6 @@ static void ReqCleanup(
{
CallBackInfo info = (CallBackInfo)closure;
unsigned long bytesafter, length;
- char *value;
int format;
Atom target;
@@ -1093,17 +1095,19 @@ static void ReqCleanup(
(ev->xproperty.state == PropertyNewValue) &&
(ev->xproperty.atom == info->property)) {
XPropertyEvent *event = (XPropertyEvent *) ev;
- (void) XGetWindowProperty(event->display, XtWindow(widget),
- event->atom, 0L, 1000000, True, AnyPropertyType,
- &target, &format, &length, &bytesafter,
- (unsigned char **) &value);
- XFree(value);
- if (length == 0) {
- XtRemoveEventHandler(widget, (EventMask) PropertyChangeMask, FALSE,
- ReqCleanup, (XtPointer) info );
- FreeSelectionProperty(XtDisplay(widget), info->property);
- XtFree(info->value); /* requestor never got this, so free now */
- FreeInfo(info);
+ char *value = NULL;
+ if (XGetWindowProperty(event->display, XtWindow(widget),
+ event->atom, 0L, 1000000, True, AnyPropertyType,
+ &target, &format, &length, &bytesafter,
+ (unsigned char **) &value) == Success) {
+ XFree(value);
+ if (length == 0) {
+ XtRemoveEventHandler(widget, (EventMask) PropertyChangeMask,
+ FALSE, ReqCleanup, (XtPointer) info );
+ FreeSelectionProperty(XtDisplay(widget), info->property);
+ XtFree(info->value); /* requestor never got this, so free now */
+ FreeInfo(info);
+ }
}
}
}
@@ -1121,20 +1125,23 @@ static void ReqTimedOut(
unsigned long bytesafter;
unsigned long proplength;
Atom type;
- IndirectPair *pairs;
XtPointer *c;
int i;
if (*info->target == info->ctx->prop_list->indirect_atom) {
- (void) XGetWindowProperty(XtDisplay(info->widget),
- XtWindow(info->widget), info->property, 0L,
- 10000000, True, AnyPropertyType, &type, &format,
- &proplength, &bytesafter, (unsigned char **) &pairs);
- XFree((char*)pairs);
- for (proplength = proplength / IndirectPairWordSize, i = 0, c = info->req_closure;
- proplength; proplength--, c++, i++)
- (*info->callbacks[i])(info->widget, *c,
- &info->ctx->selection, &resulttype, value, &length, &format);
+ IndirectPair *pairs = NULL;
+ if (XGetWindowProperty(XtDisplay(info->widget), XtWindow(info->widget),
+ info->property, 0L, 10000000, True,
+ AnyPropertyType, &type, &format, &proplength,
+ &bytesafter, (unsigned char **) &pairs)
+ == Success) {
+ XFree(pairs);
+ for (proplength = proplength / IndirectPairWordSize, i = 0,
+ c = info->req_closure;
+ proplength; proplength--, c++, i++)
+ (*info->callbacks[i])(info->widget, *c, &info->ctx->selection,
+ &resulttype, value, &length, &format);
+ }
} else {
(*info->callbacks[0])(info->widget, *info->req_closure,
&info->ctx->selection, &resulttype, value, &length, &format);
@@ -1280,12 +1287,13 @@ Boolean HandleNormal(
unsigned long length;
int format;
Atom type;
- unsigned char *value;
+ unsigned char *value = NULL;
int number = info->current;
- (void) XGetWindowProperty(dpy, XtWindow(widget), property, 0L,
- 10000000, False, AnyPropertyType,
- &type, &format, &length, &bytesafter, &value);
+ if (XGetWindowProperty(dpy, XtWindow(widget), property, 0L, 10000000,
+ False, AnyPropertyType, &type, &format, &length,
+ &bytesafter, &value) != Success)
+ return FALSE;
if (type == info->ctx->prop_list->incr_atom) {
unsigned long size = IncrPropSize(widget, value, format, length);
@@ -1370,7 +1378,6 @@ static void HandleSelectionReplies(
Display *dpy = event->display;
CallBackInfo info = (CallBackInfo) closure;
Select ctx = info->ctx;
- IndirectPair *pairs, *p;
unsigned long bytesafter;
unsigned long length;
int format;
@@ -1385,9 +1392,12 @@ static void HandleSelectionReplies(
XtRemoveEventHandler(widget, (EventMask)0, TRUE,
HandleSelectionReplies, (XtPointer) info );
if (event->target == ctx->prop_list->indirect_atom) {
- (void) XGetWindowProperty(dpy, XtWindow(widget), info->property, 0L,
- 10000000, True, AnyPropertyType, &type, &format,
- &length, &bytesafter, (unsigned char **) &pairs);
+ IndirectPair *pairs = NULL, *p;
+ if (XGetWindowProperty(dpy, XtWindow(widget), info->property, 0L,
+ 10000000, True, AnyPropertyType, &type, &format,
+ &length, &bytesafter, (unsigned char **) &pairs)
+ != Success)
+ length = 0;
for (length = length / IndirectPairWordSize, p = pairs,
c = info->req_closure;
length; length--, p++, c++, info->current++) {
--
1.8.2.3
|
