From b8beb55913471952f92770c90c372139d78c16c0 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Tue, 26 May 2020 07:50:55 +0200 Subject: [PATCH] Fixed OOB read in update_read_cache_bitmap_v3_order CVE-2020-11096 thanks @antonio-morales for finding this. --- libfreerdp/core/orders.c | 122 +++++++++++++++++++++++++++++++-------- 1 file changed, 98 insertions(+), 24 deletions(-) diff --git a/libfreerdp/core/orders.c b/libfreerdp/core/orders.c index 90d79f54d..744bf3cd6 100644 --- a/libfreerdp/core/orders.c +++ b/libfreerdp/core/orders.c @@ -113,20 +113,74 @@ BYTE get_primary_drawing_order_field_bytes(UINT32 orderType, BOOL* pValid) } } -static const BYTE CBR2_BPP[] = { 0, 0, 0, 8, 16, 24, 32 }; +static BYTE get_cbr2_bpp(UINT32 bpp, BOOL* pValid) +{ + if (pValid) + *pValid = TRUE; + switch (bpp) + { + case 3: + return 8; + case 4: + return 16; + case 5: + return 24; + case 6: + return 32; + default: + WLog_WARN(TAG, "Invalid bpp %" PRIu32, bpp); + if (pValid) + *pValid = FALSE; + return 0; + } +} -static const BYTE BPP_CBR2[] = { 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, - 0, 0, 0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 6, 0, 0, 0, 0, 0, 0, 0 }; - -static const BYTE CBR23_BPP[] = { 0, 0, 0, 8, 16, 24, 32 }; - -static const BYTE BPP_CBR23[] = { 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, - 0, 0, 0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 6, 0, 0, 0, 0, 0, 0, 0 }; - -static const BYTE BMF_BPP[] = { 0, 1, 0, 8, 16, 24, 32, 0 }; - -static const BYTE BPP_BMF[] = { 0, 1, 0, 0, 0, 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, - 0, 0, 0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 6, 0, 0, 0, 0, 0, 0, 0 }; +static BYTE get_bmf_bpp(UINT32 bmf, BOOL* pValid) +{ + if (pValid) + *pValid = TRUE; + switch (bmf) + { + case 1: + return 1; + case 3: + return 8; + case 4: + return 16; + case 5: + return 24; + case 6: + return 32; + default: + WLog_WARN(TAG, "Invalid bmf %" PRIu32, bmf); + if (pValid) + *pValid = FALSE; + return 0; + } +} +static BYTE get_bpp_bmf(UINT32 bpp, BOOL* pValid) +{ + if (pValid) + *pValid = TRUE; + switch (bpp) + { + case 1: + return 1; + case 8: + return 3; + case 16: + return 4; + case 24: + return 5; + case 32: + return 6; + default: + WLog_WARN(TAG, "Invalid color depth %" PRIu32, bpp); + if (pValid) + *pValid = FALSE; + return 0; + } +} static BOOL check_order_activated(wLog* log, rdpSettings* settings, const char* orderName, BOOL condition) @@ -814,9 +868,11 @@ static INLINE BOOL update_read_brush(wStream* s, rdpBrush* brush, BYTE fieldFlag if (brush->style & CACHED_BRUSH) { + BOOL rc; brush->index = brush->hatch; - brush->bpp = BMF_BPP[brush->style & 0x07]; - + brush->bpp = get_bmf_bpp(brush->style, &rc); + if (!rc) + return FALSE; if (brush->bpp == 0) brush->bpp = 1; } @@ -858,9 +914,11 @@ static INLINE BOOL update_write_brush(wStream* s, rdpBrush* brush, BYTE fieldFla if (brush->style & CACHED_BRUSH) { + BOOL rc; brush->hatch = brush->index; - brush->bpp = BMF_BPP[brush->style & 0x07]; - + brush->bpp = get_bmf_bpp(brush->style, &rc); + if (!rc) + return FALSE; if (brush->bpp == 0) brush->bpp = 1; } @@ -2016,6 +2074,7 @@ BOOL update_write_cache_bitmap_order(wStream* s, const CACHE_BITMAP_ORDER* cache static CACHE_BITMAP_V2_ORDER* update_read_cache_bitmap_v2_order(rdpUpdate* update, wStream* s, BOOL compressed, UINT16 flags) { + BOOL rc; BYTE bitsPerPixelId; CACHE_BITMAP_V2_ORDER* cache_bitmap_v2; @@ -2030,7 +2089,9 @@ static CACHE_BITMAP_V2_ORDER* update_read_cache_bitmap_v2_order(rdpUpdate* updat cache_bitmap_v2->cacheId = flags & 0x0003; cache_bitmap_v2->flags = (flags & 0xFF80) >> 7; bitsPerPixelId = (flags & 0x0078) >> 3; - cache_bitmap_v2->bitmapBpp = CBR2_BPP[bitsPerPixelId]; + cache_bitmap_v2->bitmapBpp = get_cbr2_bpp(bitsPerPixelId, &rc); + if (!rc) + goto fail; if (cache_bitmap_v2->flags & CBR2_PERSISTENT_KEY_PRESENT) { @@ -2109,13 +2170,16 @@ int update_approximate_cache_bitmap_v2_order(CACHE_BITMAP_V2_ORDER* cache_bitmap BOOL update_write_cache_bitmap_v2_order(wStream* s, CACHE_BITMAP_V2_ORDER* cache_bitmap_v2, BOOL compressed, UINT16* flags) { + BOOL rc; BYTE bitsPerPixelId; if (!Stream_EnsureRemainingCapacity( s, update_approximate_cache_bitmap_v2_order(cache_bitmap_v2, compressed, flags))) return FALSE; - bitsPerPixelId = BPP_CBR2[cache_bitmap_v2->bitmapBpp]; + bitsPerPixelId = get_bpp_bmf(cache_bitmap_v2->bitmapBpp, &rc); + if (!rc) + return FALSE; *flags = (cache_bitmap_v2->cacheId & 0x0003) | (bitsPerPixelId << 3) | ((cache_bitmap_v2->flags << 7) & 0xFF80); @@ -2177,6 +2241,7 @@ BOOL update_write_cache_bitmap_v2_order(wStream* s, CACHE_BITMAP_V2_ORDER* cache static CACHE_BITMAP_V3_ORDER* update_read_cache_bitmap_v3_order(rdpUpdate* update, wStream* s, UINT16 flags) { + BOOL rc; BYTE bitsPerPixelId; BITMAP_DATA_EX* bitmapData; UINT32 new_len; @@ -2194,7 +2259,9 @@ static CACHE_BITMAP_V3_ORDER* update_read_cache_bitmap_v3_order(rdpUpdate* updat cache_bitmap_v3->cacheId = flags & 0x00000003; cache_bitmap_v3->flags = (flags & 0x0000FF80) >> 7; bitsPerPixelId = (flags & 0x00000078) >> 3; - cache_bitmap_v3->bpp = CBR23_BPP[bitsPerPixelId]; + cache_bitmap_v3->bpp = get_cbr2_bpp(bitsPerPixelId, &rc); + if (!rc) + goto fail; if (Stream_GetRemainingLength(s) < 21) goto fail; @@ -2242,6 +2309,7 @@ int update_approximate_cache_bitmap_v3_order(CACHE_BITMAP_V3_ORDER* cache_bitmap BOOL update_write_cache_bitmap_v3_order(wStream* s, CACHE_BITMAP_V3_ORDER* cache_bitmap_v3, UINT16* flags) { + BOOL rc; BYTE bitsPerPixelId; BITMAP_DATA_EX* bitmapData; @@ -2250,7 +2318,9 @@ BOOL update_write_cache_bitmap_v3_order(wStream* s, CACHE_BITMAP_V3_ORDER* cache return FALSE; bitmapData = &cache_bitmap_v3->bitmapData; - bitsPerPixelId = BPP_CBR23[cache_bitmap_v3->bpp]; + bitsPerPixelId = get_bpp_bmf(cache_bitmap_v3->bpp, &rc); + if (!rc) + return FALSE; *flags = (cache_bitmap_v3->cacheId & 0x00000003) | ((cache_bitmap_v3->flags << 7) & 0x0000FF80) | ((bitsPerPixelId << 3) & 0x00000078); Stream_Write_UINT16(s, cache_bitmap_v3->cacheIndex); /* cacheIndex (2 bytes) */ @@ -2574,6 +2644,7 @@ static BOOL update_compress_brush(wStream* s, const BYTE* input, BYTE bpp) static CACHE_BRUSH_ORDER* update_read_cache_brush_order(rdpUpdate* update, wStream* s, UINT16 flags) { int i; + BOOL rc; BYTE iBitmapFormat; BOOL compressed = FALSE; CACHE_BRUSH_ORDER* cache_brush = calloc(1, sizeof(CACHE_BRUSH_ORDER)); @@ -2587,10 +2658,10 @@ static CACHE_BRUSH_ORDER* update_read_cache_brush_order(rdpUpdate* update, wStre Stream_Read_UINT8(s, cache_brush->index); /* cacheEntry (1 byte) */ Stream_Read_UINT8(s, iBitmapFormat); /* iBitmapFormat (1 byte) */ - if (iBitmapFormat >= ARRAYSIZE(BMF_BPP)) + cache_brush->bpp = get_bmf_bpp(iBitmapFormat, &rc); + if (!rc) goto fail; - cache_brush->bpp = BMF_BPP[iBitmapFormat]; Stream_Read_UINT8(s, cache_brush->cx); /* cx (1 byte) */ Stream_Read_UINT8(s, cache_brush->cy); /* cy (1 byte) */ Stream_Read_UINT8(s, cache_brush->style); /* style (1 byte) */ @@ -2661,13 +2732,16 @@ BOOL update_write_cache_brush_order(wStream* s, const CACHE_BRUSH_ORDER* cache_b { int i; BYTE iBitmapFormat; + BOOL rc; BOOL compressed = FALSE; if (!Stream_EnsureRemainingCapacity(s, update_approximate_cache_brush_order(cache_brush, flags))) return FALSE; - iBitmapFormat = BPP_BMF[cache_brush->bpp]; + iBitmapFormat = get_bpp_bmf(cache_brush->bpp, &rc); + if (!rc) + return FALSE; Stream_Write_UINT8(s, cache_brush->index); /* cacheEntry (1 byte) */ Stream_Write_UINT8(s, iBitmapFormat); /* iBitmapFormat (1 byte) */ Stream_Write_UINT8(s, cache_brush->cx); /* cx (1 byte) */