Misc fixes and result checks

diff --git a/libfreerdp/core/capabilities.c b/libfreerdp/core/capabilities.c
index 0a65fbc..5bfe086 100644 (file)
--- a/libfreerdp/core/capabilities.c
+++ b/libfreerdp/core/capabilities.c
@@ -2067,9 +2067,8 @@ BOOL rdp_recv_demand_active(rdpRdp* rdp, STREAM* s)
        stream_read_UINT32(s, rdp->settings->ShareId); /* shareId (4 bytes) */
        stream_read_UINT16(s, lengthSourceDescriptor); /* lengthSourceDescriptor (2 bytes) */
        stream_read_UINT16(s, lengthCombinedCapabilities); /* lengthCombinedCapabilities (2 bytes) */
-       if (!stream_skip(s, lengthSourceDescriptor) || stream_get_left(s) < 4)
+       if (!stream_skip(s, lengthSourceDescriptor) || stream_get_left(s) < 4) /* sourceDescriptor */
                return FALSE;
-       stream_seek(s, lengthSourceDescriptor); /* sourceDescriptor */
        stream_read_UINT16(s, numberCapabilities); /* numberCapabilities (2 bytes) */
        stream_seek(s, 2); /* pad2Octets (2 bytes) */
 

diff --git a/libfreerdp/core/certificate.c b/libfreerdp/core/certificate.c
index 7984faf..150892d 100644 (file)
--- a/libfreerdp/core/certificate.c
+++ b/libfreerdp/core/certificate.c
@@ -159,33 +159,28 @@ BOOL certificate_read_x509_certificate(rdpCertBlob* cert, rdpCertInfo* info)
                goto error1;
 
        /* signature */
-       if(!ber_read_sequence_tag(s, &length) || stream_get_left(s) < length) /* AlgorithmIdentifier (SEQUENCE) */
+       if(!ber_read_sequence_tag(s, &length) || !stream_skip(s, length)) /* AlgorithmIdentifier (SEQUENCE) */
                goto error1;
-       stream_seek(s, length);
 
        /* issuer */
-       if(!ber_read_sequence_tag(s, &length) || stream_get_left(s) < length) /* Name (SEQUENCE) */
+       if(!ber_read_sequence_tag(s, &length) || !stream_skip(s, length)) /* Name (SEQUENCE) */
                goto error1;
-       stream_seek(s, length);
 
        /* validity */
-       if(!ber_read_sequence_tag(s, &length) || stream_get_left(s) < length) /* Validity (SEQUENCE) */
+       if(!ber_read_sequence_tag(s, &length) || !stream_skip(s, length)) /* Validity (SEQUENCE) */
                goto error1;
-       stream_seek(s, length);
 
        /* subject */
-       if(!ber_read_sequence_tag(s, &length) || stream_get_left(s) < length) /* Name (SEQUENCE) */
+       if(!ber_read_sequence_tag(s, &length) || !stream_skip(s, length)) /* Name (SEQUENCE) */
                goto error1;
-       stream_seek(s, length);
 
        /* subjectPublicKeyInfo */
        if(!ber_read_sequence_tag(s, &length)) /* SubjectPublicKeyInfo (SEQUENCE) */
                goto error1;
 
        /* subjectPublicKeyInfo::AlgorithmIdentifier */
-       if(!ber_read_sequence_tag(s, &length) || stream_get_left(s) < length) /* AlgorithmIdentifier (SEQUENCE) */
+       if(!ber_read_sequence_tag(s, &length) || !stream_skip(s, length)) /* AlgorithmIdentifier (SEQUENCE) */
                goto error1;
-       stream_seek(s, length);
 
        /* subjectPublicKeyInfo::subjectPublicKey */
        if(!ber_read_bit_string(s, &length, &padding)) /* BIT_STRING */
@@ -235,6 +230,7 @@ BOOL certificate_read_x509_certificate(rdpCertBlob* cert, rdpCertInfo* info)
 
 error2:
        free(info->Modulus);
+       info->Modulus = 0;
 error1:
        stream_detach(s);
        stream_free(s);
