summaryrefslogtreecommitdiff
blob: ce4d3d989098e508098edf52a21f459f116fa7f3 (plain)
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
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
From 5ad707ddf33d1d785a8ca1fbeec91d2eee985820 Mon Sep 17 00:00:00 2001
From: Hardening <rdp.effort@gmail.com>
Date: Fri, 6 Jun 2014 23:24:16 +0200
Subject: [PATCH] Fix CVE-2014-0250

This patch fixes CVE-2014-0250 by checking width, height and bpp when
receiving a new pointer.
---
 client/X11/xf_cliprdr.c    | 11 +++++--
 libfreerdp/core/fastpath.c |  2 +-
 libfreerdp/core/orders.c   | 21 ++++++++++++
 libfreerdp/core/surface.c  |  6 ++++
 libfreerdp/core/update.c   | 82 +++++++++++++++++++++++++++++++++++++++-------
 libfreerdp/core/update.h   |  2 +-
 libfreerdp/core/window.c   |  5 +++
 7 files changed, 114 insertions(+), 15 deletions(-)

diff --git a/client/X11/xf_cliprdr.c b/client/X11/xf_cliprdr.c
index 19c4332..8fb49f9 100644
--- a/client/X11/xf_cliprdr.c
+++ b/client/X11/xf_cliprdr.c
@@ -914,7 +914,7 @@ static void xf_cliprdr_process_unicodetext(clipboardContext* cb, BYTE* data, int
 	crlf2lf(cb->data, &cb->data_length);
 }
 
-static void xf_cliprdr_process_dib(clipboardContext* cb, BYTE* data, int size)
+static BOOL xf_cliprdr_process_dib(clipboardContext* cb, BYTE* data, int size)
 {
 	wStream* s;
 	UINT16 bpp;
@@ -926,12 +926,18 @@ static void xf_cliprdr_process_dib(clipboardContext* cb, BYTE* data, int size)
 	if (size < 40)
 	{
 		DEBUG_X11_CLIPRDR("dib size %d too short", size);
-		return;
+		return FALSE;
 	}
 
 	s = Stream_New(data, size);
 	Stream_Seek(s, 14);
 	Stream_Read_UINT16(s, bpp);
+	if ((bpp < 1) || (bpp > 32))
+	{
+		fprintf(stderr, "%s: invalid bpp value %d", __FUNCTION__, bpp);
+		return FALSE;
+	}
+
 	Stream_Read_UINT32(s, ncolors);
 	offset = 14 + 40 + (bpp <= 8 ? (ncolors == 0 ? (1 << bpp) : ncolors) * 4 : 0);
 	Stream_Free(s, FALSE);
@@ -949,6 +955,7 @@ static void xf_cliprdr_process_dib(clipboardContext* cb, BYTE* data, int size)
 	cb->data = Stream_Buffer(s);
 	cb->data_length = Stream_GetPosition(s);
 	Stream_Free(s, FALSE);
+	return TRUE;
 }
 
 static void xf_cliprdr_process_html(clipboardContext* cb, BYTE* data, int size)
