From 6698e242281ed7de5542a43781ce9fa7793d828a Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Tue, 23 Jun 2015 21:29:21 +0200 Subject: [PATCH] Fixed leaks, NULL dereferences and broken init. --- channels/rail/client/rail_main.c | 5 +++++ client/Wayland/wlf_window.c | 4 ++-- client/X11/xf_tsmf.c | 2 ++ libfreerdp/codec/rfx.c | 5 ++--- libfreerdp/common/assistance.c | 2 +- libfreerdp/common/settings.c | 7 +++++++ libfreerdp/core/gateway/http.c | 6 +++--- libfreerdp/core/metrics.c | 2 +- libfreerdp/core/redirection.c | 2 +- libfreerdp/core/settings.c | 21 +++++++++++++++++++ libfreerdp/crypto/crypto.c | 1 + libfreerdp/gdi/dc.c | 1 - winpr/libwinpr/asn1/asn1.c | 3 ++- .../test/TestPipeCreateNamedPipeOverlapped.c | 8 +++---- winpr/libwinpr/shell/shell.c | 1 + winpr/libwinpr/utils/collections/Reference.c | 10 ++++----- winpr/libwinpr/utils/sam.c | 3 +++ 17 files changed, 61 insertions(+), 22 deletions(-) diff --git a/channels/rail/client/rail_main.c b/channels/rail/client/rail_main.c index 4280edfec..f6871be43 100644 --- a/channels/rail/client/rail_main.c +++ b/channels/rail/client/rail_main.c @@ -575,6 +575,8 @@ BOOL VCAPITYPE VirtualChannelEntry(PCHANNEL_ENTRY_POINTS pEntryPoints) CHANNEL_ENTRY_POINTS_FREERDP* pEntryPointsEx; rail = (railPlugin*) calloc(1, sizeof(railPlugin)); + if (!rail) + return FALSE; rail->channelDef.options = CHANNEL_OPTION_INITIALIZED | @@ -591,7 +593,10 @@ BOOL VCAPITYPE VirtualChannelEntry(PCHANNEL_ENTRY_POINTS pEntryPoints) { context = (RailClientContext*) calloc(1, sizeof(RailClientContext)); if (!context) + { + free(rail); return FALSE; + } context->handle = (void*) rail; context->custom = NULL; diff --git a/client/Wayland/wlf_window.c b/client/Wayland/wlf_window.c index fd73ede1a..72b57eaf3 100644 --- a/client/Wayland/wlf_window.c +++ b/client/Wayland/wlf_window.c @@ -149,9 +149,9 @@ wlfWindow* wlf_CreateDesktopWindow(wlfContext* wlfc, char* name, int width, int wlf_ResizeDesktopWindow(wlfc, window, width, height); wl_surface_damage(window->surface, 0, 0, window->width, window->height); - } - wlf_SetWindowText(wlfc, window, name); + wlf_SetWindowText(wlfc, window, name); + } return window; } diff --git a/client/X11/xf_tsmf.c b/client/X11/xf_tsmf.c index 450cc4150..a47bf132b 100644 --- a/client/X11/xf_tsmf.c +++ b/client/X11/xf_tsmf.c @@ -187,6 +187,7 @@ int xf_tsmf_xv_video_frame_event(TsmfClientContext* tsmf, TSMF_VIDEO_FRAME_EVENT else { WLog_DBG(TAG, "pixel format 0x%X not supported by hardware.", pixfmt); + free(xrects); return -1003; } @@ -213,6 +214,7 @@ int xf_tsmf_xv_video_frame_event(TsmfClientContext* tsmf, TSMF_VIDEO_FRAME_EVENT if (!XShmAttach(xfc->display, &shminfo)) { XFree(image); + free(xrects); WLog_DBG(TAG, "XShmAttach failed."); return -1004; } diff --git a/libfreerdp/codec/rfx.c b/libfreerdp/codec/rfx.c index 54e869977..c883787d7 100644 --- a/libfreerdp/codec/rfx.c +++ b/libfreerdp/codec/rfx.c @@ -966,10 +966,9 @@ static BOOL rfx_process_message_tileset(RFX_CONTEXT* context, RFX_MESSAGE* messa WaitForThreadpoolWorkCallbacks(work_objects[i], FALSE); CloseThreadpoolWork(work_objects[i]); } - - free(work_objects); - free(params); } + free(work_objects); + free(params); for (i = 0; i < message->numTiles; i++) { diff --git a/libfreerdp/common/assistance.c b/libfreerdp/common/assistance.c index 1e4efdc86..1c7a41726 100644 --- a/libfreerdp/common/assistance.c +++ b/libfreerdp/common/assistance.c @@ -184,7 +184,7 @@ int freerdp_assistance_parse_address_list(rdpAssistanceFile* file, char* list) file->MachinePorts[i] = (UINT32) atoi(q); if (!file->MachineAddresses[i]) - return -1; + goto out; q[-1] = ':'; } diff --git a/libfreerdp/common/settings.c b/libfreerdp/common/settings.c index 042a5555a..b31d3b242 100644 --- a/libfreerdp/common/settings.c +++ b/libfreerdp/common/settings.c @@ -117,7 +117,11 @@ int freerdp_addin_set_argument_value(ADDIN_ARGV* args, char* option, char* value new_argv = (char**) realloc(args->argv, sizeof(char*) * (args->argc + 1)); if (!new_argv) + { + free(str); return -1; + } + args->argv = new_argv; args->argc++; args->argv[args->argc - 1] = str; @@ -151,7 +155,10 @@ int freerdp_addin_replace_argument_value(ADDIN_ARGV* args, char* previous, char* new_argv = (char**) realloc(args->argv, sizeof(char*) * (args->argc + 1)); if (!new_argv) + { + free(str); return -1; + } args->argv = new_argv; args->argc++; args->argv[args->argc - 1] = str; diff --git a/libfreerdp/core/gateway/http.c b/libfreerdp/core/gateway/http.c index d56636873..f12423f93 100644 --- a/libfreerdp/core/gateway/http.c +++ b/libfreerdp/core/gateway/http.c @@ -615,7 +615,7 @@ HttpResponse* http_response_recv(rdpTls* tls) int position; char* line; char* buffer; - char* header; + char* header = NULL; char* payload; int bodyLength; int payloadOffset; @@ -728,8 +728,6 @@ HttpResponse* http_response_recv(rdpTls* tls) count++; } - free(header); - if (!http_response_parse_header(response)) goto out_error; @@ -779,10 +777,12 @@ HttpResponse* http_response_recv(rdpTls* tls) } } + free(header); Stream_Free(s, TRUE); return response; out_error: http_response_free(response); + free(header); out_free: Stream_Free(s, TRUE); return NULL; diff --git a/libfreerdp/core/metrics.c b/libfreerdp/core/metrics.c index cca4e16ac..1515b6083 100644 --- a/libfreerdp/core/metrics.c +++ b/libfreerdp/core/metrics.c @@ -25,7 +25,7 @@ double metrics_write_bytes(rdpMetrics* metrics, UINT32 UncompressedBytes, UINT32 CompressedBytes) { - double CompressionRatio; + double CompressionRatio = 0.0; metrics->TotalUncompressedBytes += UncompressedBytes; metrics->TotalCompressedBytes += CompressedBytes; diff --git a/libfreerdp/core/redirection.c b/libfreerdp/core/redirection.c index 00418056b..4883c13e8 100644 --- a/libfreerdp/core/redirection.c +++ b/libfreerdp/core/redirection.c @@ -207,7 +207,7 @@ int rdp_redirection_apply_settings(rdpRdp* rdp) settings->TargetNetAddresses[i] = _strdup(redirection->TargetNetAddresses[i]); if (!settings->TargetNetAddresses[i]) { - for (--i; i >= 0; --i) + for (; i > 0; --i) free(settings->TargetNetAddresses[i]); return -1; } diff --git a/libfreerdp/core/settings.c b/libfreerdp/core/settings.c index 38b5a3f45..5ad673f29 100644 --- a/libfreerdp/core/settings.c +++ b/libfreerdp/core/settings.c @@ -786,6 +786,13 @@ rdpSettings* freerdp_settings_clone(rdpSettings* settings) goto out_fail; } + if (_settings->DeviceArraySize < _settings->DeviceCount) + { + _settings->DeviceCount = 0; + _settings->DeviceArraySize = 0; + goto out_fail; + } + for (index = 0; index < _settings->DeviceCount; index++) { _settings->DeviceArray[index] = freerdp_device_clone(settings->DeviceArray[index]); @@ -803,6 +810,13 @@ rdpSettings* freerdp_settings_clone(rdpSettings* settings) goto out_fail; } + if (_settings->StaticChannelArraySize < _settings->StaticChannelCount) + { + _settings->StaticChannelArraySize = 0; + _settings->ChannelCount = 0; + goto out_fail; + } + for (index = 0; index < _settings->StaticChannelCount; index++) { _settings->StaticChannelArray[index] = freerdp_static_channel_clone(settings->StaticChannelArray[index]); @@ -820,6 +834,13 @@ rdpSettings* freerdp_settings_clone(rdpSettings* settings) goto out_fail; } + if (_settings->DynamicChannelArraySize < _settings->DynamicChannelCount) + { + _settings->DynamicChannelCount = 0; + _settings->DynamicChannelArraySize = 0; + goto out_fail; + } + for (index = 0; index < _settings->DynamicChannelCount; index++) { _settings->DynamicChannelArray[index] = freerdp_dynamic_channel_clone(settings->DynamicChannelArray[index]); diff --git a/libfreerdp/crypto/crypto.c b/libfreerdp/crypto/crypto.c index 78653eb79..26c69fdaf 100644 --- a/libfreerdp/crypto/crypto.c +++ b/libfreerdp/crypto/crypto.c @@ -483,6 +483,7 @@ char** crypto_cert_subject_alt_name(X509* xcert, int* count, int** lengths) if (!*lengths) { free(strings); + strings = NULL; goto out; } } diff --git a/libfreerdp/gdi/dc.c b/libfreerdp/gdi/dc.c index 54be0562d..f5fbb8c50 100644 --- a/libfreerdp/gdi/dc.c +++ b/libfreerdp/gdi/dc.c @@ -105,7 +105,6 @@ HGDI_DC gdi_CreateDC(UINT32 flags, int bpp) fail: gdi_DeleteDC(hDC); - free(hDC); return NULL; } diff --git a/winpr/libwinpr/asn1/asn1.c b/winpr/libwinpr/asn1/asn1.c index 4b99a831e..2b981b24b 100644 --- a/winpr/libwinpr/asn1/asn1.c +++ b/winpr/libwinpr/asn1/asn1.c @@ -117,7 +117,8 @@ LABEL_ENCODER_COMPLETE: { //if (ASN1BEREncCheck(encoder, 1)) { - *encoder->buf = 0; + if (encoder->buf) + *encoder->buf = 0; LABEL_SET_BUFFER: if (pParent) pParent[1].version = (ASN1uint32_t) encoder; diff --git a/winpr/libwinpr/pipe/test/TestPipeCreateNamedPipeOverlapped.c b/winpr/libwinpr/pipe/test/TestPipeCreateNamedPipeOverlapped.c index 6b9ffbe7f..ecb8bdf5b 100644 --- a/winpr/libwinpr/pipe/test/TestPipeCreateNamedPipeOverlapped.c +++ b/winpr/libwinpr/pipe/test/TestPipeCreateNamedPipeOverlapped.c @@ -19,10 +19,10 @@ static LPTSTR lpszPipeName = _T("\\\\.\\pipe\\winpr_test_pipe_overlapped"); static void* named_pipe_client_thread(void* arg) { DWORD status; - HANDLE hEvent; - HANDLE hNamedPipe; - BYTE* lpReadBuffer; - BYTE* lpWriteBuffer; + HANDLE hEvent = NULL; + HANDLE hNamedPipe = NULL; + BYTE* lpReadBuffer = NULL; + BYTE* lpWriteBuffer = NULL; BOOL fSuccess = FALSE; OVERLAPPED overlapped; DWORD nNumberOfBytesToRead; diff --git a/winpr/libwinpr/shell/shell.c b/winpr/libwinpr/shell/shell.c index ebe90189c..bcbf5b04b 100644 --- a/winpr/libwinpr/shell/shell.c +++ b/winpr/libwinpr/shell/shell.c @@ -78,6 +78,7 @@ BOOL GetUserProfileDirectoryA(HANDLE hToken, LPSTR lpProfileDir, LPDWORD lpcchSi if ((status != 0) || !pw) { SetLastError(ERROR_INVALID_PARAMETER); + free (buf); return FALSE; } diff --git a/winpr/libwinpr/utils/collections/Reference.c b/winpr/libwinpr/utils/collections/Reference.c index 210c375ff..0ca4e6a63 100644 --- a/winpr/libwinpr/utils/collections/Reference.c +++ b/winpr/libwinpr/utils/collections/Reference.c @@ -157,19 +157,19 @@ wReferenceTable* ReferenceTable_New(BOOL synchronized, void* context, REFERENCE_ { wReferenceTable* referenceTable; - referenceTable = (wReferenceTable*) malloc(sizeof(wReferenceTable)); + referenceTable = (wReferenceTable*) calloc(1, sizeof(wReferenceTable)); if (!referenceTable) return NULL; - referenceTable->array = (wReference*) calloc(1, sizeof(wReference) * referenceTable->size); - if (!referenceTable->array) - goto error_array; - referenceTable->context = context; referenceTable->ReferenceFree = ReferenceFree; referenceTable->size = 32; + referenceTable->array = (wReference*) calloc(referenceTable->size, sizeof(wReference)); + if (!referenceTable->array) + goto error_array; + referenceTable->synchronized = synchronized; if (synchronized && !InitializeCriticalSectionAndSpinCount(&referenceTable->lock, 4000)) goto error_critical_section; diff --git a/winpr/libwinpr/utils/sam.c b/winpr/libwinpr/utils/sam.c index 937ecf02f..08c5b1441 100644 --- a/winpr/libwinpr/utils/sam.c +++ b/winpr/libwinpr/utils/sam.c @@ -239,7 +239,10 @@ WINPR_SAM_ENTRY* SamLookupUserA(WINPR_SAM* sam, LPSTR User, UINT32 UserLength, L return NULL; if (!SamLookupStart(sam)) + { + free(entry); return NULL; + } while (sam->line != NULL) {