Fixes for CVE-2014-0250

This patch introduce misc checks when receiving pointer updates. We check
that the cursor are in the bounds defined by the spec. We also check that
the announced mask sizes are what they should be.
This commit is contained in:
Hardening 2014-05-28 23:07:00 +02:00
parent ea0a2a32ad
commit 532c42052a
3 changed files with 68 additions and 13 deletions

View File

@ -299,7 +299,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;

View File

@ -286,16 +286,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 bpp)
{
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) */
@ -312,26 +328,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 + bpp * 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);
}
@ -348,7 +403,7 @@ 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 */
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)
@ -387,7 +442,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;

View File

@ -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 bpp);
BOOL update_read_pointer_new(wStream* s, POINTER_NEW_UPDATE* pointer_new);
BOOL update_read_pointer_cached(wStream* s, POINTER_CACHED_UPDATE* pointer_cached);