diff --git a/libfreerdp/core/fastpath.c b/libfreerdp/core/fastpath.c
index 8448160..dcc7117 100644
--- a/libfreerdp/core/fastpath.c
+++ b/libfreerdp/core/fastpath.c
@@ -254,7 +254,7 @@ static int fastpath_recv_update(rdpFastPath* fastpath, BYTE updateCode, UINT32 s
 			break;
 
 		case FASTPATH_UPDATETYPE_COLOR:
-			if (!update_read_pointer_color(s, &pointer->pointer_color))
+			if (!update_read_pointer_color(s, &pointer->pointer_color, 24))
 				return -1;
 			IFCALL(pointer->PointerColor, context, &pointer->pointer_color);
 			break;
diff --git a/libfreerdp/core/orders.c b/libfreerdp/core/orders.c
index e5cc520..f22407f 100644
--- a/libfreerdp/core/orders.c
+++ b/libfreerdp/core/orders.c
@@ -1830,6 +1830,11 @@ BOOL update_read_cache_bitmap_order(wStream* s, CACHE_BITMAP_ORDER* cache_bitmap
 	Stream_Read_UINT8(s, cache_bitmap->bitmapWidth); /* bitmapWidth (1 byte) */
 	Stream_Read_UINT8(s, cache_bitmap->bitmapHeight); /* bitmapHeight (1 byte) */
 	Stream_Read_UINT8(s, cache_bitmap->bitmapBpp); /* bitmapBpp (1 byte) */
+	if ((cache_bitmap->bitmapBpp < 1) || (cache_bitmap->bitmapBpp > 32))
+	{
+		fprintf(stderr, "%s: invalid bitmap bpp %d\n", __FUNCTION__, cache_bitmap->bitmapBpp);
+		return FALSE;
+	}
 	Stream_Read_UINT16(s, cache_bitmap->bitmapLength); /* bitmapLength (2 bytes) */
 	Stream_Read_UINT16(s, cache_bitmap->cacheIndex); /* cacheIndex (2 bytes) */
 
@@ -2068,6 +2073,11 @@ BOOL update_read_cache_bitmap_v3_order(wStream* s, CACHE_BITMAP_V3_ORDER* cache_
 	bitmapData = &cache_bitmap_v3->bitmapData;
 
 	Stream_Read_UINT8(s, bitmapData->bpp);
+	if ((bitmapData->bpp < 1) || (bitmapData->bpp > 32))
+	{
+		fprintf(stderr, "%s: invalid bpp value %d", __FUNCTION__, bitmapData->bpp);
+		return FALSE;
+	}
 	Stream_Seek_UINT8(s); /* reserved1 (1 byte) */
 	Stream_Seek_UINT8(s); /* reserved2 (1 byte) */
 	Stream_Read_UINT8(s, bitmapData->codecID); /* codecID (1 byte) */
@@ -2672,6 +2682,11 @@ BOOL update_read_create_nine_grid_bitmap_order(wStream* s, CREATE_NINE_GRID_BITM
 		return FALSE;
 
 	Stream_Read_UINT8(s, create_nine_grid_bitmap->bitmapBpp); /* bitmapBpp (1 byte) */
+	if ((create_nine_grid_bitmap->bitmapBpp < 1) || (create_nine_grid_bitmap->bitmapBpp > 32))
+	{
+		fprintf(stderr, "%s: invalid bpp value %d", __FUNCTION__, create_nine_grid_bitmap->bitmapBpp);
+		return FALSE;
+	}
 	Stream_Read_UINT16(s, create_nine_grid_bitmap->bitmapId); /* bitmapId (2 bytes) */
 
 	nineGridInfo = &(create_nine_grid_bitmap->nineGridInfo);
@@ -2707,6 +2722,12 @@ BOOL update_read_stream_bitmap_first_order(wStream* s, STREAM_BITMAP_FIRST_ORDER
 
 	Stream_Read_UINT8(s, stream_bitmap_first->bitmapFlags); /* bitmapFlags (1 byte) */
 	Stream_Read_UINT8(s, stream_bitmap_first->bitmapBpp); /* bitmapBpp (1 byte) */
+	if ((stream_bitmap_first->bitmapBpp < 1) || (stream_bitmap_first->bitmapBpp > 32))
+	{
+		fprintf(stderr, "%s: invalid bpp value %d", __FUNCTION__, stream_bitmap_first->bitmapBpp);
+		return FALSE;
+	}
+
 	Stream_Read_UINT16(s, stream_bitmap_first->bitmapType); /* bitmapType (2 bytes) */
 	Stream_Read_UINT16(s, stream_bitmap_first->bitmapWidth); /* bitmapWidth (2 bytes) */
 	Stream_Read_UINT16(s, stream_bitmap_first->bitmapHeight); /* bitmapHeigth (2 bytes) */
diff --git a/libfreerdp/core/surface.c b/libfreerdp/core/surface.c
index 7d0fb22..992a3dd 100644
--- a/libfreerdp/core/surface.c
+++ b/libfreerdp/core/surface.c
@@ -38,6 +38,12 @@ static int update_recv_surfcmd_surface_bits(rdpUpdate* update, wStream* s, UINT3
 	Stream_Read_UINT16(s, cmd->destRight);
 	Stream_Read_UINT16(s, cmd->destBottom);
 	Stream_Read_UINT8(s, cmd->bpp);
+	if ((cmd->bpp < 1) || (cmd->bpp > 32))
+	{
+		fprintf(stderr, "%s: invalid bpp value %d", __FUNCTION__, cmd->bpp);
+		return FALSE;
+	}
+
 	Stream_Seek(s, 2); /* reserved1, reserved2 */
 	Stream_Read_UINT8(s, cmd->codecID);
 	Stream_Read_UINT16(s, cmd->width);
diff --git a/libfreerdp/core/update.c b/libfreerdp/core/update.c
index c4eaede..27c36e3 100644
--- a/libfreerdp/core/update.c
+++ b/libfreerdp/core/update.c
@@ -219,16 +219,32 @@ BOOL update_read_pointer_system(wStream* s, POINTER_SYSTEM_UPDATE* pointer_syste
 	return TRUE;
 }
 
-BOOL update_read_pointer_color(wStream* s, POINTER_COLOR_UPDATE* pointer_color)
+BOOL update_read_pointer_color(wStream* s, POINTER_COLOR_UPDATE* pointer_color, int xorBpp)
 {
+	BYTE *newMask;
+	int scanlineSize;
+
 	if (Stream_GetRemainingLength(s) < 14)
 		return FALSE;
 
 	Stream_Read_UINT16(s, pointer_color->cacheIndex); /* cacheIndex (2 bytes) */
 	Stream_Read_UINT16(s, pointer_color->xPos); /* xPos (2 bytes) */
 	Stream_Read_UINT16(s, pointer_color->yPos); /* yPos (2 bytes) */
+
+	/**
+	 *  As stated in 2.2.9.1.1.4.4 Color Pointer Update:
+	 *  The maximum allowed pointer width/height is 96 pixels if the client indicated support
+	 *  for large pointers by setting the LARGE_POINTER_FLAG (0x00000001) in the Large
+	 *  Pointer Capability Set (section 2.2.7.2.7). If the LARGE_POINTER_FLAG was not
+	 *  set, the maximum allowed pointer width/height is 32 pixels.
+	 *
+	 *  So we check for a maximum of 96 for CVE-2014-0250.
+	 */
 	Stream_Read_UINT16(s, pointer_color->width); /* width (2 bytes) */
 	Stream_Read_UINT16(s, pointer_color->height); /* height (2 bytes) */
+	if ((pointer_color->width > 96) || (pointer_color->height > 96))
+		return FALSE;
+
 	Stream_Read_UINT16(s, pointer_color->lengthAndMask); /* lengthAndMask (2 bytes) */
 	Stream_Read_UINT16(s, pointer_color->lengthXorMask); /* lengthXorMask (2 bytes) */
 
@@ -245,26 +261,65 @@ BOOL update_read_pointer_color(wStream* s, POINTER_COLOR_UPDATE* pointer_color)
 
 	if (pointer_color->lengthXorMask > 0)
 	{
+		/**
+		 * Spec states that:
+		 *
+		 * xorMaskData (variable): A variable-length array of bytes. Contains the 24-bpp, bottom-up
+		 * XOR mask scan-line data. The XOR mask is padded to a 2-byte boundary for each encoded
+		 * scan-line. For example, if a 3x3 pixel cursor is being sent, then each scan-line will consume 10
+		 * bytes (3 pixels per scan-line multiplied by 3 bytes per pixel, rounded up to the next even
+		 * number of bytes).
+		 *
+		 * In fact instead of 24-bpp, the bpp parameter is given by the containing packet.
+		 */
 		if (Stream_GetRemainingLength(s) < pointer_color->lengthXorMask)
 			return FALSE;
 
-		if (!pointer_color->xorMaskData)
-			pointer_color->xorMaskData = malloc(pointer_color->lengthXorMask);
-		else
-			pointer_color->xorMaskData = realloc(pointer_color->xorMaskData, pointer_color->lengthXorMask);
+		scanlineSize = (7 + xorBpp * pointer_color->width) / 8;
+		scanlineSize = ((scanlineSize + 1) / 2) * 2;
+		if (scanlineSize * pointer_color->height != pointer_color->lengthXorMask)
+		{
+			fprintf(stderr, "%s: invalid lengthXorMask: width=%d height=%d, %d instead of %d\n", __FUNCTION__,
+					pointer_color->width, pointer_color->height,
+					pointer_color->lengthXorMask, scanlineSize * pointer_color->height);
+			return FALSE;
+		}
+
+		newMask = realloc(pointer_color->xorMaskData, pointer_color->lengthXorMask);
+		if (!newMask)
+			return FALSE;
+
+		pointer_color->xorMaskData = newMask;
 
 		Stream_Read(s, pointer_color->xorMaskData, pointer_color->lengthXorMask);
 	}
 
 	if (pointer_color->lengthAndMask > 0)
 	{
+		/**
+		 * andMaskData (variable): A variable-length array of bytes. Contains the 1-bpp, bottom-up
+		 * AND mask scan-line data. The AND mask is padded to a 2-byte boundary for each encoded
+		 * scan-line. For example, if a 7x7 pixel cursor is being sent, then each scan-line will consume 2
+		 * bytes (7 pixels per scan-line multiplied by 1 bpp, rounded up to the next even number of
+		 * bytes).
+		 */
 		if (Stream_GetRemainingLength(s) < pointer_color->lengthAndMask)
 			return FALSE;
 
-		if (!pointer_color->andMaskData)
-			pointer_color->andMaskData = malloc(pointer_color->lengthAndMask);
-		else
-			pointer_color->andMaskData = realloc(pointer_color->andMaskData, pointer_color->lengthAndMask);
+		scanlineSize = ((7 + pointer_color->width) / 8);
+		scanlineSize = ((1 + scanlineSize) / 2) * 2;
+		if (scanlineSize * pointer_color->height != pointer_color->lengthAndMask)
+		{
+			fprintf(stderr, "%s: invalid lengthAndMask: %d instead of %d\n", __FUNCTION__,
+					pointer_color->lengthAndMask, scanlineSize * pointer_color->height);
+			return FALSE;
+		}
+
+		newMask = realloc(pointer_color->andMaskData, pointer_color->lengthAndMask);
+		if (!newMask)
+			return FALSE;
+
+		pointer_color->andMaskData = newMask;
 
 		Stream_Read(s, pointer_color->andMaskData, pointer_color->lengthAndMask);
 	}
@@ -281,7 +336,12 @@ BOOL update_read_pointer_new(wStream* s, POINTER_NEW_UPDATE* pointer_new)
 		return FALSE;
 
 	Stream_Read_UINT16(s, pointer_new->xorBpp); /* xorBpp (2 bytes) */
-	return update_read_pointer_color(s, &pointer_new->colorPtrAttr); /* colorPtrAttr */
+	if ((pointer_new->xorBpp < 1) || (pointer_new->xorBpp > 32))
+	{
+		fprintf(stderr, "%s: invalid xorBpp %d\n", __FUNCTION__, pointer_new->xorBpp);
+		return FALSE;
+	}
+	return update_read_pointer_color(s, &pointer_new->colorPtrAttr, pointer_new->xorBpp); /* colorPtrAttr */
 }
 
 BOOL update_read_pointer_cached(wStream* s, POINTER_CACHED_UPDATE* pointer_cached)
@@ -320,7 +380,7 @@ BOOL update_recv_pointer(rdpUpdate* update, wStream* s)
 			break;
 
 		case PTR_MSG_TYPE_COLOR:
-			if (!update_read_pointer_color(s, &pointer->pointer_color))
+			if (!update_read_pointer_color(s, &pointer->pointer_color, 24))
 				return FALSE;
 			IFCALL(pointer->PointerColor, context, &pointer->pointer_color);
 			break;
diff --git a/libfreerdp/core/update.h b/libfreerdp/core/update.h
index c3088f2..d6c2d59 100644
--- a/libfreerdp/core/update.h
+++ b/libfreerdp/core/update.h
@@ -53,7 +53,7 @@ BOOL update_recv(rdpUpdate* update, wStream* s);
 
 BOOL update_read_pointer_position(wStream* s, POINTER_POSITION_UPDATE* pointer_position);
 BOOL update_read_pointer_system(wStream* s, POINTER_SYSTEM_UPDATE* pointer_system);
-BOOL update_read_pointer_color(wStream* s, POINTER_COLOR_UPDATE* pointer_color);
+BOOL update_read_pointer_color(wStream* s, POINTER_COLOR_UPDATE* pointer_color, int xorBpp);
 BOOL update_read_pointer_new(wStream* s, POINTER_NEW_UPDATE* pointer_new);
 BOOL update_read_pointer_cached(wStream* s, POINTER_CACHED_UPDATE* pointer_cached);
 
diff --git a/libfreerdp/core/window.c b/libfreerdp/core/window.c
index 7422f5b..c10fa33 100644
--- a/libfreerdp/core/window.c
+++ b/libfreerdp/core/window.c
@@ -35,6 +35,11 @@ BOOL update_read_icon_info(wStream* s, ICON_INFO* icon_info)
 	Stream_Read_UINT16(s, icon_info->cacheEntry); /* cacheEntry (2 bytes) */
 	Stream_Read_UINT8(s, icon_info->cacheId); /* cacheId (1 byte) */
 	Stream_Read_UINT8(s, icon_info->bpp); /* bpp (1 byte) */
+	if ((icon_info->bpp < 1) || (icon_info->bpp > 32))
+	{
+		fprintf(stderr, "%s: invalid bpp %d\n", __FUNCTION__, icon_info->bpp);
+		return FALSE;
+	}
 	Stream_Read_UINT16(s, icon_info->width); /* width (2 bytes) */
 	Stream_Read_UINT16(s, icon_info->height); /* height (2 bytes) */
 
-- 
1.9.3