Skip to content

Commit

Permalink
Bitmap update fix (FreeRDP#7349)
Browse files Browse the repository at this point in the history
* Added checks for bitmap width and heigth values

Data received from the server might have invalid values for bitmap
with or height. Abort parsing if such a value is found.
Reported by Sunglin from the Knownsec 404 team & 0103 sec team

* Added checks for glyph width & height
  • Loading branch information
akallabeth committed Oct 15, 2021
1 parent 7f05f1a commit 503942b
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 0 deletions.
14 changes: 14 additions & 0 deletions libfreerdp/core/orders.c
Expand Up @@ -1881,6 +1881,13 @@ static BOOL update_read_fast_glyph_order(wStream* s, const ORDER_INFO* orderInfo
!update_read_2byte_unsigned(&sub, &glyph->cy))
return FALSE;

if ((glyph->cx == 0) || (glyph->cy == 0))
{
WLog_ERR(TAG, "GLYPH_DATA_V2::cx=%" PRIu32 ", GLYPH_DATA_V2::cy=%" PRIu32,
glyph->cx, glyph->cy);
return FALSE;
}

glyph->cb = Stream_GetRemainingLength(&sub);
if (glyph->cb > 0)
{
Expand Down Expand Up @@ -2867,6 +2874,13 @@ update_read_create_offscreen_bitmap_order(wStream* s,
Stream_Read_UINT16(s, create_offscreen_bitmap->cy); /* cy (2 bytes) */
deleteList = &(create_offscreen_bitmap->deleteList);

if ((create_offscreen_bitmap->cx == 0) || (create_offscreen_bitmap->cy == 0))
{
WLog_ERR(TAG, "Invalid OFFSCREEN_DELETE_LIST: cx=%" PRIu16 ", cy=%" PRIu16,
create_offscreen_bitmap->cx, create_offscreen_bitmap->cy);
return FALSE;
}

if (deleteListPresent)
{
UINT32 i;
Expand Down
45 changes: 45 additions & 0 deletions libfreerdp/core/surface.c
Expand Up @@ -21,6 +21,8 @@
#include "config.h"
#endif

#include <winpr/assert.h>

#include <freerdp/utils/pcap.h>
#include <freerdp/log.h>

Expand Down Expand Up @@ -62,6 +64,13 @@ static BOOL update_recv_surfcmd_bitmap_ex(wStream* s, TS_BITMAP_DATA_EX* bmp)
Stream_Read_UINT16(s, bmp->height);
Stream_Read_UINT32(s, bmp->bitmapDataLength);

if ((bmp->width == 0) || (bmp->height == 0))
{
WLog_ERR(TAG, "invalid size value width=%" PRIu16 ", height=%" PRIu16, bmp->width,
bmp->height);
return FALSE;
}

if ((bmp->bpp < 1) || (bmp->bpp > 32))
{
WLog_ERR(TAG, "invalid bpp value %" PRIu32 "", bmp->bpp);
Expand All @@ -85,6 +94,39 @@ static BOOL update_recv_surfcmd_bitmap_ex(wStream* s, TS_BITMAP_DATA_EX* bmp)
return TRUE;
}

static BOOL update_recv_surfcmd_is_rect_valid(const rdpContext* context,
const SURFACE_BITS_COMMAND* cmd)
{
WINPR_ASSERT(context);
WINPR_ASSERT(context->settings);
WINPR_ASSERT(cmd);

/* We need a rectangle with left/top being smaller than right/bottom.
* Also do not allow empty rectangles. */
if ((cmd->destTop >= cmd->destBottom) || (cmd->destLeft >= cmd->destRight))
{
WLog_WARN(TAG,
"Empty surface bits command rectangle: %" PRIu16 "x%" PRIu16 "-%" PRIu16
"x%" PRIu16,
cmd->destLeft, cmd->destTop, cmd->destRight, cmd->destBottom);
return FALSE;
}

/* The rectangle needs to fit into our session size */
if ((cmd->destRight > context->settings->DesktopWidth) ||
(cmd->destBottom > context->settings->DesktopHeight))
{
WLog_WARN(TAG,
"Invalid surface bits command rectangle: %" PRIu16 "x%" PRIu16 "-%" PRIu16
"x%" PRIu16 " does not fit %" PRIu32 "x%" PRIu32,
cmd->destLeft, cmd->destTop, cmd->destRight, cmd->destBottom,
context->settings->DesktopWidth, context->settings->DesktopHeight);
return FALSE;
}

return TRUE;
}

static BOOL update_recv_surfcmd_surface_bits(rdpUpdate* update, wStream* s, UINT16 cmdType)
{
SURFACE_BITS_COMMAND cmd = { 0 };
Expand All @@ -98,6 +140,9 @@ static BOOL update_recv_surfcmd_surface_bits(rdpUpdate* update, wStream* s, UINT
Stream_Read_UINT16(s, cmd.destRight);
Stream_Read_UINT16(s, cmd.destBottom);

if (!update_recv_surfcmd_is_rect_valid(update->context, &cmd))
goto fail;

if (!update_recv_surfcmd_bitmap_ex(s, &cmd.bmp))
goto fail;

Expand Down
7 changes: 7 additions & 0 deletions libfreerdp/core/update.c
Expand Up @@ -99,6 +99,13 @@ static BOOL update_read_bitmap_data(rdpUpdate* update, wStream* s, BITMAP_DATA*
Stream_Read_UINT16(s, bitmapData->flags);
Stream_Read_UINT16(s, bitmapData->bitmapLength);

if ((bitmapData->width == 0) || (bitmapData->height == 0))
{
WLog_ERR(TAG, "Invalid BITMAP_DATA: width=%" PRIu16 ", height=%" PRIu16, bitmapData->width,
bitmapData->height);
return FALSE;
}

if (bitmapData->flags & BITMAP_COMPRESSION)
{
if (!(bitmapData->flags & NO_BITMAP_COMPRESSION_HDR))
Expand Down

0 comments on commit 503942b

Please sign in to comment.