From 81c0e99ceb37a463ee23b5c2cc7039fe48e346f6 Mon Sep 17 00:00:00 2001 From: "rdp.effort" Date: Sun, 13 Jan 2013 23:37:50 +0100 Subject: [PATCH] Misc fixes and result checks --- libfreerdp/core/capabilities.c | 3 +-- libfreerdp/core/certificate.c | 29 +++++++++++++++-------------- libfreerdp/core/channel.c | 2 +- libfreerdp/core/fastpath.c | 19 +++++++++++++------ libfreerdp/core/fastpath.h | 2 +- libfreerdp/core/license.c | 29 ++++++++++++++++++----------- libfreerdp/core/nego.c | 2 +- libfreerdp/core/orders.c | 2 +- libfreerdp/core/peer.c | 3 ++- libfreerdp/core/rdp.c | 3 ++- libfreerdp/core/surface.c | 22 ++++++++++++++++------ libfreerdp/core/update.c | 21 +++++++++++++-------- 12 files changed, 84 insertions(+), 53 deletions(-) diff --git a/libfreerdp/core/capabilities.c b/libfreerdp/core/capabilities.c index 0a65fbc..5bfe086 100644 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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++) -- 2.7.4