From ebfec72e88cf747f913ab34743b394e35b2526ef Mon Sep 17 00:00:00 2001 From: hardening Date: Mon, 28 Jan 2013 22:23:10 +0100 Subject: [PATCH] Add checks on RemoteFX --- libfreerdp/codec/rfx.c | 135 +++++++++++++++++++++++++++++----- libfreerdp/codec/rfx_decode.c | 8 +- libfreerdp/codec/rfx_decode.h | 2 +- 3 files changed, 125 insertions(+), 20 deletions(-) diff --git a/libfreerdp/codec/rfx.c b/libfreerdp/codec/rfx.c index ffc099d50..f4315210a 100644 --- a/libfreerdp/codec/rfx.c +++ b/libfreerdp/codec/rfx.c @@ -323,17 +323,22 @@ int rfx_tile_pool_return(RFX_CONTEXT* context, RFX_TILE* tile) return 0; } -static void rfx_process_message_sync(RFX_CONTEXT* context, STREAM* s) +static BOOL rfx_process_message_sync(RFX_CONTEXT* context, STREAM* s) { UINT32 magic; /* RFX_SYNC */ + if (stream_get_left(s) < 6) + { + DEBUG_WARN("RfxSync packet too small"); + return FALSE; + } stream_read_UINT32(s, magic); /* magic (4 bytes), 0xCACCACCA */ if (magic != WF_MAGIC) { DEBUG_WARN("invalid magic number 0x%X", magic); - return; + return FALSE; } stream_read_UINT16(s, context->version); /* version (2 bytes), WF_VERSION_1_0 (0x0100) */ @@ -341,22 +346,34 @@ static void rfx_process_message_sync(RFX_CONTEXT* context, STREAM* s) if (context->version != WF_VERSION_1_0) { DEBUG_WARN("unknown version number 0x%X", context->version); - return; + return FALSE; } DEBUG_RFX("version 0x%X", context->version); + return TRUE; } -static void rfx_process_message_codec_versions(RFX_CONTEXT* context, STREAM* s) +static BOOL rfx_process_message_codec_versions(RFX_CONTEXT* context, STREAM* s) { BYTE numCodecs; + if (stream_get_left(s) < 1) + { + DEBUG_WARN("RfxCodecVersion packet too small"); + return FALSE; + } stream_read_BYTE(s, numCodecs); /* numCodecs (1 byte), must be set to 0x01 */ if (numCodecs != 1) { DEBUG_WARN("numCodecs: %d, expected:1", numCodecs); - return; + return FALSE; + } + + if (stream_get_left(s) < 2 * numCodecs) + { + DEBUG_WARN("RfxCodecVersion packet too small for numCodecs=%d", numCodecs); + return FALSE; } /* RFX_CODEC_VERSIONT */ @@ -364,13 +381,20 @@ static void rfx_process_message_codec_versions(RFX_CONTEXT* context, STREAM* s) stream_read_BYTE(s, context->codec_version); /* version (2 bytes) */ DEBUG_RFX("id %d version 0x%X.", context->codec_id, context->codec_version); + return TRUE; } -static void rfx_process_message_channels(RFX_CONTEXT* context, STREAM* s) +static BOOL rfx_process_message_channels(RFX_CONTEXT* context, STREAM* s) { BYTE channelId; BYTE numChannels; + if (stream_get_left(s) < 1) + { + DEBUG_WARN("RfxMessageChannels packet too small"); + return FALSE; + } + stream_read_BYTE(s, numChannels); /* numChannels (1 byte), must bet set to 0x01 */ /* In RDVH sessions, numChannels will represent the number of virtual monitors @@ -379,7 +403,13 @@ static void rfx_process_message_channels(RFX_CONTEXT* context, STREAM* s) if (numChannels < 1) { DEBUG_WARN("numChannels:%d, expected:1", numChannels); - return; + return TRUE; + } + + if (stream_get_left(s) < numChannels * 5) + { + DEBUG_WARN("RfxMessageChannels packet too small for numChannels=%d", numChannels); + return FALSE; } /* RFX_CHANNELT */ @@ -392,14 +422,21 @@ static void rfx_process_message_channels(RFX_CONTEXT* context, STREAM* s) DEBUG_RFX("numChannels %d id %d, %dx%d.", numChannels, channelId, context->width, context->height); + return TRUE; } -static void rfx_process_message_context(RFX_CONTEXT* context, STREAM* s) +static BOOL rfx_process_message_context(RFX_CONTEXT* context, STREAM* s) { BYTE ctxId; UINT16 tileSize; UINT16 properties; + if (stream_get_left(s) < 5) + { + DEBUG_WARN("RfxMessageContext packet too small"); + return FALSE; + } + stream_read_BYTE(s, ctxId); /* ctxId (1 byte), must be set to 0x00 */ stream_read_UINT16(s, tileSize); /* tileSize (2 bytes), must be set to CT_TILE_64x64 (0x0040) */ stream_read_UINT16(s, properties); /* properties (2 bytes) */ @@ -430,17 +467,24 @@ static void rfx_process_message_context(RFX_CONTEXT* context, STREAM* s) DEBUG_WARN("unknown RLGR algorithm."); break; } + return TRUE; } -static void rfx_process_message_frame_begin(RFX_CONTEXT* context, RFX_MESSAGE* message, STREAM* s) +static BOOL rfx_process_message_frame_begin(RFX_CONTEXT* context, RFX_MESSAGE* message, STREAM* s) { UINT32 frameIdx; UINT16 numRegions; + if (stream_get_left(s) < 6) + { + DEBUG_WARN("RfxMessageFrameBegin packet too small"); + return FALSE; + } stream_read_UINT32(s, frameIdx); /* frameIdx (4 bytes), if codec is in video mode, must be ignored */ stream_read_UINT16(s, numRegions); /* numRegions (2 bytes) */ DEBUG_RFX("RFX_FRAME_BEGIN: frameIdx:%d numRegions:%d", frameIdx, numRegions); + return TRUE; } static void rfx_process_message_frame_end(RFX_CONTEXT* context, RFX_MESSAGE* message, STREAM* s) @@ -448,17 +492,29 @@ static void rfx_process_message_frame_end(RFX_CONTEXT* context, RFX_MESSAGE* mes DEBUG_RFX("RFX_FRAME_END"); } -static void rfx_process_message_region(RFX_CONTEXT* context, RFX_MESSAGE* message, STREAM* s) +static BOOL rfx_process_message_region(RFX_CONTEXT* context, RFX_MESSAGE* message, STREAM* s) { int i; + if (stream_get_left(s) < 3) + { + DEBUG_WARN("RfxMessageRegion packet too small"); + return FALSE; + } + stream_seek_BYTE(s); /* regionFlags (1 byte) */ stream_read_UINT16(s, message->num_rects); /* numRects (2 bytes) */ if (message->num_rects < 1) { DEBUG_WARN("no rects."); - return; + return TRUE; + } + + if (stream_get_left(s) < 8 * message->num_rects) + { + DEBUG_WARN("RfxMessageRegion packet too small for num_rects=%d", message->num_rects); + return FALSE; } if (message->rects != NULL) @@ -478,9 +534,10 @@ static void rfx_process_message_region(RFX_CONTEXT* context, RFX_MESSAGE* messag DEBUG_RFX("rect %d (%d %d %d %d).", i, message->rects[i].x, message->rects[i].y, message->rects[i].width, message->rects[i].height); } + return TRUE; } -static void rfx_process_message_tile(RFX_CONTEXT* context, RFX_TILE* tile, STREAM* s) +static BOOL rfx_process_message_tile(RFX_CONTEXT* context, RFX_TILE* tile, STREAM* s) { BYTE quantIdxY; BYTE quantIdxCb; @@ -488,6 +545,12 @@ static void rfx_process_message_tile(RFX_CONTEXT* context, RFX_TILE* tile, STREA UINT16 xIdx, yIdx; UINT16 YLen, CbLen, CrLen; + if (stream_get_left(s) < 13) + { + DEBUG_WARN("RfxMessageTile packet too small"); + return FALSE; + } + /* RFX_TILE */ stream_read_BYTE(s, quantIdxY); /* quantIdxY (1 byte) */ stream_read_BYTE(s, quantIdxCb); /* quantIdxCb (1 byte) */ @@ -504,7 +567,7 @@ static void rfx_process_message_tile(RFX_CONTEXT* context, RFX_TILE* tile, STREA tile->x = xIdx * 64; tile->y = yIdx * 64; - rfx_decode_rgb(context, s, + return rfx_decode_rgb(context, s, YLen, context->quants + (quantIdxY * 10), CbLen, context->quants + (quantIdxCb * 10), CrLen, context->quants + (quantIdxCr * 10), @@ -525,7 +588,7 @@ void CALLBACK rfx_process_message_tile_work_callback(PTP_CALLBACK_INSTANCE insta rfx_process_message_tile(param->context, param->tile, &(param->s)); } -static void rfx_process_message_tileset(RFX_CONTEXT* context, RFX_MESSAGE* message, STREAM* s) +static BOOL rfx_process_message_tileset(RFX_CONTEXT* context, RFX_MESSAGE* message, STREAM* s) { int i; int pos; @@ -538,12 +601,18 @@ static void rfx_process_message_tileset(RFX_CONTEXT* context, RFX_MESSAGE* messa PTP_WORK* work_objects = NULL; RFX_TILE_WORK_PARAM* params = NULL; + if (stream_get_left(s) < 14) + { + DEBUG_WARN("RfxMessageTileSet packet too small"); + return FALSE; + } + stream_read_UINT16(s, subtype); /* subtype (2 bytes) must be set to CBT_TILESET (0xCAC2) */ if (subtype != CBT_TILESET) { DEBUG_WARN("invalid subtype, expected CBT_TILESET."); - return; + return FALSE; } stream_seek_UINT16(s); /* idx (2 bytes), must be set to 0x0000 */ @@ -555,7 +624,7 @@ static void rfx_process_message_tileset(RFX_CONTEXT* context, RFX_MESSAGE* messa if (context->num_quants < 1) { DEBUG_WARN("no quantization value."); - return; + return TRUE; } stream_read_UINT16(s, message->num_tiles); /* numTiles (2 bytes) */ @@ -563,7 +632,7 @@ static void rfx_process_message_tileset(RFX_CONTEXT* context, RFX_MESSAGE* messa if (message->num_tiles < 1) { DEBUG_WARN("no tiles."); - return; + return TRUE; } stream_read_UINT32(s, tilesDataSize); /* tilesDataSize (4 bytes) */ @@ -575,6 +644,12 @@ static void rfx_process_message_tileset(RFX_CONTEXT* context, RFX_MESSAGE* messa quants = context->quants; /* quantVals */ + if (stream_get_left(s) < context->num_quants * 5) + { + DEBUG_WARN("RfxMessageTileSet packet too small for num_quants=%d", context->num_quants); + return FALSE; + } + for (i = 0; i < context->num_quants; i++) { /* RFX_CODEC_QUANT */ @@ -615,9 +690,21 @@ static void rfx_process_message_tileset(RFX_CONTEXT* context, RFX_MESSAGE* messa for (i = 0; i < message->num_tiles; i++) { /* RFX_TILE */ + if (stream_get_left(s) < 6) + { + DEBUG_WARN("RfxMessageTileSet packet too small to read tile %d/%d", i, message->num_tiles); + return FALSE; + } + stream_read_UINT16(s, blockType); /* blockType (2 bytes), must be set to CBT_TILE (0xCAC3) */ stream_read_UINT32(s, blockLen); /* blockLen (4 bytes) */ + if (stream_get_left(s) < blockLen - 6) + { + DEBUG_WARN("RfxMessageTileSet not enough bytes to read tile %d/%d with blocklen=%d", i, message->num_tiles, blockLen); + return FALSE; + } + pos = stream_get_pos(s) - 6 + blockLen; if (blockType != CBT_TILE) @@ -655,6 +742,7 @@ static void rfx_process_message_tileset(RFX_CONTEXT* context, RFX_MESSAGE* messa free(work_objects); free(params); } + return TRUE; } RFX_MESSAGE* rfx_process_message(RFX_CONTEXT* context, BYTE* data, UINT32 length) @@ -685,6 +773,13 @@ RFX_MESSAGE* rfx_process_message(RFX_CONTEXT* context, BYTE* data, UINT32 length break; } + if (stream_get_left(s) < blockLen - 6) + { + DEBUG_WARN("rfx_process_message: packet too small for blocklen=%d", blockLen); + break; + } + + pos = stream_get_pos(s) - 6 + blockLen; if (blockType >= WBT_CONTEXT && blockType <= WBT_EXTENSION) @@ -692,7 +787,11 @@ RFX_MESSAGE* rfx_process_message(RFX_CONTEXT* context, BYTE* data, UINT32 length /* RFX_CODEC_CHANNELT */ /* codecId (1 byte) must be set to 0x01 */ /* channelId (1 byte) must be set to 0x00 */ - stream_seek(s, 2); + if (!stream_skip(s, 2)) + { + DEBUG_WARN("rfx_process_message: unable to skip RFX_CODEC_CHANNELT"); + break; + } } switch (blockType) diff --git a/libfreerdp/codec/rfx_decode.c b/libfreerdp/codec/rfx_decode.c index 8af33be6c..2d9cf3481 100644 --- a/libfreerdp/codec/rfx_decode.c +++ b/libfreerdp/codec/rfx_decode.c @@ -142,7 +142,7 @@ void CALLBACK rfx_decode_component_work_callback(PTP_CALLBACK_INSTANCE instance, } /* stride is bytes between rows in the output buffer. */ -void rfx_decode_rgb(RFX_CONTEXT* context, STREAM* data_in, +BOOL rfx_decode_rgb(RFX_CONTEXT* context, STREAM* data_in, int y_size, const UINT32* y_quants, int cb_size, const UINT32* cb_quants, int cr_size, const UINT32* cr_quants, BYTE* rgb_buffer, int stride) @@ -202,6 +202,11 @@ void rfx_decode_rgb(RFX_CONTEXT* context, STREAM* data_in, else #endif { + if (stream_get_left(data_in) < y_size+cb_size+cr_size) + { + DEBUG_WARN("rfx_decode_rgb: packet too small for y_size+cb_size+cr_size"); + return FALSE; + } rfx_decode_component(context, y_quants, stream_get_tail(data_in), y_size, pSrcDst[0]); /* YData */ stream_seek(data_in, y_size); @@ -225,4 +230,5 @@ void rfx_decode_rgb(RFX_CONTEXT* context, STREAM* data_in, BufferPool_Return(context->priv->BufferPool, pSrcDst[0]); BufferPool_Return(context->priv->BufferPool, pSrcDst[1]); BufferPool_Return(context->priv->BufferPool, pSrcDst[2]); + return TRUE; } diff --git a/libfreerdp/codec/rfx_decode.h b/libfreerdp/codec/rfx_decode.h index e96eb66e2..fa957cd69 100644 --- a/libfreerdp/codec/rfx_decode.h +++ b/libfreerdp/codec/rfx_decode.h @@ -23,7 +23,7 @@ #include /* stride is bytes between rows in the output buffer. */ -void rfx_decode_rgb(RFX_CONTEXT* context, STREAM* data_in, +BOOL rfx_decode_rgb(RFX_CONTEXT* context, STREAM* data_in, int y_size, const UINT32 * y_quants, int cb_size, const UINT32 * cb_quants, int cr_size, const UINT32 * cr_quants, BYTE* rgb_buffer,