@@ -306,7 +302,7 @@ static BOOL certificate_process_server_public_key(rdpCertificate* certificate, S
        stream_read(s, certificate->cert_info.exponent, 4);
        modlen = keylen - 8;
 
-       if(stream_get_left(s) < modlen + 8)
+       if(stream_get_left(s) < modlen + 8)     // count padding
                return FALSE;
        certificate->cert_info.ModulusLength = modlen;
        certificate->cert_info.Modulus = malloc(certificate->cert_info.ModulusLength);
@@ -317,7 +313,8 @@ static BOOL certificate_process_server_public_key(rdpCertificate* certificate, S
        return TRUE;
 }
 
-static BOOL certificate_process_server_public_signature(rdpCertificate* certificate, BYTE* sigdata, int sigdatalen, STREAM* s, UINT32 siglen)
+static BOOL certificate_process_server_public_signature(rdpCertificate* certificate,
+               const BYTE* sigdata, int sigdatalen, STREAM* s, UINT32 siglen)
 {
        int i, sum;
        CryptoMd5 md5ctx;
@@ -463,6 +460,7 @@ BOOL certificate_read_server_x509_certificate_chain(rdpCertificate* certificate,
        int i;
        UINT32 certLength;
        UINT32 numCertBlobs;
+       BOOL ret;
 
        DEBUG_CERTIFICATE("Server X.509 Certificate Chain");
 
@@ -490,14 +488,17 @@ BOOL certificate_read_server_x509_certificate_chain(rdpCertificate* certificate,
                {
                        rdpCertInfo cert_info;
                        DEBUG_CERTIFICATE("License Server Certificate");
-                       certificate_read_x509_certificate(&certificate->x509_cert_chain->array[i], &cert_info);
+                       ret = certificate_read_x509_certificate(&certificate->x509_cert_chain->array[i], &cert_info);
                        DEBUG_LICENSE("modulus length:%d", (int) cert_info.ModulusLength);
                        free(cert_info.Modulus);
+                       if(!ret)
+                               return FALSE;
                }
                else if (numCertBlobs - i == 1)
                {
                        DEBUG_CERTIFICATE("Terminal Server Certificate");
-                       certificate_read_x509_certificate(&certificate->x509_cert_chain->array[i], &certificate->cert_info);
+                       if (!certificate_read_x509_certificate(&certificate->x509_cert_chain->array[i], &certificate->cert_info))
+                               return FALSE;
                        DEBUG_CERTIFICATE("modulus length:%d", (int) certificate->cert_info.ModulusLength);
                }
        }

diff --git a/libfreerdp/core/channel.c b/libfreerdp/core/channel.c
index 2740bf9..b9bccc9 100644 (file)
--- a/libfreerdp/core/channel.c
+++ b/libfreerdp/core/channel.c
@@ -97,7 +97,7 @@ BOOL freerdp_channel_process(freerdp* instance, STREAM* s, UINT16 channel_id)
        UINT32 flags;
        int chunk_length;
 
-       if(stream_get_left(s) < 4)
+       if(stream_get_left(s) < 8)
                return FALSE;
        stream_read_UINT32(s, length);
        stream_read_UINT32(s, flags);

diff --git a/libfreerdp/core/fastpath.c b/libfreerdp/core/fastpath.c
index fb1319d..7aaf093 100644 (file)
--- a/libfreerdp/core/fastpath.c
+++ b/libfreerdp/core/fastpath.c
@@ -127,10 +127,9 @@ static INLINE void fastpath_write_update_header(STREAM* s, BYTE updateCode, BYTE
        stream_write_BYTE(s, updateHeader);
 }
 
-UINT16 fastpath_read_header_rdp(rdpFastPath* fastpath, STREAM* s)
+BOOL fastpath_read_header_rdp(rdpFastPath* fastpath, STREAM* s, UINT16 *length)
 {
        BYTE header;
-       UINT16 length;
 
        stream_read_BYTE(s, header);
 
@@ -140,9 +139,11 @@ UINT16 fastpath_read_header_rdp(rdpFastPath* fastpath, STREAM* s)
                fastpath->numberEvents = (header & 0x3C) >> 2;
        }
 
-       per_read_length(s, &length);
+       if (!per_read_length(s, length))
+               return FALSE;
 
-       return length - stream_get_length(s);
+       *length = *length - stream_get_length(s);
+       return TRUE;
 }
 
 static BOOL fastpath_recv_orders(rdpFastPath* fastpath, STREAM* s)
@@ -214,7 +215,9 @@ static BOOL fastpath_recv_update(rdpFastPath* fastpath, BYTE updateCode, UINT32
 
                case FASTPATH_UPDATETYPE_BITMAP:
                case FASTPATH_UPDATETYPE_PALETTE:
-                       return fastpath_recv_update_common(fastpath, s);
+                       if(!fastpath_recv_update_common(fastpath, s))
+                               return FALSE;
+                       break;
 
                case FASTPATH_UPDATETYPE_SYNCHRONIZE:
                        if (!fastpath_recv_update_synchronize(fastpath, s))
@@ -256,7 +259,8 @@ static BOOL fastpath_recv_update(rdpFastPath* fastpath, BYTE updateCode, UINT32
                        break;
 
                case FASTPATH_UPDATETYPE_POINTER:
-                       update_read_pointer_new(s, &pointer->pointer_new);
+                       if (!update_read_pointer_new(s, &pointer->pointer_new))
+                               return FALSE;
                        IFCALL(pointer->PointerNew, context, &pointer->pointer_new);
                        break;
 
@@ -293,6 +297,8 @@ static BOOL fastpath_recv_update_data(rdpFastPath* fastpath, STREAM* s)
                compressionFlags = 0;
 
        stream_read_UINT16(s, size);
+       if(stream_get_left(s) < size)
+               return FALSE;
        next_pos = stream_get_pos(s) + size;
        comp_stream = s;
 
@@ -326,6 +332,7 @@ static BOOL fastpath_recv_update_data(rdpFastPath* fastpath, STREAM* s)
 
                stream_check_size(fastpath->updateData, size);
                stream_copy(fastpath->updateData, comp_stream, size);
+               /* TODO: add a limit on the fragmentation buffer size */
 
                if (fragmentation == FASTPATH_FRAGMENT_LAST)
                {

diff --git a/libfreerdp/core/fastpath.h b/libfreerdp/core/fastpath.h
index 1299939..7a7d847 100644 (file)
--- a/libfreerdp/core/fastpath.h
+++ b/libfreerdp/core/fastpath.h
@@ -104,7 +104,7 @@ struct rdp_fastpath
 
 UINT16 fastpath_header_length(STREAM* s);
 UINT16 fastpath_read_header(rdpFastPath* fastpath, STREAM* s);
-UINT16 fastpath_read_header_rdp(rdpFastPath* fastpath, STREAM* s);
+BOOL fastpath_read_header_rdp(rdpFastPath* fastpath, STREAM* s, UINT16 *length);
 BOOL fastpath_recv_updates(rdpFastPath* fastpath, STREAM* s);
 BOOL fastpath_recv_inputs(rdpFastPath* fastpath, STREAM* s);
 

diff --git a/libfreerdp/core/license.c b/libfreerdp/core/license.c
index 807e1e9..3f3829d 100644 (file)
--- a/libfreerdp/core/license.c
+++ b/libfreerdp/core/license.c
@@ -213,12 +213,14 @@ BOOL license_recv(rdpLicense* license, STREAM* s)
        switch (bMsgType)
        {
                case LICENSE_REQUEST:
-                       license_read_license_request_packet(license, s);
+                       if (!license_read_license_request_packet(license, s))
+                               return FALSE;
                        license_send_new_license_request_packet(license);
                        break;
 
                case PLATFORM_CHALLENGE:
-                       license_read_platform_challenge_packet(license, s);
+                       if (!license_read_platform_challenge_packet(license, s))
+                               return FALSE;
                        license_send_platform_challenge_response_packet(license);
                        break;
 
@@ -231,7 +233,8 @@ BOOL license_recv(rdpLicense* license, STREAM* s)
                        break;
 
                case ERROR_ALERT:
-                       license_read_error_alert_packet(license, s);
+                       if (!license_read_error_alert_packet(license, s))
+                               return FALSE;
                        break;
 
                default:
@@ -644,20 +647,25 @@ BOOL license_read_license_request_packet(rdpLicense* license, STREAM* s)
        stream_read(s, license->server_random, 32);
 
        /* ProductInfo */
-       license_read_product_info(s, license->product_info);
+       if (!license_read_product_info(s, license->product_info))
+               return FALSE;
 
        /* KeyExchangeList */
-       license_read_binary_blob(s, license->key_exchange_list);
+       if (!license_read_binary_blob(s, license->key_exchange_list))
+               return FALSE;
 
        /* ServerCertificate */
-       license_read_binary_blob(s, license->server_certificate);
+       if (!license_read_binary_blob(s, license->server_certificate))
+               return FALSE;
 
        /* ScopeList */
-       license_read_scope_list(s, license->scope_list);
+       if (!license_read_scope_list(s, license->scope_list))
+               return FALSE;
 
        /* Parse Server Certificate */
-       certificate_read_server_certificate(license->certificate,
-                       license->server_certificate->data, license->server_certificate->length);
+       if (!certificate_read_server_certificate(license->certificate,
+                       license->server_certificate->data, license->server_certificate->length))
+               return FALSE;
 
        license_generate_keys(license);
        license_generate_hwid(license);
@@ -685,9 +693,8 @@ BOOL license_read_platform_challenge_packet(rdpLicense* license, STREAM* s)
        license->encrypted_platform_challenge->type = BB_ENCRYPTED_DATA_BLOB;
 
        /* MACData (16 bytes) */
-       if(stream_get_left(s) < 16)
+       if(!stream_skip(s, 16))
                return FALSE;
-       stream_seek(s, 16);
 
        license_decrypt_platform_challenge(license);
        return TRUE;

diff --git a/libfreerdp/core/nego.c b/libfreerdp/core/nego.c
index 31b8811..b01fd55 100644 (file)
--- a/libfreerdp/core/nego.c
+++ b/libfreerdp/core/nego.c
@@ -465,7 +465,7 @@ BOOL nego_recv_response(rdpNego* nego)
        STREAM* s = transport_recv_stream_init(nego->transport, 1024);
 
        if (transport_read(nego->transport, s) < 0)
-               return -1;
+               return FALSE;
 
        return ((nego_recv(nego->transport, s, nego) < 0) ? FALSE : TRUE);
 }

diff --git a/libfreerdp/core/orders.c b/libfreerdp/core/orders.c
index d5e63de..35e0f3c 100644 (file)
--- a/libfreerdp/core/orders.c
+++ b/libfreerdp/core/orders.c
@@ -1441,7 +1441,7 @@ BOOL update_read_create_offscreen_bitmap_order(STREAM* s, CREATE_OFFSCREEN_BITMA
        BOOL deleteListPresent;
        OFFSCREEN_DELETE_LIST* deleteList;
 
-       if(stream_get_left(s) < 4)
+       if(stream_get_left(s) < 6)
                return FALSE;
        stream_read_UINT16(s, flags); /* flags (2 bytes) */
        create_offscreen_bitmap->id = flags & 0x7FFF;

diff --git a/libfreerdp/core/peer.c b/libfreerdp/core/peer.c
index 8157a24..e8e4fc5 100644 (file)
--- a/libfreerdp/core/peer.c
+++ b/libfreerdp/core/peer.c
@@ -224,7 +224,8 @@ static int peer_recv_fastpath_pdu(freerdp_peer* client, STREAM* s)
 
        rdp = client->context->rdp;
        fastpath = rdp->fastpath;
-       length = fastpath_read_header_rdp(fastpath, s);
+       if (!fastpath_read_header_rdp(fastpath, s, &length))
+               return -1;
 
        if ((length == 0) || (length > stream_get_left(s)))
        {

diff --git a/libfreerdp/core/rdp.c b/libfreerdp/core/rdp.c
index 8a612f3..b308b80 100644 (file)
--- a/libfreerdp/core/rdp.c
+++ b/libfreerdp/core/rdp.c
@@ -832,7 +832,8 @@ static int rdp_recv_fastpath_pdu(rdpRdp* rdp, STREAM* s)
        rdpFastPath* fastpath;
 
        fastpath = rdp->fastpath;
-       length = fastpath_read_header_rdp(fastpath, s);
+       if (!fastpath_read_header_rdp(fastpath, s, &length))
+               return -1;
 
        if ((length == 0) || (length > stream_get_left(s)))
        {

diff --git a/libfreerdp/core/surface.c b/libfreerdp/core/surface.c
index e550503..c53a351 100644 (file)
--- a/libfreerdp/core/surface.c
+++ b/libfreerdp/core/surface.c
@@ -25,11 +25,13 @@
 
 #include "surface.h"
 
-static int update_recv_surfcmd_surface_bits(rdpUpdate* update, STREAM* s)
+static BOOL update_recv_surfcmd_surface_bits(rdpUpdate* update, STREAM* s, UINT32 *length)
 {
        int pos;
        SURFACE_BITS_COMMAND* cmd = &update->surface_bits_command;
 
+       if(stream_get_left(s) < 20)
+               return FALSE;
        stream_read_UINT16(s, cmd->destLeft);
        stream_read_UINT16(s, cmd->destTop);
        stream_read_UINT16(s, cmd->destRight);
@@ -40,6 +42,8 @@ static int update_recv_surfcmd_surface_bits(rdpUpdate* update, STREAM* s)
        stream_read_UINT16(s, cmd->width);
        stream_read_UINT16(s, cmd->height);
        stream_read_UINT32(s, cmd->bitmapDataLength);
+       if(stream_get_left(s) < cmd->bitmapDataLength)
+               return FALSE;
        pos = stream_get_pos(s) + cmd->bitmapDataLength;
        cmd->bitmapData = stream_get_tail(s);
 
@@ -47,7 +51,8 @@ static int update_recv_surfcmd_surface_bits(rdpUpdate* update, STREAM* s)
 
        stream_set_pos(s, pos);
 
-       return 20 + cmd->bitmapDataLength;
+       *length = 20 + cmd->bitmapDataLength;
+       return TRUE;
 }
 
 static void update_send_frame_acknowledge(rdpRdp* rdp, UINT32 frameId)
@@ -59,10 +64,12 @@ static void update_send_frame_acknowledge(rdpRdp* rdp, UINT32 frameId)
        rdp_send_data_pdu(rdp, s, DATA_PDU_TYPE_FRAME_ACKNOWLEDGE, rdp->mcs->user_id);
 }
 
-static int update_recv_surfcmd_frame_marker(rdpUpdate* update, STREAM* s)
+static BOOL update_recv_surfcmd_frame_marker(rdpUpdate* update, STREAM* s, UINT32 *length)
 {
        SURFACE_FRAME_MARKER* marker = &update->surface_frame_marker;
 
+       if(stream_get_left(s) < 6)
+               return FALSE;
        stream_read_UINT16(s, marker->frameAction);
        stream_read_UINT32(s, marker->frameId);
 
@@ -73,7 +80,8 @@ static int update_recv_surfcmd_frame_marker(rdpUpdate* update, STREAM* s)
                update_send_frame_acknowledge(update->context->rdp, marker->frameId);
        }
 
-       return 6;
+       *length = 6;
+       return TRUE;
 }
 
 int update_recv_surfcmds(rdpUpdate* update, UINT32 size, STREAM* s)
@@ -93,11 +101,13 @@ int update_recv_surfcmds(rdpUpdate* update, UINT32 size, STREAM* s)
                {
                        case CMDTYPE_SET_SURFACE_BITS:
                        case CMDTYPE_STREAM_SURFACE_BITS:
-                               cmdLength = update_recv_surfcmd_surface_bits(update, s);
+                               if (!update_recv_surfcmd_surface_bits(update, s, &cmdLength))
+                                       return -1;
                                break;
 
                        case CMDTYPE_FRAME_MARKER:
-                               cmdLength = update_recv_surfcmd_frame_marker(update, s);
+                               if (!update_recv_surfcmd_frame_marker(update, s, &cmdLength))
+                                       return -1;
                                break;
 
                        default:

diff --git a/libfreerdp/core/update.c b/libfreerdp/core/update.c
index 0c4f7b6..7e27357 100644 (file)
--- a/libfreerdp/core/update.c
+++ b/libfreerdp/core/update.c
@@ -125,7 +125,7 @@ BOOL update_read_bitmap(rdpUpdate* update, STREAM* s, BITMAP_UPDATE* bitmap_upda
        /* rectangles */
        for (i = 0; i < (int) bitmap_update->number; i++)
        {
-               if(!update_read_bitmap_data(s, &bitmap_update->rectangles[i]))
+               if (!update_read_bitmap_data(s, &bitmap_update->rectangles[i]))
                        return FALSE;
        }
        return TRUE;
@@ -250,6 +250,8 @@ BOOL update_read_pointer_color(STREAM* s, POINTER_COLOR_UPDATE* pointer_color)
 
 BOOL update_read_pointer_new(STREAM* s, POINTER_NEW_UPDATE* pointer_new)
 {
+       if(stream_get_left(s) < 2)
+               return FALSE;
        stream_read_UINT16(s, pointer_new->xorBpp); /* xorBpp (2 bytes) */
        return update_read_pointer_color(s, &pointer_new->colorPtrAttr); /* colorPtrAttr */
 }
@@ -276,31 +278,31 @@ BOOL update_recv_pointer(rdpUpdate* update, STREAM* s)
        switch (messageType)
        {
                case PTR_MSG_TYPE_POSITION:
-                       if(update_read_pointer_position(s, &pointer->pointer_position) == FALSE)
+                       if (!update_read_pointer_position(s, &pointer->pointer_position))
                                return FALSE;
                        IFCALL(pointer->PointerPosition, context, &pointer->pointer_position);
                        break;
 
                case PTR_MSG_TYPE_SYSTEM:
-                       if(update_read_pointer_system(s, &pointer->pointer_system) == FALSE)
+                       if (!update_read_pointer_system(s, &pointer->pointer_system))
                                return FALSE;
                        IFCALL(pointer->PointerSystem, context, &pointer->pointer_system);
                        break;
 
                case PTR_MSG_TYPE_COLOR:
-                       if(update_read_pointer_color(s, &pointer->pointer_color) == FALSE)
+                       if (!update_read_pointer_color(s, &pointer->pointer_color))
                                return FALSE;
                        IFCALL(pointer->PointerColor, context, &pointer->pointer_color);
                        break;
 
                case PTR_MSG_TYPE_POINTER:
-                       if(update_read_pointer_new(s, &pointer->pointer_new) == FALSE)
+                       if (!update_read_pointer_new(s, &pointer->pointer_new))
                                return FALSE;
                        IFCALL(pointer->PointerNew, context, &pointer->pointer_new);
                        break;
 
                case PTR_MSG_TYPE_CACHED:
-                       if(update_read_pointer_cached(s, &pointer->pointer_cached) == FALSE)
+                       if (!update_read_pointer_cached(s, &pointer->pointer_cached))
                                return FALSE;
                        IFCALL(pointer->PointerCached, context, &pointer->pointer_cached);
                        break;
@@ -335,12 +337,13 @@ BOOL update_recv(rdpUpdate* update, STREAM* s)
                        break;
 
                case UPDATE_TYPE_BITMAP:
-                       update_read_bitmap(update, s, &update->bitmap_update);
+                       if (!update_read_bitmap(update, s, &update->bitmap_update))
+                               return FALSE;
                        IFCALL(update->BitmapUpdate, context, &update->bitmap_update);
                        break;
 
                case UPDATE_TYPE_PALETTE:
-                       if(update_read_palette(update, s, &update->palette_update) == FALSE)
+                       if (!update_read_palette(update, s, &update->palette_update))
                                return FALSE;
                        IFCALL(update->Palette, context, &update->palette_update);
                        break;
@@ -612,6 +615,8 @@ BOOL update_read_refresh_rect(rdpUpdate* update, STREAM* s)
        stream_read_BYTE(s, numberOfAreas);
        stream_seek(s, 3); /* pad3Octects */
 
+       if(stream_get_left(s) < numberOfAreas * 4 * 2)
+               return FALSE;
        areas = (RECTANGLE_16*) malloc(sizeof(RECTANGLE_16) * numberOfAreas);
 
        for (index = 0; index < numberOfAreas; index++)