|
1 From eae57493feec958bcf733ad0d334715107029f8b Mon Sep 17 00:00:00 2001 |
|
2 From: Alan Coopersmith <[email protected]> |
|
3 Date: Sat, 9 Mar 2013 11:29:21 -0800 |
|
4 Subject: [PATCH:libXt 1/3] Unchecked return values of XGetWindowProperty |
|
5 [CVE-2013-2005] |
|
6 |
|
7 Multiple functions in Selection.c assumed that XGetWindowProperty() would |
|
8 always set the pointer to the property, but before libX11 1.6, it could |
|
9 fail to do so in some cases, leading to libXt freeing or operating on an |
|
10 uninitialized pointer value, so libXt should always initialize the pointers |
|
11 and check for failure itself. |
|
12 |
|
13 Reported-by: Ilja Van Sprundel <[email protected]> |
|
14 Signed-off-by: Alan Coopersmith <[email protected]> |
|
15 --- |
|
16 src/Selection.c | 84 +++++++++++++++++++++++++++++++------------------------ |
|
17 1 file changed, 47 insertions(+), 37 deletions(-) |
|
18 |
|
19 diff --git a/src/Selection.c b/src/Selection.c |
|
20 index f35cb44..4f59d70 100644 |
|
21 --- a/src/Selection.c |
|
22 +++ b/src/Selection.c |
|
23 @@ -839,14 +839,16 @@ static void HandleSelectionEvents( |
|
24 IndirectPair *p; |
|
25 int format; |
|
26 unsigned long bytesafter, length; |
|
27 - unsigned char *value; |
|
28 + unsigned char *value = NULL; |
|
29 ev.property = event->xselectionrequest.property; |
|
30 StartProtectedSection(ev.display, ev.requestor); |
|
31 - (void) XGetWindowProperty(ev.display, ev.requestor, |
|
32 + if (XGetWindowProperty(ev.display, ev.requestor, |
|
33 event->xselectionrequest.property, 0L, 1000000, |
|
34 False,(Atom)AnyPropertyType, &target, &format, &length, |
|
35 - &bytesafter, &value); |
|
36 - count = BYTELENGTH(length, format) / sizeof(IndirectPair); |
|
37 + &bytesafter, &value) == Success) |
|
38 + count = BYTELENGTH(length, format) / sizeof(IndirectPair); |
|
39 + else |
|
40 + count = 0; |
|
41 for (p = (IndirectPair *)value; count; p++, count--) { |
|
42 EndProtectedSection(ctx->dpy); |
|
43 if (!GetConversion(ctx, (XSelectionRequestEvent*)event, |
|
44 @@ -1053,9 +1055,10 @@ static Boolean IsINCRtype( |
|
45 |
|
46 if (prop == None) return False; |
|
47 |
|
48 - (void)XGetWindowProperty(XtDisplay(info->widget), window, prop, 0L, 0L, |
|
49 - False, info->ctx->prop_list->incr_atom, |
|
50 - &type, &format, &length, &bytesafter, &value); |
|
51 + if (XGetWindowProperty(XtDisplay(info->widget), window, prop, 0L, 0L, |
|
52 + False, info->ctx->prop_list->incr_atom, &type, |
|
53 + &format, &length, &bytesafter, &value) != Success) |
|
54 + return False; |
|
55 |
|
56 return (type == info->ctx->prop_list->incr_atom); |
|
57 } |
|
58 @@ -1069,7 +1072,6 @@ static void ReqCleanup( |
|
59 { |
|
60 CallBackInfo info = (CallBackInfo)closure; |
|
61 unsigned long bytesafter, length; |
|
62 - char *value; |
|
63 int format; |
|
64 Atom target; |
|
65 |
|
66 @@ -1093,17 +1095,19 @@ static void ReqCleanup( |
|
67 (ev->xproperty.state == PropertyNewValue) && |
|
68 (ev->xproperty.atom == info->property)) { |
|
69 XPropertyEvent *event = (XPropertyEvent *) ev; |
|
70 - (void) XGetWindowProperty(event->display, XtWindow(widget), |
|
71 - event->atom, 0L, 1000000, True, AnyPropertyType, |
|
72 - &target, &format, &length, &bytesafter, |
|
73 - (unsigned char **) &value); |
|
74 - XFree(value); |
|
75 - if (length == 0) { |
|
76 - XtRemoveEventHandler(widget, (EventMask) PropertyChangeMask, FALSE, |
|
77 - ReqCleanup, (XtPointer) info ); |
|
78 - FreeSelectionProperty(XtDisplay(widget), info->property); |
|
79 - XtFree(info->value); /* requestor never got this, so free now */ |
|
80 - FreeInfo(info); |
|
81 + char *value = NULL; |
|
82 + if (XGetWindowProperty(event->display, XtWindow(widget), |
|
83 + event->atom, 0L, 1000000, True, AnyPropertyType, |
|
84 + &target, &format, &length, &bytesafter, |
|
85 + (unsigned char **) &value) == Success) { |
|
86 + XFree(value); |
|
87 + if (length == 0) { |
|
88 + XtRemoveEventHandler(widget, (EventMask) PropertyChangeMask, |
|
89 + FALSE, ReqCleanup, (XtPointer) info ); |
|
90 + FreeSelectionProperty(XtDisplay(widget), info->property); |
|
91 + XtFree(info->value); /* requestor never got this, so free now */ |
|
92 + FreeInfo(info); |
|
93 + } |
|
94 } |
|
95 } |
|
96 } |
|
97 @@ -1121,20 +1125,23 @@ static void ReqTimedOut( |
|
98 unsigned long bytesafter; |
|
99 unsigned long proplength; |
|
100 Atom type; |
|
101 - IndirectPair *pairs; |
|
102 XtPointer *c; |
|
103 int i; |
|
104 |
|
105 if (*info->target == info->ctx->prop_list->indirect_atom) { |
|
106 - (void) XGetWindowProperty(XtDisplay(info->widget), |
|
107 - XtWindow(info->widget), info->property, 0L, |
|
108 - 10000000, True, AnyPropertyType, &type, &format, |
|
109 - &proplength, &bytesafter, (unsigned char **) &pairs); |
|
110 - XFree((char*)pairs); |
|
111 - for (proplength = proplength / IndirectPairWordSize, i = 0, c = info->req_closure; |
|
112 - proplength; proplength--, c++, i++) |
|
113 - (*info->callbacks[i])(info->widget, *c, |
|
114 - &info->ctx->selection, &resulttype, value, &length, &format); |
|
115 + IndirectPair *pairs = NULL; |
|
116 + if (XGetWindowProperty(XtDisplay(info->widget), XtWindow(info->widget), |
|
117 + info->property, 0L, 10000000, True, |
|
118 + AnyPropertyType, &type, &format, &proplength, |
|
119 + &bytesafter, (unsigned char **) &pairs) |
|
120 + == Success) { |
|
121 + XFree(pairs); |
|
122 + for (proplength = proplength / IndirectPairWordSize, i = 0, |
|
123 + c = info->req_closure; |
|
124 + proplength; proplength--, c++, i++) |
|
125 + (*info->callbacks[i])(info->widget, *c, &info->ctx->selection, |
|
126 + &resulttype, value, &length, &format); |
|
127 + } |
|
128 } else { |
|
129 (*info->callbacks[0])(info->widget, *info->req_closure, |
|
130 &info->ctx->selection, &resulttype, value, &length, &format); |
|
131 @@ -1280,12 +1287,13 @@ Boolean HandleNormal( |
|
132 unsigned long length; |
|
133 int format; |
|
134 Atom type; |
|
135 - unsigned char *value; |
|
136 + unsigned char *value = NULL; |
|
137 int number = info->current; |
|
138 |
|
139 - (void) XGetWindowProperty(dpy, XtWindow(widget), property, 0L, |
|
140 - 10000000, False, AnyPropertyType, |
|
141 - &type, &format, &length, &bytesafter, &value); |
|
142 + if (XGetWindowProperty(dpy, XtWindow(widget), property, 0L, 10000000, |
|
143 + False, AnyPropertyType, &type, &format, &length, |
|
144 + &bytesafter, &value) != Success) |
|
145 + return FALSE; |
|
146 |
|
147 if (type == info->ctx->prop_list->incr_atom) { |
|
148 unsigned long size = IncrPropSize(widget, value, format, length); |
|
149 @@ -1370,7 +1378,6 @@ static void HandleSelectionReplies( |
|
150 Display *dpy = event->display; |
|
151 CallBackInfo info = (CallBackInfo) closure; |
|
152 Select ctx = info->ctx; |
|
153 - IndirectPair *pairs, *p; |
|
154 unsigned long bytesafter; |
|
155 unsigned long length; |
|
156 int format; |
|
157 @@ -1385,9 +1392,12 @@ static void HandleSelectionReplies( |
|
158 XtRemoveEventHandler(widget, (EventMask)0, TRUE, |
|
159 HandleSelectionReplies, (XtPointer) info ); |
|
160 if (event->target == ctx->prop_list->indirect_atom) { |
|
161 - (void) XGetWindowProperty(dpy, XtWindow(widget), info->property, 0L, |
|
162 - 10000000, True, AnyPropertyType, &type, &format, |
|
163 - &length, &bytesafter, (unsigned char **) &pairs); |
|
164 + IndirectPair *pairs = NULL, *p; |
|
165 + if (XGetWindowProperty(dpy, XtWindow(widget), info->property, 0L, |
|
166 + 10000000, True, AnyPropertyType, &type, &format, |
|
167 + &length, &bytesafter, (unsigned char **) &pairs) |
|
168 + != Success) |
|
169 + length = 0; |
|
170 for (length = length / IndirectPairWordSize, p = pairs, |
|
171 c = info->req_closure; |
|
172 length; length--, p++, c++, info->current++) { |
|
173 -- |
|
174 1.7.9.2 |
|
175 |