channels/rdpgfx: harden parsing code

diff --git a/channels/rdpgfx/client/rdpgfx_common.c b/channels/rdpgfx/client/rdpgfx_common.c
index fde1f93..f66fb77 100644 (file)
--- a/channels/rdpgfx/client/rdpgfx_common.c
+++ b/channels/rdpgfx/client/rdpgfx_common.c
@@ -87,6 +87,9 @@ const char* rdpgfx_get_codec_id_string(UINT16 codecId)
 
 int rdpgfx_read_header(wStream* s, RDPGFX_HEADER* header)
 {
+       if (Stream_GetRemainingLength(s) < 8)
+               return -1;
+
        Stream_Read_UINT16(s, header->cmdId); /* cmdId (2 bytes) */
        Stream_Read_UINT16(s, header->flags); /* flags (2 bytes) */
        Stream_Read_UINT32(s, header->pduLength); /* pduLength (4 bytes) */
@@ -105,6 +108,9 @@ int rdpgfx_write_header(wStream* s, RDPGFX_HEADER* header)
 
 int rdpgfx_read_point16(wStream* s, RDPGFX_POINT16* pt16)
 {
+       if (Stream_GetRemainingLength(s) < 4)
+               return -1;
+
        Stream_Read_UINT16(s, pt16->x); /* x (2 bytes) */
        Stream_Read_UINT16(s, pt16->y); /* y (2 bytes) */
 
@@ -121,6 +127,9 @@ int rdpgfx_write_point16(wStream* s, RDPGFX_POINT16* point16)
 
 int rdpgfx_read_rect16(wStream* s, RDPGFX_RECT16* rect16)
 {
+       if (Stream_GetRemainingLength(s) < 8)
+               return -1;
+
        Stream_Read_UINT16(s, rect16->left); /* left (2 bytes) */
        Stream_Read_UINT16(s, rect16->top); /* top (2 bytes) */
        Stream_Read_UINT16(s, rect16->right); /* right (2 bytes) */
@@ -141,6 +150,9 @@ int rdpgfx_write_rect16(wStream* s, RDPGFX_RECT16* rect16)
 
 int rdpgfx_read_color32(wStream* s, RDPGFX_COLOR32* color32)
 {
+       if (Stream_GetRemainingLength(s) < 4)
+               return -1;
+
        Stream_Read_UINT8(s, color32->B); /* B (1 byte) */
        Stream_Read_UINT8(s, color32->G); /* G (1 byte) */
        Stream_Read_UINT8(s, color32->R); /* R (1 byte) */

diff --git a/channels/rdpgfx/client/rdpgfx_main.c b/channels/rdpgfx/client/rdpgfx_main.c
index 032b51b..42761fc 100644 (file)
--- a/channels/rdpgfx/client/rdpgfx_main.c
+++ b/channels/rdpgfx/client/rdpgfx_main.c
@@ -131,6 +131,9 @@ int rdpgfx_recv_caps_confirm_pdu(RDPGFX_CHANNEL_CALLBACK* callback, wStream* s)
 
        pdu.capsSet = &capsSet;
 
+       if (Stream_GetRemainingLength(s) < 12)
+               return -1;
+
        Stream_Read_UINT32(s, capsSet.version); /* version (4 bytes) */
        Stream_Read_UINT32(s, capsDataLength); /* capsDataLength (4 bytes) */
        Stream_Read_UINT32(s, capsSet.flags); /* capsData (4 bytes) */
@@ -180,10 +183,16 @@ int rdpgfx_recv_reset_graphics_pdu(RDPGFX_CHANNEL_CALLBACK* callback, wStream* s
        RDPGFX_PLUGIN* gfx = (RDPGFX_PLUGIN*) callback->plugin;
        RdpgfxClientContext* context = (RdpgfxClientContext*) gfx->iface.pInterface;
 
+       if (Stream_GetRemainingLength(s) < 12)
+               return -1;
+
        Stream_Read_UINT32(s, pdu.width); /* width (4 bytes) */
        Stream_Read_UINT32(s, pdu.height); /* height (4 bytes) */
        Stream_Read_UINT32(s, pdu.monitorCount); /* monitorCount (4 bytes) */
 
+       if (Stream_GetRemainingLength(s) < (pdu.monitorCount * 20))
+               return -1;
+
        pdu.monitorDefArray = (MONITOR_DEF*) calloc(pdu.monitorCount, sizeof(MONITOR_DEF));
 
        if (!pdu.monitorDefArray)
@@ -200,6 +209,10 @@ int rdpgfx_recv_reset_graphics_pdu(RDPGFX_CHANNEL_CALLBACK* callback, wStream* s
        }
 
        pad = 340 - (RDPGFX_HEADER_SIZE + 12 + (pdu.monitorCount * 20));
+
+       if (Stream_GetRemainingLength(s) < pad)
+               return -1;
+
        Stream_Seek(s, pad); /* pad (total size is 340 bytes) */
 
        WLog_Print(gfx->log, WLOG_DEBUG, "RecvResetGraphicsPdu: width: %d height: %d count: %d",
@@ -221,6 +234,9 @@ int rdpgfx_recv_evict_cache_entry_pdu(RDPGFX_CHANNEL_CALLBACK* callback, wStream
        RDPGFX_PLUGIN* gfx = (RDPGFX_PLUGIN*) callback->plugin;
        RdpgfxClientContext* context = (RdpgfxClientContext*) gfx->iface.pInterface;
 
+       if (Stream_GetRemainingLength(s) < 2)
+               return -1;
+
        Stream_Read_UINT16(s, pdu.cacheSlot); /* cacheSlot (2 bytes) */
 
        WLog_Print(gfx->log, WLOG_DEBUG, "RecvEvictCacheEntryPdu: cacheSlot: %d", pdu.cacheSlot);
@@ -240,8 +256,14 @@ int rdpgfx_recv_cache_import_reply_pdu(RDPGFX_CHANNEL_CALLBACK* callback, wStrea
        RDPGFX_PLUGIN* gfx = (RDPGFX_PLUGIN*) callback->plugin;
        RdpgfxClientContext* context = (RdpgfxClientContext*) gfx->iface.pInterface;
 
+       if (Stream_GetRemainingLength(s) < 2)
+               return -1;
+
        Stream_Read_UINT16(s, pdu.importedEntriesCount); /* cacheSlot (2 bytes) */
 
+       if (Stream_GetRemainingLength(s) < (pdu.importedEntriesCount * 2))
+               return -1;
+
        pdu.cacheSlots = (UINT16*) calloc(pdu.importedEntriesCount, sizeof(UINT16));
 
        if (!pdu.cacheSlots)
@@ -260,6 +282,8 @@ int rdpgfx_recv_cache_import_reply_pdu(RDPGFX_CHANNEL_CALLBACK* callback, wStrea
                context->CacheImportReply(context, &pdu);
        }
 
+       free(pdu.cacheSlots);
+
        return 1;
 }
 
@@ -269,6 +293,9 @@ int rdpgfx_recv_create_surface_pdu(RDPGFX_CHANNEL_CALLBACK* callback, wStream* s
        RDPGFX_PLUGIN* gfx = (RDPGFX_PLUGIN*) callback->plugin;
        RdpgfxClientContext* context = (RdpgfxClientContext*) gfx->iface.pInterface;
 
+       if (Stream_GetRemainingLength(s) < 7)
+               return -1;
+
        Stream_Read_UINT16(s, pdu.surfaceId); /* surfaceId (2 bytes) */
        Stream_Read_UINT16(s, pdu.width); /* width (2 bytes) */
        Stream_Read_UINT16(s, pdu.height); /* height (2 bytes) */
@@ -291,6 +318,9 @@ int rdpgfx_recv_delete_surface_pdu(RDPGFX_CHANNEL_CALLBACK* callback, wStream* s
        RDPGFX_PLUGIN* gfx = (RDPGFX_PLUGIN*) callback->plugin;
        RdpgfxClientContext* context = (RdpgfxClientContext*) gfx->iface.pInterface;
 
+       if (Stream_GetRemainingLength(s) < 2)
+               return -1;
+
        Stream_Read_UINT16(s, pdu.surfaceId); /* surfaceId (2 bytes) */
 
        WLog_Print(gfx->log, WLOG_DEBUG, "RecvDeleteSurfacePdu: surfaceId: %d", pdu.surfaceId);
@@ -309,6 +339,9 @@ int rdpgfx_recv_start_frame_pdu(RDPGFX_CHANNEL_CALLBACK* callback, wStream* s)
        RDPGFX_PLUGIN* gfx = (RDPGFX_PLUGIN*) callback->plugin;
        RdpgfxClientContext* context = (RdpgfxClientContext*) gfx->iface.pInterface;
 
+       if (Stream_GetRemainingLength(s) < 8)
+               return -1;
+
        Stream_Read_UINT32(s, pdu.timestamp); /* timestamp (4 bytes) */
        Stream_Read_UINT32(s, pdu.frameId); /* frameId (4 bytes) */
 
@@ -332,6 +365,9 @@ int rdpgfx_recv_end_frame_pdu(RDPGFX_CHANNEL_CALLBACK* callback, wStream* s)
        RDPGFX_PLUGIN* gfx = (RDPGFX_PLUGIN*) callback->plugin;
        RdpgfxClientContext* context = (RdpgfxClientContext*) gfx->iface.pInterface;
 
+       if (Stream_GetRemainingLength(s) < 4)
+               return -1;
+
        Stream_Read_UINT32(s, pdu.frameId); /* frameId (4 bytes) */
 
        WLog_Print(gfx->log, WLOG_DEBUG, "RecvEndFramePdu: frameId: %d\n", pdu.frameId);
@@ -362,6 +398,9 @@ int rdpgfx_recv_wire_to_surface_1_pdu(RDPGFX_CHANNEL_CALLBACK* callback, wStream
        RDPGFX_PLUGIN* gfx = (RDPGFX_PLUGIN*) callback->plugin;
        RdpgfxClientContext* context = (RdpgfxClientContext*) gfx->iface.pInterface;
 
+       if (Stream_GetRemainingLength(s) < 17)
+               return -1;
+
        Stream_Read_UINT16(s, pdu.surfaceId); /* surfaceId (2 bytes) */
        Stream_Read_UINT16(s, pdu.codecId); /* codecId (2 bytes) */
        Stream_Read_UINT8(s, pdu.pixelFormat); /* pixelFormat (1 byte) */
@@ -410,6 +449,9 @@ int rdpgfx_recv_wire_to_surface_2_pdu(RDPGFX_CHANNEL_CALLBACK* callback, wStream
        RDPGFX_PLUGIN* gfx = (RDPGFX_PLUGIN*) callback->plugin;
        RdpgfxClientContext* context = (RdpgfxClientContext*) gfx->iface.pInterface;
 
+       if (Stream_GetRemainingLength(s) < 13)
+               return -1;
+
        Stream_Read_UINT16(s, pdu.surfaceId); /* surfaceId (2 bytes) */
        Stream_Read_UINT16(s, pdu.codecId); /* codecId (2 bytes) */
        Stream_Read_UINT32(s, pdu.codecContextId); /* codecContextId (4 bytes) */
@@ -451,6 +493,9 @@ int rdpgfx_recv_delete_encoding_context_pdu(RDPGFX_CHANNEL_CALLBACK* callback, w
        RDPGFX_PLUGIN* gfx = (RDPGFX_PLUGIN*) callback->plugin;
        RdpgfxClientContext* context = (RdpgfxClientContext*) gfx->iface.pInterface;
 
+       if (Stream_GetRemainingLength(s) < 6)
+               return -1;
+
        Stream_Read_UINT16(s, pdu.surfaceId); /* surfaceId (2 bytes) */
        Stream_Read_UINT32(s, pdu.codecContextId); /* codecContextId (4 bytes) */
 
@@ -473,12 +518,16 @@ int rdpgfx_recv_solid_fill_pdu(RDPGFX_CHANNEL_CALLBACK* callback, wStream* s)
        RDPGFX_PLUGIN* gfx = (RDPGFX_PLUGIN*) callback->plugin;
        RdpgfxClientContext* context = (RdpgfxClientContext*) gfx->iface.pInterface;
 
-       Stream_Read_UINT16(s, pdu.surfaceId); /* surfaceId (2 bytes) */
+       if (Stream_GetRemainingLength(s) < 8)
+               return -1;
 
+       Stream_Read_UINT16(s, pdu.surfaceId); /* surfaceId (2 bytes) */
        rdpgfx_read_color32(s, &(pdu.fillPixel)); /* fillPixel (4 bytes) */
-
        Stream_Read_UINT16(s, pdu.fillRectCount); /* fillRectCount (2 bytes) */
 
+       if (Stream_GetRemainingLength(s) < (pdu.fillRectCount * 8))
+               return -1;
+
        pdu.fillRects = (RDPGFX_RECT16*) calloc(pdu.fillRectCount, sizeof(RDPGFX_RECT16));
 
        if (!pdu.fillRects)
@@ -509,13 +558,17 @@ int rdpgfx_recv_surface_to_surface_pdu(RDPGFX_CHANNEL_CALLBACK* callback, wStrea
        RDPGFX_PLUGIN* gfx = (RDPGFX_PLUGIN*) callback->plugin;
        RdpgfxClientContext* context = (RdpgfxClientContext*) gfx->iface.pInterface;
 
+       if (Stream_GetRemainingLength(s) < 14)
+                       return -1;
+
        Stream_Read_UINT16(s, pdu.surfaceIdSrc); /* surfaceIdSrc (2 bytes) */
        Stream_Read_UINT16(s, pdu.surfaceIdDest); /* surfaceIdDest (2 bytes) */
-
        rdpgfx_read_rect16(s, &(pdu.rectSrc)); /* rectSrc (8 bytes ) */
-
        Stream_Read_UINT16(s, pdu.destPtsCount); /* destPtsCount (2 bytes) */
 
+       if (Stream_GetRemainingLength(s) < (pdu.destPtsCount * 4))
+                       return -1;
+
        pdu.destPts = (RDPGFX_POINT16*) calloc(pdu.destPtsCount, sizeof(RDPGFX_POINT16));
 
        if (!pdu.destPts)
@@ -547,6 +600,9 @@ int rdpgfx_recv_surface_to_cache_pdu(RDPGFX_CHANNEL_CALLBACK* callback, wStream*
        RDPGFX_PLUGIN* gfx = (RDPGFX_PLUGIN*) callback->plugin;
        RdpgfxClientContext* context = (RdpgfxClientContext*) gfx->iface.pInterface;
 
+       if (Stream_GetRemainingLength(s) < 20)
+               return -1;
+
        Stream_Read_UINT16(s, pdu.surfaceId); /* surfaceId (2 bytes) */
        Stream_Read_UINT64(s, pdu.cacheKey); /* cacheKey (8 bytes) */
        Stream_Read_UINT16(s, pdu.cacheSlot); /* cacheSlot (2 bytes) */
@@ -574,10 +630,16 @@ int rdpgfx_recv_cache_to_surface_pdu(RDPGFX_CHANNEL_CALLBACK* callback, wStream*
        RDPGFX_PLUGIN* gfx = (RDPGFX_PLUGIN*) callback->plugin;
        RdpgfxClientContext* context = (RdpgfxClientContext*) gfx->iface.pInterface;
 
+       if (Stream_GetRemainingLength(s) < 6)
+               return -1;
+
        Stream_Read_UINT16(s, pdu.cacheSlot); /* cacheSlot (2 bytes) */
        Stream_Read_UINT16(s, pdu.surfaceId); /* surfaceId (2 bytes) */
        Stream_Read_UINT16(s, pdu.destPtsCount); /* destPtsCount (2 bytes) */
 
+       if (Stream_GetRemainingLength(s) < (pdu.destPtsCount * 4))
+               return -1;
+
        pdu.destPts = (RDPGFX_POINT16*) calloc(pdu.destPtsCount, sizeof(RDPGFX_POINT16));
 
        if (!pdu.destPts)
@@ -608,6 +670,9 @@ int rdpgfx_recv_map_surface_to_output_pdu(RDPGFX_CHANNEL_CALLBACK* callback, wSt
        RDPGFX_PLUGIN* gfx = (RDPGFX_PLUGIN*) callback->plugin;
        RdpgfxClientContext* context = (RdpgfxClientContext*) gfx->iface.pInterface;
 
+       if (Stream_GetRemainingLength(s) < 12)
+               return -1;
+
        Stream_Read_UINT16(s, pdu.surfaceId); /* surfaceId (2 bytes) */
        Stream_Read_UINT16(s, pdu.reserved); /* reserved (2 bytes) */
        Stream_Read_UINT32(s, pdu.outputOriginX); /* outputOriginX (4 bytes) */
@@ -630,6 +695,9 @@ int rdpgfx_recv_map_surface_to_window_pdu(RDPGFX_CHANNEL_CALLBACK* callback, wSt
        RDPGFX_PLUGIN* gfx = (RDPGFX_PLUGIN*) callback->plugin;
        RdpgfxClientContext* context = (RdpgfxClientContext*) gfx->iface.pInterface;
 
+       if (Stream_GetRemainingLength(s) < 18)
+               return -1;
+
        Stream_Read_UINT16(s, pdu.surfaceId); /* surfaceId (2 bytes) */
        Stream_Read_UINT64(s, pdu.windowId); /* windowId (8 bytes) */
        Stream_Read_UINT32(s, pdu.mappedWidth); /* mappedWidth (4 bytes) */
@@ -655,7 +723,10 @@ int rdpgfx_recv_pdu(RDPGFX_CHANNEL_CALLBACK* callback, wStream* s)
 
        beg = Stream_GetPosition(s);
 
-       rdpgfx_read_header(s, &header);
+       status = rdpgfx_read_header(s, &header);
+
+       if (status < 0)
+               return -1;
 
 #if 1
        WLog_Print(gfx->log, WLOG_DEBUG, "cmdId: %s (0x%04X) flags: 0x%04X pduLength: %d",
@@ -733,10 +804,17 @@ int rdpgfx_recv_pdu(RDPGFX_CHANNEL_CALLBACK* callback, wStream* s)
                        break;
 
                default:
-                       fprintf(stderr, "Unknown GFX cmdId: 0x%04X\n", header.cmdId);
+                       status = -1;
                        break;
        }
 
+       if (status < 0)
+       {
+               fprintf(stderr, "Error while parsing GFX cmdId: %s (0x%04X)\n",
+                               rdpgfx_get_cmd_id_string(header.cmdId), header.cmdId);
+               return -1;
+       }
+
        end = Stream_GetPosition(s);
 
        if (end != (beg + header.pduLength))
@@ -772,6 +850,9 @@ static int rdpgfx_on_data_received(IWTSVirtualChannelCallback* pChannelCallback,
        while (Stream_GetPosition(s) < Stream_Length(s))
        {
                status = rdpgfx_recv_pdu(callback, s);
+
+               if (status < 0)
+                       break;
        }
 
        Stream_Free(s, TRUE);

diff --git a/libfreerdp/codec/clear.c b/libfreerdp/codec/clear.c
index 000c257..e9936c8 100644 (file)
--- a/libfreerdp/codec/clear.c
+++ b/libfreerdp/codec/clear.c
@@ -118,7 +118,7 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize,
 
                        for (y = 0; y < nHeight; y++)
                        {
-                               pDstPixel = (UINT32*) &pSrcData[y * (nWidth * 4)];
+                               pDstPixel = (UINT32*) &glyphData[y * (nWidth * 4)];
                                pSrcPixel = (UINT32*) &pDstData[((nYDst + y) * nDstStep) + (nXDst * 4)];
                                CopyMemory(pDstPixel, pSrcPixel, nWidth * 4);
                        }
@@ -460,7 +460,7 @@ int clear_decompress(CLEAR_CONTEXT* clear, BYTE* pSrcData, UINT32 SrcSize,
                for (y = 0; y < nHeight; y++)
                {
                        pSrcPixel = (UINT32*) &pDstData[((nYDst + y) * nDstStep) + (nXDst * 4)];
-                       pDstPixel = (UINT32*) &pSrcData[y * (nWidth * 4)];
+                       pDstPixel = (UINT32*) &glyphData[y * (nWidth * 4)];
                        CopyMemory(pDstPixel, pSrcPixel, nWidth * 4);
                }
        }