From bf73f4e4f17840a5bc231f6956fb51e65e08041f Mon Sep 17 00:00:00 2001 From: Bernhard Miklautz Date: Wed, 17 Jun 2015 22:08:02 +0200 Subject: [PATCH] Fix unchecked strdups * add missing checks * adapt function return values where necessary * add initial test for settings --- client/Windows/cli/wfreerdp.c | 20 +- client/Windows/wf_client.c | 22 ++ client/X11/xf_client.c | 7 +- client/X11/xf_cliprdr.c | 16 + client/X11/xf_event.c | 28 +- client/X11/xf_keyboard.c | 26 +- client/X11/xf_keyboard.h | 2 +- client/X11/xf_rail.c | 5 + client/common/client.c | 28 +- client/common/cmdline.c | 17 +- client/common/file.c | 336 +++++++++++++----- client/common/test/TestClientRdpFile.c | 18 +- client/iOS/FreeRDP/ios_freerdp.m | 12 +- client/iOS/FreeRDP/ios_freerdp_ui.m | 8 + .../codec/test/TestFreeRDPCodecProgressive.c | 5 + libfreerdp/common/addin.c | 21 ++ libfreerdp/common/assistance.c | 165 +++++---- libfreerdp/common/settings.c | 109 +++--- libfreerdp/common/test/TestCommonAssistance.c | 18 +- libfreerdp/core/certificate.c | 25 ++ libfreerdp/core/connection.c | 16 +- libfreerdp/core/gateway/ncacn_http.c | 21 +- libfreerdp/core/gateway/rdg.c | 21 +- libfreerdp/core/nla.c | 6 + libfreerdp/core/redirection.c | 25 ++ libfreerdp/core/settings.c | 149 ++++---- libfreerdp/core/tcp.c | 14 +- libfreerdp/core/test/CMakeLists.txt | 3 +- libfreerdp/core/test/TestSettings.c | 28 ++ libfreerdp/locale/keyboard_layout.c | 24 ++ libfreerdp/locale/timezone.c | 2 + server/Mac/mf_peer.c | 8 + server/Sample/sfreerdp.c | 101 ++++-- server/Windows/wf_peer.c | 16 +- server/shadow/X11/x11_shadow.c | 38 +- server/shadow/shadow_client.c | 6 +- server/shadow/shadow_server.c | 55 ++- winpr/include/winpr/wlog.h | 6 +- winpr/libwinpr/clipboard/clipboard.c | 19 +- .../clipboard/test/TestClipboardFormats.c | 5 + .../file/test/TestFileFindFirstFile.c | 5 + .../libwinpr/file/test/TestFileFindNextFile.c | 5 + winpr/libwinpr/io/device.c | 67 +++- winpr/libwinpr/io/test/TestIoDevice.c | 3 + .../library/test/TestLibraryFreeLibrary.c | 5 + .../library/test/TestLibraryGetProcAddress.c | 5 + .../library/test/TestLibraryLoadLibrary.c | 6 + winpr/libwinpr/path/shell.c | 27 +- winpr/libwinpr/path/test/TestPathShell.c | 14 + winpr/libwinpr/registry/registry_reg.c | 14 + winpr/libwinpr/smartcard/smartcard_pcsc.c | 4 +- winpr/libwinpr/sspi/sspi_winpr.c | 10 + .../sspi/test/TestAcquireCredentialsHandle.c | 12 +- .../sspi/test/TestInitializeSecurityContext.c | 13 +- winpr/libwinpr/sspicli/sspicli.c | 13 + winpr/libwinpr/utils/ini.c | 24 +- winpr/libwinpr/utils/wlog/BinaryAppender.c | 28 +- winpr/libwinpr/utils/wlog/FileAppender.c | 30 +- winpr/libwinpr/utils/wlog/Layout.c | 7 +- winpr/libwinpr/wnd/wnd.c | 19 + winpr/libwinpr/wtsapi/wtsapi_win32.c | 7 + winpr/tools/makecert/cli/main.c | 8 +- winpr/tools/makecert/makecert.c | 179 ++++------ 63 files changed, 1408 insertions(+), 548 deletions(-) create mode 100644 libfreerdp/core/test/TestSettings.c diff --git a/client/Windows/cli/wfreerdp.c b/client/Windows/cli/wfreerdp.c index b481bbac6..9fb8374ff 100644 --- a/client/Windows/cli/wfreerdp.c +++ b/client/Windows/cli/wfreerdp.c @@ -50,6 +50,7 @@ INT WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine rdpContext* context; rdpSettings* settings; RDP_CLIENT_ENTRY_POINTS clientEntryPoints; + int ret = 0; ZeroMemory(&clientEntryPoints, sizeof(RDP_CLIENT_ENTRY_POINTS)); clientEntryPoints.Size = sizeof(RDP_CLIENT_ENTRY_POINTS); @@ -66,9 +67,24 @@ INT WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine context->argc = __argc; context->argv = (char**) malloc(sizeof(char*) * __argc); + if (!context->argv) + { + ret = 1; + goto out; + } for (index = 0; index < context->argc; index++) + { context->argv[index] = _strdup(__argv[index]); + if (!context->argv[index]) + { + ret = 1; + free(context->argv); + context->argv = NULL; + goto out; + } + + } status = freerdp_client_settings_parse_command_line(settings, context->argc, context->argv, FALSE); @@ -89,8 +105,8 @@ INT WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine GetExitCodeThread(thread, &dwExitCode); freerdp_client_stop(context); - +out: freerdp_client_context_free(context); - return 0; + return ret; } diff --git a/client/Windows/wf_client.c b/client/Windows/wf_client.c index 0c424595d..f52cc31cb 100644 --- a/client/Windows/wf_client.c +++ b/client/Windows/wf_client.c @@ -534,13 +534,31 @@ BOOL wf_authenticate(freerdp* instance, char** username, char** password, char** status = CredUIParseUserNameA(UserName, User, sizeof(User), Domain, sizeof(Domain)); //WLog_ERR(TAG, "User: %s Domain: %s Password: %s", User, Domain, Password); *username = _strdup(User); + if (!(*username)) + { + WLog_ERR(TAG, "strdup failed", status); + return FALSE; + } if (strlen(Domain) > 0) *domain = _strdup(Domain); else *domain = _strdup("\0"); + if (!(*domain)) + { + free(*username); + WLog_ERR(TAG, "strdup failed", status); + return FALSE; + } + *password = _strdup(Password); + if (!(*password)) + { + free(*username); + free(*domain); + return FALSE; + } return TRUE; } @@ -873,6 +891,10 @@ int freerdp_client_load_settings_from_rdp_file(wfContext* wfc, char* filename) if (filename) { settings->ConnectionFile = _strdup(filename); + if (!settings->ConnectionFile) + { + return 3; + } // free old settings file freerdp_client_rdp_file_free(wfc->connectionRdpFile); diff --git a/client/X11/xf_client.c b/client/X11/xf_client.c index 39f25f1f7..5707a12fc 100644 --- a/client/X11/xf_client.c +++ b/client/X11/xf_client.c @@ -552,6 +552,8 @@ BOOL xf_create_window(xfContext* xfc) if (settings->WindowTitle) { windowTitle = _strdup(settings->WindowTitle); + if (!windowTitle) + return FALSE; } else if (settings->ServerPort == 3389) { @@ -999,6 +1001,8 @@ BOOL xf_pre_connect(freerdp* instance) if (login_name) { settings->Username = _strdup(login_name); + if (!settings->Username) + return FALSE; WLog_INFO(TAG, "No user name set. - Using login name: %s", settings->Username); } } @@ -1018,7 +1022,8 @@ BOOL xf_pre_connect(freerdp* instance) if (!context->cache) context->cache = cache_new(settings); - xf_keyboard_init(xfc); + if (!xf_keyboard_init(xfc)) + return FALSE; xf_detect_monitors(xfc, &maxWidth, &maxHeight); diff --git a/client/X11/xf_cliprdr.c b/client/X11/xf_cliprdr.c index 8d8c7341e..58662fcde 100644 --- a/client/X11/xf_cliprdr.c +++ b/client/X11/xf_cliprdr.c @@ -836,6 +836,16 @@ static int xf_cliprdr_server_format_list(CliprdrClientContext* context, CLIPRDR_ format = &formatList->formats[i]; clipboard->serverFormats[i].formatId = format->formatId; clipboard->serverFormats[i].formatName = _strdup(format->formatName); + if (!clipboard->serverFormats[i].formatName) + { + for (--i; i >= 0; --i) + free(clipboard->serverFormats[i].formatName); + + clipboard->numServerFormats = 0; + free(clipboard->serverFormats); + clipboard->serverFormats = NULL; + return -1; + } } clipboard->numTargets = 2; @@ -1103,6 +1113,12 @@ xfClipboard* xf_clipboard_new(xfContext* xfc) clipboard->clientFormats[n].atom = XInternAtom(xfc->display, "text/html", False); clipboard->clientFormats[n].formatId = CB_FORMAT_HTML; clipboard->clientFormats[n].formatName = _strdup("HTML Format"); + if (!clipboard->clientFormats[n].formatName) + { + ClipboardDestroy(clipboard->system); + free(clipboard); + return NULL; + } n++; clipboard->numClientFormats = n; diff --git a/client/X11/xf_event.c b/client/X11/xf_event.c index c6c700c3e..b5b2b71b5 100644 --- a/client/X11/xf_event.c +++ b/client/X11/xf_event.c @@ -87,7 +87,6 @@ const char* const X11_EVENT_STRINGS[] = BOOL xf_event_action_script_init(xfContext* xfc) { - int exitCode; char* xevent; FILE* actionScript; char buffer[1024] = { 0 }; @@ -102,18 +101,22 @@ BOOL xf_event_action_script_init(xfContext* xfc) actionScript = popen(command, "r"); - if (actionScript < 0) + if (actionScript <= 0) return FALSE; while (fgets(buffer, sizeof(buffer), actionScript)) { strtok(buffer, "\n"); xevent = _strdup(buffer); - if (ArrayList_Add(xfc->xevents, xevent) < 0) + if (!xevent || ArrayList_Add(xfc->xevents, xevent) < 0) + { + ArrayList_Free(xfc->xevents); + xfc->xevents = NULL; return FALSE; + } } - exitCode = pclose(actionScript); + pclose(actionScript); return TRUE; } @@ -127,23 +130,22 @@ void xf_event_action_script_free(xfContext* xfc) } } -int xf_event_execute_action_script(xfContext* xfc, XEvent* event) +static BOOL xf_event_execute_action_script(xfContext* xfc, XEvent* event) { int index; int count; char* name; - int exitCode; FILE* actionScript; BOOL match = FALSE; const char* xeventName; char buffer[1024] = { 0 }; char command[1024] = { 0 }; - if (!xfc->actionScript) - return 1; + if (!xfc->actionScript || !xfc->xevents) + return FALSE; if (event->type > (sizeof(X11_EVENT_STRINGS) / sizeof(const char*))) - return 1; + return FALSE; xeventName = X11_EVENT_STRINGS[event->type]; @@ -161,7 +163,7 @@ int xf_event_execute_action_script(xfContext* xfc, XEvent* event) } if (!match) - return 1; + return FALSE; sprintf_s(command, sizeof(command), "%s xevent %s %d", xfc->actionScript, xeventName, (int) xfc->window->handle); @@ -169,16 +171,16 @@ int xf_event_execute_action_script(xfContext* xfc, XEvent* event) actionScript = popen(command, "r"); if (actionScript < 0) - return -1; + return FALSE; while (fgets(buffer, sizeof(buffer), actionScript)) { strtok(buffer, "\n"); } - exitCode = pclose(actionScript); + pclose(actionScript); - return 1; + return TRUE; } void xf_event_adjust_coordinates(xfContext* xfc, int* x, int *y) diff --git a/client/X11/xf_keyboard.c b/client/X11/xf_keyboard.c index 0bd1e081d..fdb435d51 100644 --- a/client/X11/xf_keyboard.c +++ b/client/X11/xf_keyboard.c @@ -61,11 +61,11 @@ BOOL xf_keyboard_action_script_init(xfContext* xfc) xfc->actionScript = _strdup("/usr/share/freerdp/action.sh"); if (!xfc->actionScript) - return 0; + return FALSE; xfc->keyCombinations = ArrayList_New(TRUE); if (!xfc->keyCombinations) - return 0; + return FALSE; ArrayList_Object(xfc->keyCombinations)->fnObjectFree = free; @@ -77,20 +77,26 @@ BOOL xf_keyboard_action_script_init(xfContext* xfc) { free(xfc->actionScript); xfc->actionScript = NULL; - return 0; + return FALSE; } while (fgets(buffer, sizeof(buffer), keyScript) != NULL) { strtok(buffer, "\n"); keyCombination = _strdup(buffer); - if (ArrayList_Add(xfc->keyCombinations, keyCombination) < 0) - return 0; + if (!keyCombination || ArrayList_Add(xfc->keyCombinations, keyCombination) < 0) + { + ArrayList_Free(xfc->keyCombinations); + free(xfc->actionScript); + xfc->actionScript = NULL; + pclose(keyScript); + return FALSE; + } } - exitCode = pclose(keyScript); - + pclose(keyScript); return xf_event_action_script_init(xfc); + } void xf_keyboard_action_script_free(xfContext* xfc) @@ -110,7 +116,7 @@ void xf_keyboard_action_script_free(xfContext* xfc) } } -void xf_keyboard_init(xfContext* xfc) +BOOL xf_keyboard_init(xfContext* xfc) { xf_keyboard_clear(xfc); @@ -121,9 +127,11 @@ void xf_keyboard_init(xfContext* xfc) if (xfc->modifierMap) XFreeModifiermap(xfc->modifierMap); - xfc->modifierMap = XGetModifierMapping(xfc->display); + if (!(xfc->modifierMap = XGetModifierMapping(xfc->display))) + return FALSE; xf_keyboard_action_script_init(xfc); + return TRUE; } void xf_keyboard_free(xfContext* xfc) diff --git a/client/X11/xf_keyboard.h b/client/X11/xf_keyboard.h index 0261ed91a..606fdcff9 100644 --- a/client/X11/xf_keyboard.h +++ b/client/X11/xf_keyboard.h @@ -44,7 +44,7 @@ struct _XF_MODIFIER_KEYS }; typedef struct _XF_MODIFIER_KEYS XF_MODIFIER_KEYS; -void xf_keyboard_init(xfContext* xfc); +BOOL xf_keyboard_init(xfContext* xfc); void xf_keyboard_free(xfContext* xfc); void xf_keyboard_clear(xfContext* xfc); void xf_keyboard_key_press(xfContext* xfc, BYTE keycode, KeySym keysym); diff --git a/client/X11/xf_rail.c b/client/X11/xf_rail.c index fefaf63c1..7a164b5c2 100644 --- a/client/X11/xf_rail.c +++ b/client/X11/xf_rail.c @@ -312,6 +312,11 @@ static BOOL xf_rail_window_common(rdpContext* context, WINDOW_ORDER_INFO* orderI { appWindow->title = _strdup("RdpRailWindow"); } + if (!appWindow->title) + { + free(appWindow); + return FALSE; + } HashTable_Add(xfc->railWindows, (void*) (UINT_PTR) orderInfo->windowId, (void*) appWindow); diff --git a/client/common/client.c b/client/common/client.c index 53c441342..7d2e5c8d4 100644 --- a/client/common/client.c +++ b/client/common/client.c @@ -213,13 +213,20 @@ int freerdp_client_settings_parse_command_line(rdpSettings* settings, int argc, int freerdp_client_settings_parse_connection_file(rdpSettings* settings, const char* filename) { rdpFile* file; + int ret = -1; file = freerdp_client_rdp_file_new(); - freerdp_client_parse_rdp_file(file, filename); - freerdp_client_populate_settings_from_rdp_file(file, settings); - freerdp_client_rdp_file_free(file); + if (!file) + return -1; + if (!freerdp_client_parse_rdp_file(file, filename)) + goto out; + if (!freerdp_client_populate_settings_from_rdp_file(file, settings)) + goto out; - return 0; + ret = 0; +out: + freerdp_client_rdp_file_free(file); + return ret; } int freerdp_client_settings_parse_connection_file_buffer(rdpSettings* settings, const BYTE* buffer, size_t size) @@ -228,6 +235,8 @@ int freerdp_client_settings_parse_connection_file_buffer(rdpSettings* settings, int status = -1; file = freerdp_client_rdp_file_new(); + if (!file) + return -1; if (freerdp_client_parse_rdp_file_buffer(file, buffer, size) && freerdp_client_populate_settings_from_rdp_file(file, settings)) @@ -243,18 +252,23 @@ int freerdp_client_settings_parse_connection_file_buffer(rdpSettings* settings, int freerdp_client_settings_write_connection_file(const rdpSettings* settings, const char* filename, BOOL unicode) { rdpFile* file; + int ret = -1; file = freerdp_client_rdp_file_new(); + if (!file) + return -1; if (!freerdp_client_populate_rdp_file_from_settings(file, settings)) - return -1; + goto out; if (!freerdp_client_write_rdp_file(file, filename, unicode)) - return -1; + goto out; + ret = 0; +out: freerdp_client_rdp_file_free(file); - return 0; + return ret; } int freerdp_client_settings_parse_assistance_file(rdpSettings* settings, const char* filename) diff --git a/client/common/cmdline.c b/client/common/cmdline.c index ef95c5cda..d71ed14a9 100644 --- a/client/common/cmdline.c +++ b/client/common/cmdline.c @@ -963,6 +963,8 @@ int freerdp_map_keyboard_layout_name_to_id(char* name) RDP_KEYBOARD_LAYOUT* layouts; layouts = freerdp_keyboard_get_layouts(RDP_KEYBOARD_LAYOUT_TYPE_STANDARD); + if (!layouts) + return -1; for (i = 0; layouts[i].code; i++) { @@ -976,6 +978,8 @@ int freerdp_map_keyboard_layout_name_to_id(char* name) return id; layouts = freerdp_keyboard_get_layouts(RDP_KEYBOARD_LAYOUT_TYPE_VARIANT); + if (!layouts) + return -1; for (i = 0; layouts[i].code; i++) { @@ -989,6 +993,8 @@ int freerdp_map_keyboard_layout_name_to_id(char* name) return id; layouts = freerdp_keyboard_get_layouts(RDP_KEYBOARD_LAYOUT_TYPE_IME); + if (!layouts) + return -1; for (i = 0; layouts[i].code; i++) { @@ -1187,18 +1193,21 @@ int freerdp_client_settings_command_line_status_print(rdpSettings* settings, int RDP_KEYBOARD_LAYOUT* layouts; layouts = freerdp_keyboard_get_layouts(RDP_KEYBOARD_LAYOUT_TYPE_STANDARD); + //if (!layouts) /* FIXME*/ printf("\nKeyboard Layouts\n"); for (i = 0; layouts[i].code; i++) printf("0x%08X\t%s\n", (int) layouts[i].code, layouts[i].name); free(layouts); layouts = freerdp_keyboard_get_layouts(RDP_KEYBOARD_LAYOUT_TYPE_VARIANT); + //if (!layouts) /* FIXME*/ printf("\nKeyboard Layout Variants\n"); for (i = 0; layouts[i].code; i++) printf("0x%08X\t%s\n", (int) layouts[i].code, layouts[i].name); free(layouts); layouts = freerdp_keyboard_get_layouts(RDP_KEYBOARD_LAYOUT_TYPE_IME); + //if (!layouts) /* FIXME*/ printf("\nKeyboard Input Method Editors (IMEs)\n"); for (i = 0; layouts[i].code; i++) printf("0x%08X\t%s\n", (int) layouts[i].code, layouts[i].name); @@ -1469,11 +1478,15 @@ int freerdp_client_settings_parse_command_line_arguments(rdpSettings* settings, if (id == 0) { id = (unsigned long int) freerdp_map_keyboard_layout_name_to_id(arg->Value); - - if (!id) + if (id == -1) + WLog_ERR(TAG, "A problem occured while mapping the layout name to id"); + else if (id == 0) { WLog_ERR(TAG, "Could not identify keyboard layout: %s", arg->Value); + WLog_ERR(TAG, "Use /kbd-list to list available layouts"); } + if (id <= 0) + return COMMAND_LINE_STATUS_PRINT; } settings->KeyboardLayout = (UINT32) id; diff --git a/client/common/file.c b/client/common/file.c index 7ac48022d..758d66a0b 100644 --- a/client/common/file.c +++ b/client/common/file.c @@ -52,9 +52,16 @@ static WCHAR CR_LF_STR_W[] = { '\r', '\n', '\0' }; #define INVALID_INTEGER_VALUE 0xFFFFFFFF -BOOL freerdp_client_rdp_file_set_integer(rdpFile* file, const char* name, int value, int index) +/* + * Set an integer in a rdpFile + * + * @return 0 if a standard name was set, 1 for a non-standard name, -1 on error + * + */ + +static int freerdp_client_rdp_file_set_integer(rdpFile* file, const char* name, int value, int index) { - BOOL bStandard = TRUE; + int bStandard = 1; #ifdef DEBUG_CLIENT_FILE WLog_DBG(TAG, "%s:i:%d", name, value); @@ -189,11 +196,13 @@ BOOL freerdp_client_rdp_file_set_integer(rdpFile* file, const char* name, int va else if (_stricmp(name, "rdgiskdcproxy") == 0) file->RdgIsKdcProxy = value; else - bStandard = FALSE; + bStandard = 1; if (index >= 0) { file->lines[index].name = _strdup(name); + if (!file->lines[index].name) + return -1; file->lines[index].iValue = (DWORD) value; file->lines[index].flags = RDP_FILE_LINE_FLAG_FORMATTED; @@ -208,94 +217,125 @@ BOOL freerdp_client_rdp_file_set_integer(rdpFile* file, const char* name, int va return bStandard; } -void freerdp_client_parse_rdp_file_integer_unicode(rdpFile* file, WCHAR* name, WCHAR* value, int index) +static BOOL freerdp_client_parse_rdp_file_integer_unicode(rdpFile* file, WCHAR* name, WCHAR* value, int index) { int length; int ivalue; char* nameA; char* valueA; + BOOL ret = TRUE; length = (int) _wcslen(name); nameA = (char*) malloc(length + 1); + if (!nameA) + return FALSE; WideCharToMultiByte(CP_UTF8, 0, name, length, nameA, length, NULL, NULL); nameA[length] = '\0'; length = (int) _wcslen(value); valueA = (char*) malloc(length + 1); + if (!valueA) + { + free(nameA); + return FALSE; + } WideCharToMultiByte(CP_UTF8, 0, value, length, valueA, length, NULL, NULL); valueA[length] = '\0'; ivalue = atoi(valueA); - freerdp_client_rdp_file_set_integer(file, nameA, ivalue, index); + if (freerdp_client_rdp_file_set_integer(file, nameA, ivalue, index) < 0) + ret = FALSE; free(nameA); free(valueA); + return ret; } -void freerdp_client_parse_rdp_file_integer_ascii(rdpFile* file, const char* name, const char* value, int index) +static BOOL freerdp_client_parse_rdp_file_integer_ascii(rdpFile* file, const char* name, const char* value, int index) { int ivalue = atoi(value); - freerdp_client_rdp_file_set_integer(file, name, ivalue, index); + if (freerdp_client_rdp_file_set_integer(file, name, ivalue, index) < 0) + return FALSE; + return TRUE; } -BOOL freerdp_client_rdp_file_set_string(rdpFile* file, const char* name, const char* value, int index) +/** + * + * @param file rdpFile + * @param name name of the string + * @param value value of the string to set + * @param index line index of the rdpFile + * @return 0 on success, 1 if the key wasn't found (not a standard key), -1 on error + */ + +static int freerdp_client_rdp_file_set_string(rdpFile* file, const char* name, const char* value, int index) { - BOOL bStandard = TRUE; + int bStandard = 0; + LPSTR *tmp = NULL; #ifdef DEBUG_CLIENT_FILE WLog_DBG(TAG, "%s:s:%s", name, value); #endif if (_stricmp(name, "username") == 0) - file->Username = _strdup(value); + tmp = &file->Username; else if (_stricmp(name, "domain") == 0) - file->Domain = _strdup(value); + tmp = &file->Domain; else if (_stricmp(name, "full address") == 0) - file->FullAddress = _strdup(value); + tmp = &file->FullAddress; else if (_stricmp(name, "alternate full address") == 0) - file->AlternateFullAddress = _strdup(value); + tmp = &file->AlternateFullAddress; else if (_stricmp(name, "usbdevicestoredirect") == 0) - file->UsbDevicesToRedirect = _strdup(value); + tmp = &file->UsbDevicesToRedirect; else if (_stricmp(name, "loadbalanceinfo") == 0) - file->LoadBalanceInfo = _strdup(value); + tmp = &file->LoadBalanceInfo; else if (_stricmp(name, "remoteapplicationname") == 0) - file->RemoteApplicationName = _strdup(value); + tmp = &file->RemoteApplicationName; else if (_stricmp(name, "remoteapplicationicon") == 0) - file->RemoteApplicationIcon = _strdup(value); + tmp = &file->RemoteApplicationIcon; else if (_stricmp(name, "remoteapplicationprogram") == 0) - file->RemoteApplicationProgram = _strdup(value); + tmp = &file->RemoteApplicationProgram; else if (_stricmp(name, "remoteapplicationfile") == 0) - file->RemoteApplicationFile = _strdup(value); + tmp = &file->RemoteApplicationFile; else if (_stricmp(name, "remoteapplicationguid") == 0) - file->RemoteApplicationGuid = _strdup(value); + tmp = &file->RemoteApplicationGuid; else if (_stricmp(name, "remoteapplicationcmdline") == 0) - file->RemoteApplicationCmdLine = _strdup(value); + tmp = &file->RemoteApplicationCmdLine; else if (_stricmp(name, "alternate shell") == 0) - file->AlternateShell = _strdup(value); + tmp = &file->AlternateShell; else if (_stricmp(name, "shell working directory") == 0) - file->ShellWorkingDirectory = _strdup(value); + tmp = &file->ShellWorkingDirectory; else if (_stricmp(name, "gatewayhostname") == 0) - file->GatewayHostname = _strdup(value); + tmp = &file->GatewayHostname; else if (_stricmp(name, "kdcproxyname") == 0) - file->KdcProxyName = _strdup(value); + tmp = &file->KdcProxyName; else if (_stricmp(name, "drivestoredirect") == 0) - file->DrivesToRedirect = _strdup(value); + tmp = &file->DrivesToRedirect; else if (_stricmp(name, "devicestoredirect") == 0) - file->DevicesToRedirect = _strdup(value); + tmp = &file->DevicesToRedirect; else if (_stricmp(name, "winposstr") == 0) - file->WinPosStr = _strdup(value); + tmp = &file->WinPosStr; else - bStandard = FALSE; + bStandard = 1; + + if (tmp && !(*tmp = strdup(value))) + return -1; if (index >= 0) { file->lines[index].name = _strdup(name); file->lines[index].sValue = _strdup(value); + if (!file->lines[index].name || !file->lines[index].sValue) + { + free(file->lines[index].name); + free(file->lines[index].sValue); + return -1; + } file->lines[index].flags = RDP_FILE_LINE_FLAG_FORMATTED; file->lines[index].flags |= RDP_FILE_LINE_FLAG_TYPE_STRING; - if (bStandard) + if (bStandard == 0) file->lines[index].flags |= RDP_FILE_LINE_FLAG_STANDARD; file->lines[index].valueLength = 0; @@ -304,7 +344,7 @@ BOOL freerdp_client_rdp_file_set_string(rdpFile* file, const char* name, const c return bStandard; } -void freerdp_client_add_option(rdpFile* file, char* option) +static BOOL freerdp_client_add_option(rdpFile* file, char* option) { while ((file->argc + 1) > file->argSize) { @@ -314,16 +354,19 @@ void freerdp_client_add_option(rdpFile* file, char* option) new_size = file->argSize * 2; new_argv = (char**) realloc(file->argv, new_size * sizeof(char*)); if (!new_argv) - return; + return FALSE; file->argv = new_argv; file->argSize = new_size; } file->argv[file->argc] = _strdup(option); + if (!file->argv[file->argc]) + return FALSE; (file->argc)++; + return TRUE; } -int freerdp_client_parse_rdp_file_add_line(rdpFile* file, char* line, int index) +static int freerdp_client_parse_rdp_file_add_line(rdpFile* file, char* line, int index) { if (index < 0) index = file->lineCount; @@ -343,72 +386,105 @@ int freerdp_client_parse_rdp_file_add_line(rdpFile* file, char* line, int index) ZeroMemory(&(file->lines[file->lineCount]), sizeof(rdpFileLine)); file->lines[file->lineCount].text = _strdup(line); + if (!file->lines[file->lineCount].text) + return -1; + file->lines[file->lineCount].index = index; (file->lineCount)++; return index; } -void freerdp_client_parse_rdp_file_add_line_unicode(rdpFile* file, WCHAR* line, int index) +static BOOL freerdp_client_parse_rdp_file_add_line_unicode(rdpFile* file, WCHAR* line, int index) { char* lineA = NULL; + BOOL ret = TRUE; ConvertFromUnicode(CP_UTF8, 0, line, -1, &lineA, 0, NULL, NULL); - freerdp_client_parse_rdp_file_add_line(file, lineA, index); + if (!lineA) + return FALSE; + + if (freerdp_client_parse_rdp_file_add_line(file, lineA, index) == -1) + ret = FALSE; free(lineA); + return ret; } -void freerdp_client_parse_rdp_file_add_line_ascii(rdpFile* file, char* line, int index) +static BOOL freerdp_client_parse_rdp_file_add_line_ascii(rdpFile* file, char* line, int index) { - freerdp_client_parse_rdp_file_add_line(file, line, index); + if (freerdp_client_parse_rdp_file_add_line(file, line, index) == -1) + return FALSE; + return TRUE; } -void freerdp_client_parse_rdp_file_string_unicode(rdpFile* file, WCHAR* name, WCHAR* value, int index) +static BOOL freerdp_client_parse_rdp_file_string_unicode(rdpFile* file, WCHAR* name, WCHAR* value, int index) { int length; char* nameA; char* valueA; + BOOL ret = TRUE; length = (int) _wcslen(name); nameA = (char*) malloc(length + 1); + if (!nameA) + return FALSE; WideCharToMultiByte(CP_UTF8, 0, name, length, nameA, length, NULL, NULL); nameA[length] = '\0'; length = (int) _wcslen(value); valueA = (char*) malloc(length + 1); + if (!valueA) + { + free(nameA); + return FALSE; + } WideCharToMultiByte(CP_UTF8, 0, value, length, valueA, length, NULL, NULL); valueA[length] = '\0'; - freerdp_client_rdp_file_set_string(file, nameA, valueA, index); + if (freerdp_client_rdp_file_set_string(file, nameA, valueA, index) == -1) + ret = FALSE; free(nameA); free(valueA); + return ret; } -void freerdp_client_parse_rdp_file_string_ascii(rdpFile* file, char* name, char* value, int index) +static BOOL freerdp_client_parse_rdp_file_string_ascii(rdpFile* file, char* name, char* value, int index) { + BOOL ret = TRUE; char* valueA = _strdup(value); - freerdp_client_rdp_file_set_string(file, name, valueA, index); + if (!valueA) + return FALSE; + + if (freerdp_client_rdp_file_set_string(file, name, valueA, index) == -1) + ret = FALSE; + free(valueA); + return ret; } -void freerdp_client_parse_rdp_file_option_unicode(rdpFile* file, WCHAR* option, int index) +static BOOL freerdp_client_parse_rdp_file_option_unicode(rdpFile* file, WCHAR* option, int index) { char* optionA = NULL; + BOOL ret; ConvertFromUnicode(CP_UTF8, 0, option, -1, &optionA, 0, NULL, NULL); - freerdp_client_add_option(file, optionA); + if (!optionA) + return FALSE; + ret = freerdp_client_add_option(file, optionA); free(optionA); + + return ret; } -void freerdp_client_parse_rdp_file_option_ascii(rdpFile* file, char* option, int index) +static BOOL freerdp_client_parse_rdp_file_option_ascii(rdpFile* file, char* option, int index) { - freerdp_client_add_option(file, option); + return freerdp_client_add_option(file, option); } -BOOL freerdp_client_parse_rdp_file_buffer_ascii(rdpFile* file, const BYTE* buffer, size_t size) +static BOOL freerdp_client_parse_rdp_file_buffer_ascii(rdpFile* file, const BYTE* buffer, size_t size) { int index; int length; @@ -431,11 +507,14 @@ BOOL freerdp_client_parse_rdp_file_buffer_ascii(rdpFile* file, const BYTE* buffe beg = line; end = &line[length - 1]; - freerdp_client_parse_rdp_file_add_line_ascii(file, line, index); + if (!freerdp_client_parse_rdp_file_add_line_ascii(file, line, index)) + return FALSE; if (beg[0] == '/') { - freerdp_client_parse_rdp_file_option_ascii(file, line, index); + if (!freerdp_client_parse_rdp_file_option_ascii(file, line, index)) + return FALSE; + goto next_line; /* FreeRDP option */ } @@ -462,12 +541,14 @@ BOOL freerdp_client_parse_rdp_file_buffer_ascii(rdpFile* file, const BYTE* buffe if (*type == 'i') { /* integer type */ - freerdp_client_parse_rdp_file_integer_ascii(file, name, value, index); + if (!freerdp_client_parse_rdp_file_integer_ascii(file, name, value, index)) + return FALSE; } else if (*type == 's') { /* string type */ - freerdp_client_parse_rdp_file_string_ascii(file, name, value, index); + if (!freerdp_client_parse_rdp_file_string_ascii(file, name, value, index)) + return FALSE; } else if (*type == 'b') { @@ -483,7 +564,7 @@ next_line: return TRUE; } -BOOL freerdp_client_parse_rdp_file_buffer_unicode(rdpFile* file, const BYTE* buffer, size_t size) +static BOOL freerdp_client_parse_rdp_file_buffer_unicode(rdpFile* file, const BYTE* buffer, size_t size) { int index; int length; @@ -506,7 +587,8 @@ BOOL freerdp_client_parse_rdp_file_buffer_unicode(rdpFile* file, const BYTE* buf beg = line; end = &line[length - 1]; - freerdp_client_parse_rdp_file_add_line_unicode(file, line, index); + if (!freerdp_client_parse_rdp_file_add_line_unicode(file, line, index)) + return FALSE; if (beg[0] == '/') { @@ -538,12 +620,14 @@ BOOL freerdp_client_parse_rdp_file_buffer_unicode(rdpFile* file, const BYTE* buf if (*type == 'i') { /* integer type */ - freerdp_client_parse_rdp_file_integer_unicode(file, name, value, index); + if (!freerdp_client_parse_rdp_file_integer_unicode(file, name, value, index)) + return FALSE; } else if (*type == 's') { /* string type */ - freerdp_client_parse_rdp_file_string_unicode(file, name, value, index); + if (!freerdp_client_parse_rdp_file_string_unicode(file, name, value, index)) + return FALSE; } else if (*type == 'b') { @@ -594,6 +678,11 @@ BOOL freerdp_client_parse_rdp_file(rdpFile* file, const char* name) } buffer = (BYTE*) malloc(file_size + 2); + if (!buffer) + { + fclose(fp); + return FALSE; + } read_size = fread(buffer, file_size, 1, fp); if (!read_size) @@ -606,7 +695,6 @@ BOOL freerdp_client_parse_rdp_file(rdpFile* file, const char* name) if (read_size < 1) { free(buffer); - buffer = NULL; return FALSE; } @@ -623,7 +711,9 @@ BOOL freerdp_client_parse_rdp_file(rdpFile* file, const char* name) #define WRITE_ALL_SETTINGS TRUE #define SETTING_MODIFIED(_settings, _field) (WRITE_ALL_SETTINGS || _settings->SettingsModified[FreeRDP_##_field]) #define SETTING_MODIFIED_SET(_target, _settings, _field) if SETTING_MODIFIED(_settings, _field) _target = _settings->_field -#define SETTING_MODIFIED_SET_STRING(_target, _settings, _field) if SETTING_MODIFIED(_settings, _field) _target = _strdup(_settings->_field) +#define SETTING_MODIFIED_SET_STRING(_target, _settings, _field) do { if SETTING_MODIFIED(_settings, _field) _target = _strdup(_settings->_field); \ + if (!_target) return FALSE; \ + } while (0) BOOL freerdp_client_populate_rdp_file_from_settings(rdpFile* file, const rdpSettings* settings) { @@ -763,18 +853,26 @@ size_t freerdp_client_write_rdp_file_buffer(const rdpFile* file, char* buffer, s BOOL freerdp_client_populate_settings_from_rdp_file(rdpFile* file, rdpSettings* settings) { if (~((size_t) file->Domain)) - freerdp_set_param_string(settings, FreeRDP_Domain, file->Domain); + { + if (freerdp_set_param_string(settings, FreeRDP_Domain, file->Domain) != 0) + return FALSE; + } if (~((size_t) file->Username)) { char* user = NULL; char* domain = NULL; - freerdp_parse_username(file->Username, &user, &domain); - freerdp_set_param_string(settings, FreeRDP_Username, user); + if (freerdp_parse_username(file->Username, &user, &domain) != 0) + return FALSE; + if (freerdp_set_param_string(settings, FreeRDP_Username, user) != 0) + return FALSE; if (domain) - freerdp_set_param_string(settings, FreeRDP_Domain, domain); + { + if (freerdp_set_param_string(settings, FreeRDP_Domain, domain) != 0) + return FALSE; + } free(user); free(domain); @@ -785,9 +883,11 @@ BOOL freerdp_client_populate_settings_from_rdp_file(rdpFile* file, rdpSettings* int port = -1; char* host = NULL; - freerdp_parse_hostname(file->FullAddress, &host, &port); + if (freerdp_parse_hostname(file->FullAddress, &host, &port) != 0) + return FALSE; - freerdp_set_param_string(settings, FreeRDP_ServerHostname, host); + if (freerdp_set_param_string(settings, FreeRDP_ServerHostname, host) != 0) + return FALSE; if (port > 0) freerdp_set_param_uint32(settings, FreeRDP_ServerPort, (UINT32) port); @@ -813,9 +913,15 @@ BOOL freerdp_client_populate_settings_from_rdp_file(rdpFile* file, rdpSettings* if (~file->EnableCredSSPSupport) freerdp_set_param_bool(settings, FreeRDP_NlaSecurity, file->EnableCredSSPSupport); if (~((size_t) file->AlternateShell)) - freerdp_set_param_string(settings, FreeRDP_AlternateShell, file->AlternateShell); + { + if(freerdp_set_param_string(settings, FreeRDP_AlternateShell, file->AlternateShell) != 0) + return FALSE; + } if (~((size_t) file->ShellWorkingDirectory)) - freerdp_set_param_string(settings, FreeRDP_ShellWorkingDirectory, file->ShellWorkingDirectory); + { + if (freerdp_set_param_string(settings, FreeRDP_ShellWorkingDirectory, file->ShellWorkingDirectory) != 0) + return FALSE; + } if (~file->ScreenModeId) { @@ -845,6 +951,8 @@ BOOL freerdp_client_populate_settings_from_rdp_file(rdpFile* file, rdpSettings* if (~((size_t) file->LoadBalanceInfo)) { settings->LoadBalanceInfo = (BYTE*) _strdup(file->LoadBalanceInfo); + if (!settings->LoadBalanceInfo) + return FALSE; settings->LoadBalanceInfoLength = (int) strlen((char*) settings->LoadBalanceInfo); } @@ -897,9 +1005,11 @@ BOOL freerdp_client_populate_settings_from_rdp_file(rdpFile* file, rdpSettings* int port = -1; char* host = NULL; - freerdp_parse_hostname(file->GatewayHostname, &host, &port); + if (freerdp_parse_hostname(file->GatewayHostname, &host, &port) != 0) + return FALSE; - freerdp_set_param_string(settings, FreeRDP_GatewayHostname, host); + if (freerdp_set_param_string(settings, FreeRDP_GatewayHostname, host) != 0) + return FALSE; if (port > 0) freerdp_set_param_uint32(settings, FreeRDP_GatewayPort, (UINT32) port); @@ -916,15 +1026,30 @@ BOOL freerdp_client_populate_settings_from_rdp_file(rdpFile* file, rdpSettings* if (~file->RemoteApplicationMode) freerdp_set_param_bool(settings, FreeRDP_RemoteApplicationMode, file->RemoteApplicationMode); if (~((size_t) file->RemoteApplicationProgram)) - freerdp_set_param_string(settings, FreeRDP_RemoteApplicationProgram, file->RemoteApplicationProgram); + { + if (freerdp_set_param_string(settings, FreeRDP_RemoteApplicationProgram, file->RemoteApplicationProgram) != 0) + return FALSE; + } if (~((size_t) file->RemoteApplicationName)) - freerdp_set_param_string(settings, FreeRDP_RemoteApplicationName, file->RemoteApplicationName); + { + if (freerdp_set_param_string(settings, FreeRDP_RemoteApplicationName, file->RemoteApplicationName) != 0) + return FALSE; + } if (~((size_t) file->RemoteApplicationIcon)) - freerdp_set_param_string(settings, FreeRDP_RemoteApplicationIcon, file->RemoteApplicationIcon); + { + if (freerdp_set_param_string(settings, FreeRDP_RemoteApplicationIcon, file->RemoteApplicationIcon) != 0) + return FALSE; + } if (~((size_t) file->RemoteApplicationFile)) - freerdp_set_param_string(settings, FreeRDP_RemoteApplicationGuid, file->RemoteApplicationGuid); + { + if (freerdp_set_param_string(settings, FreeRDP_RemoteApplicationGuid, file->RemoteApplicationGuid) != 0) + return FALSE; + } if (~((size_t) file->RemoteApplicationCmdLine)) - freerdp_set_param_string(settings, FreeRDP_RemoteApplicationCmdLine, file->RemoteApplicationCmdLine); + { + if (freerdp_set_param_string(settings, FreeRDP_RemoteApplicationCmdLine, file->RemoteApplicationCmdLine) != 0) + return FALSE; + } if (~file->SpanMonitors) freerdp_set_param_bool(settings, FreeRDP_SpanMonitors, file->SpanMonitors); @@ -1036,14 +1161,15 @@ BOOL freerdp_client_populate_settings_from_rdp_file(rdpFile* file, rdpSettings* char* ConnectionFile = settings->ConnectionFile; settings->ConnectionFile = NULL; - freerdp_client_settings_parse_command_line(settings, file->argc, file->argv, FALSE); + if (freerdp_client_settings_parse_command_line(settings, file->argc, file->argv, FALSE) < 0) + return FALSE; settings->ConnectionFile = ConnectionFile; } return TRUE; } -rdpFileLine* freerdp_client_rdp_file_find_line_index(rdpFile* file, int index) +static rdpFileLine* freerdp_client_rdp_file_find_line_index(rdpFile* file, int index) { rdpFileLine* line; @@ -1052,7 +1178,7 @@ rdpFileLine* freerdp_client_rdp_file_find_line_index(rdpFile* file, int index) return line; } -rdpFileLine* freerdp_client_rdp_file_find_line_by_name(rdpFile* file, const char* name) +static rdpFileLine* freerdp_client_rdp_file_find_line_by_name(rdpFile* file, const char* name) { int index; BOOL bFound = FALSE; @@ -1075,6 +1201,14 @@ rdpFileLine* freerdp_client_rdp_file_find_line_by_name(rdpFile* file, const char return (bFound) ? line : NULL; } +/** + * Set a string option to a rdpFile + * @param file rdpFile + * @param name name of the option + * @param value value of the option + * @return 0 on success + */ + int freerdp_client_rdp_file_set_string_option(rdpFile* file, const char* name, const char* value) { int index; @@ -1082,17 +1216,20 @@ int freerdp_client_rdp_file_set_string_option(rdpFile* file, const char* name, c char* text; rdpFileLine* line; - line = freerdp_client_rdp_file_find_line_by_name(file, name); - length = _scprintf("%s:s:%s", name, value); text = (char*) malloc(length + 1); + if (!text) + return -1; sprintf_s(text, length + 1, "%s:s:%s", name, value ? value : ""); text[length] = '\0'; + line = freerdp_client_rdp_file_find_line_by_name(file, name); if (line) { free(line->sValue); line->sValue = _strdup(value); + if (!line->sValue) + goto out_fail; free(line->text); line->text = text; @@ -1100,14 +1237,24 @@ int freerdp_client_rdp_file_set_string_option(rdpFile* file, const char* name, c else { index = freerdp_client_parse_rdp_file_add_line(file, text, -1); - line = freerdp_client_rdp_file_find_line_index(file, index); + if (index == -1) + goto out_fail; - freerdp_client_rdp_file_set_string(file, name, value, index); + if (!(line = freerdp_client_rdp_file_find_line_index(file, index))) + goto out_fail; + + if (freerdp_client_rdp_file_set_string(file, name, value, index) == -1) + goto out_fail; free(text); } return 0; + +out_fail: + free(text); + return -1; + } const char* freerdp_client_rdp_file_get_string_option(rdpFile* file, const char* name) @@ -1149,9 +1296,18 @@ int freerdp_client_rdp_file_set_integer_option(rdpFile* file, const char* name, else { index = freerdp_client_parse_rdp_file_add_line(file, text, -1); + if (index < 0) + { + free(text); + return -1; + } line = freerdp_client_rdp_file_find_line_index(file, index); - freerdp_client_rdp_file_set_integer(file, (char*) name, value, index); + if (freerdp_client_rdp_file_set_integer(file, (char*) name, value, index) < 0) + { + free(text); + return -1; + } free(text); } @@ -1174,7 +1330,7 @@ int freerdp_client_rdp_file_get_integer_option(rdpFile* file, const char* name) return line->iValue; } -void freerdp_client_file_string_check_free(LPSTR str) +static void freerdp_client_file_string_check_free(LPSTR str) { if (~((size_t) str)) free(str); @@ -1193,12 +1349,30 @@ rdpFile* freerdp_client_rdp_file_new() file->lineCount = 0; file->lineSize = 32; file->lines = (rdpFileLine*) malloc(file->lineSize * sizeof(rdpFileLine)); + if (!file->lines) + { + free(file); + return NULL; + } + file->argc = 0; file->argSize = 32; file->argv = (char**) malloc(file->argSize * sizeof(char*)); + if (!file->argv) + { + free(file->lines); + free(file); + return NULL; + } - freerdp_client_add_option(file, "freerdp"); + if (!freerdp_client_add_option(file, "freerdp")) + { + free(file->argv); + free(file->lines); + free(file); + return NULL; + } } return file; diff --git a/client/common/test/TestClientRdpFile.c b/client/common/test/TestClientRdpFile.c index 8d5c8fb9a..ce15e6942 100644 --- a/client/common/test/TestClientRdpFile.c +++ b/client/common/test/TestClientRdpFile.c @@ -271,6 +271,11 @@ int TestClientRdpFile(int argc, char* argv[]) /* Unicode */ file = freerdp_client_rdp_file_new(); + if (!file) + { + printf("rdp_file_new failed\n"); + return -1; + } freerdp_client_parse_rdp_file_buffer(file, testRdpFileUTF16, sizeof(testRdpFileUTF16)); if (file->UseMultiMon != 0) @@ -331,7 +336,12 @@ int TestClientRdpFile(int argc, char* argv[]) } iValue = freerdp_client_rdp_file_get_integer_option(file, "vendor integer"); - freerdp_client_rdp_file_set_integer_option(file, "vendor integer", 456); + if (freerdp_client_rdp_file_set_integer_option(file, "vendor integer", 456) == -1) + { + printf("failed to set integer: vendor integer"); + return -1; + } + iValue = freerdp_client_rdp_file_get_integer_option(file, "vendor integer"); sValue = (char*) freerdp_client_rdp_file_get_string_option(file, "vendor string"); @@ -339,7 +349,11 @@ int TestClientRdpFile(int argc, char* argv[]) sValue = (char*) freerdp_client_rdp_file_get_string_option(file, "vendor string"); freerdp_client_rdp_file_set_string_option(file, "fruits", "banana,oranges"); - freerdp_client_rdp_file_set_integer_option(file, "numbers", 123456789); + if (freerdp_client_rdp_file_set_integer_option(file, "numbers", 123456789) == -1) + { + printf("failed to set integer: numbers"); + return -1; + } for (index = 0; index < file->lineCount; index++) { diff --git a/client/iOS/FreeRDP/ios_freerdp.m b/client/iOS/FreeRDP/ios_freerdp.m index d309fa63d..f4ef32aab 100644 --- a/client/iOS/FreeRDP/ios_freerdp.m +++ b/client/iOS/FreeRDP/ios_freerdp.m @@ -11,6 +11,7 @@ #import #import #import +#import #import "ios_freerdp.h" #import "ios_freerdp_ui.h" @@ -19,7 +20,6 @@ #import "RDPSession.h" #import "Utils.h" - #pragma mark Connection helpers static BOOL ios_pre_connect(freerdp* instance) @@ -279,6 +279,8 @@ void ios_context_free(freerdp* instance, rdpContext* context) freerdp* ios_freerdp_new() { freerdp* inst = freerdp_new(); + if (!inst) + return NULL; inst->PreConnect = ios_pre_connect; inst->PostConnect = ios_post_connect; @@ -297,6 +299,14 @@ freerdp* ios_freerdp_new() free(inst->settings->ConfigPath); inst->settings->HomePath = strdup([home_path UTF8String]); inst->settings->ConfigPath = strdup([[home_path stringByAppendingPathComponent:@".freerdp"] UTF8String]); + if (!inst->settings->HomePath || !inst->settings->ConfigPath) + { + free(inst->settings->HomePath); + free(inst->settings->ConfigPath); + freerdp_context_free(inst); + freerdp_free(inst); + return NULL; + } return inst; } diff --git a/client/iOS/FreeRDP/ios_freerdp_ui.m b/client/iOS/FreeRDP/ios_freerdp_ui.m index 2ecdbbf8b..191b25ec2 100644 --- a/client/iOS/FreeRDP/ios_freerdp_ui.m +++ b/client/iOS/FreeRDP/ios_freerdp_ui.m @@ -51,6 +51,14 @@ BOOL ios_ui_authenticate(freerdp * instance, char** username, char** password, c *username = strdup([[params objectForKey:@"username"] UTF8String]); *password = strdup([[params objectForKey:@"password"] UTF8String]); *domain = strdup([[params objectForKey:@"domain"] UTF8String]); + + if (!(*username) || !(*password) || !(*domain)) + { + free(*username); + free(*password); + free(*domain); + return FALSE; + } return TRUE; } diff --git a/libfreerdp/codec/test/TestFreeRDPCodecProgressive.c b/libfreerdp/codec/test/TestFreeRDPCodecProgressive.c index fe84a418c..43c644476 100644 --- a/libfreerdp/codec/test/TestFreeRDPCodecProgressive.c +++ b/libfreerdp/codec/test/TestFreeRDPCodecProgressive.c @@ -1010,6 +1010,11 @@ int TestFreeRDPCodecProgressive(int argc, char* argv[]) char* ms_sample_path; ms_sample_path = _strdup("/tmp/EGFX_PROGRESSIVE_MS_SAMPLE"); + if (!ms_sample_path) + { + printf("Memory allocation failed\n"); + return -1; + } if (PathFileExistsA(ms_sample_path)) return test_progressive_ms_sample(ms_sample_path); diff --git a/libfreerdp/common/addin.c b/libfreerdp/common/addin.c index f5689c385..b23b2c7b9 100644 --- a/libfreerdp/common/addin.c +++ b/libfreerdp/common/addin.c @@ -69,6 +69,8 @@ LPSTR freerdp_get_dynamic_addin_install_path() cchPath = cchInstallPrefix + cchAddinPath + 2; pszPath = (LPSTR) malloc(cchPath + 1); + if (!pszPath) + return NULL; CopyMemory(pszPath, pszInstallPrefix, cchInstallPrefix); pszPath[cchInstallPrefix] = '\0'; @@ -106,20 +108,39 @@ void* freerdp_load_dynamic_addin(LPCSTR pszFileName, LPCSTR pszPath, LPCSTR pszE } pszAddinInstallPath = freerdp_get_dynamic_addin_install_path(); + if (!pszAddinInstallPath) + return NULL; cchAddinInstallPath = strlen(pszAddinInstallPath); cchFilePath = cchAddinInstallPath + cchFileName + 32; pszFilePath = (LPSTR) malloc(cchFilePath + 1); + if (!pszFilePath) + { + free(pszAddinInstallPath); + return NULL; + } if (bHasExt) { pszAddinFile = _strdup(pszFileName); + if (!pszAddinFile) + { + free(pszAddinInstallPath); + free(pszFilePath); + return NULL; + } cchAddinFile = strlen(pszAddinFile); } else { cchAddinFile = cchFileName + cchExt + 2 + sizeof(CMAKE_SHARED_LIBRARY_PREFIX); pszAddinFile = (LPSTR) malloc(cchAddinFile + 1); + if (!pszAddinFile) + { + free(pszAddinInstallPath); + free(pszFilePath); + return NULL; + } sprintf_s(pszAddinFile, cchAddinFile, CMAKE_SHARED_LIBRARY_PREFIX"%s%s", pszFileName, pszExt); cchAddinFile = strlen(pszAddinFile); } diff --git a/libfreerdp/common/assistance.c b/libfreerdp/common/assistance.c index 717edc9fc..308eab747 100644 --- a/libfreerdp/common/assistance.c +++ b/libfreerdp/common/assistance.c @@ -124,6 +124,7 @@ int freerdp_assistance_parse_address_list(rdpAssistanceFile* file, char* list) int count; int length; char** tokens; + int ret = -1; count = 1; str = _strdup(list); @@ -140,9 +141,11 @@ int freerdp_assistance_parse_address_list(rdpAssistanceFile* file, char* list) } tokens = (char**) malloc(sizeof(char*) * count); - if (!tokens) + { + free(str); return -1; + } count = 0; tokens[count++] = str; @@ -161,12 +164,7 @@ int freerdp_assistance_parse_address_list(rdpAssistanceFile* file, char* list) file->MachinePorts = (UINT32*) calloc(count, sizeof(UINT32)); if (!file->MachineAddresses || !file->MachinePorts) - { - free(file->MachineAddresses); - free(file->MachinePorts); - free(tokens); - return -1; - } + goto out; for (i = 0; i < count; i++) { @@ -177,10 +175,7 @@ int freerdp_assistance_parse_address_list(rdpAssistanceFile* file, char* list) q = strchr(p, ':'); if (!q) - { - free(tokens); - return -1; - } + goto out; q[0] = '\0'; q++; @@ -209,26 +204,29 @@ int freerdp_assistance_parse_address_list(rdpAssistanceFile* file, char* list) q = strchr(p, ':'); if (!q) - { - free(tokens); - return -1; - } + goto out; q[0] = '\0'; q++; + if (file->MachineAddress) + free(file->MachineAddress); file->MachineAddress = _strdup(p); + if (!file->MachineAddress) + goto out; file->MachinePort = (UINT32) atoi(q); if (!file->MachineAddress) - return -1; + goto out; break; } + ret = 1; +out: free(tokens); - - return 1; + free(str); + return ret; } int freerdp_assistance_parse_connection_string1(rdpAssistanceFile* file) @@ -238,6 +236,7 @@ int freerdp_assistance_parse_connection_string1(rdpAssistanceFile* file) int count; int length; char* tokens[8]; + int ret; /** * ,,,, @@ -298,10 +297,13 @@ int freerdp_assistance_parse_connection_string1(rdpAssistanceFile* file) if (!file->RASpecificParams) return -1; - freerdp_assistance_parse_address_list(file, tokens[2]); + ret = freerdp_assistance_parse_address_list(file, tokens[2]); free(str); + if (ret != 1) + return -1; + return 1; } @@ -323,96 +325,92 @@ int freerdp_assistance_parse_connection_string1(rdpAssistanceFile* file) int freerdp_assistance_parse_connection_string2(rdpAssistanceFile* file) { - char* p; - char* q; - int port; char* str; - size_t length; + char* tag; + char* end; + char* p; + int ret = -1; + + + str = file->ConnectionString2; + + if (!strstr(str, "")) + return -1; + + if (!strstr(str, "")) + return -1; str = _strdup(file->ConnectionString2); - if (!str) return -1; - p = strstr(str, ""); + if (!(tag = strstr(str, ") */ + end = strstr(tag, "/>"); + if (!end) + goto out_fail; - p = strstr(str, ""); - - if (!p) - return -1; - - /* Auth String Node () */ - - p = strstr(str, "RASpecificParams); file->RASpecificParams = (char*) malloc(length + 1); - if (!file->RASpecificParams) - return -1; + goto out_fail; CopyMemory(file->RASpecificParams, p, length); file->RASpecificParams[length] = '\0'; - - p += length; } - if (p) - p = strstr(p, "ID=\""); - else - p = _strdup("ID=\""); - + p = strstr(tag, "ID=\""); if (p) { + char *q; + size_t length; p += sizeof("ID=\"") - 1; q = strchr(p, '"'); if (!q) - return -1; + goto out_fail; length = q - p; free(file->RASessionId); file->RASessionId = (char*) malloc(length + 1); - if (!file->RASessionId) - return -1; + goto out_fail; CopyMemory(file->RASessionId, p, length); file->RASessionId[length] = '\0'; - - p += length; } + *end = '/'; - if (p) - p = strstr(p, "MachineAddress) + free(file->MachineAddress); file->MachineAddress = _strdup(p); + if (!file->MachineAddress) + goto out_fail; file->MachinePort = (UINT32) port; break; } @@ -449,9 +451,11 @@ int freerdp_assistance_parse_connection_string2(rdpAssistanceFile* file) p = strstr(q, "RASessionId) + if (!file->RASessionId || !file->MachineAddress) return -1; - freerdp_set_param_string(settings, FreeRDP_RemoteAssistanceSessionId, file->RASessionId); - - if (file->RCTicket) - freerdp_set_param_string(settings, FreeRDP_RemoteAssistanceRCTicket, file->RCTicket); - - if (file->PassStub) - freerdp_set_param_string(settings, FreeRDP_RemoteAssistancePassStub, file->PassStub); - - if (!file->MachineAddress) + if (freerdp_set_param_string(settings, FreeRDP_RemoteAssistanceSessionId, file->RASessionId) != 0) + return -1; + + if (file->RCTicket && (freerdp_set_param_string(settings, FreeRDP_RemoteAssistanceRCTicket, file->RCTicket) != 0)) + return -1; + + if (file->PassStub && (freerdp_set_param_string(settings, FreeRDP_RemoteAssistancePassStub, file->PassStub) != 0)) + return -1; + + if (freerdp_set_param_string(settings, FreeRDP_ServerHostname, file->MachineAddress) != 0) return -1; - freerdp_set_param_string(settings, FreeRDP_ServerHostname, file->MachineAddress); freerdp_set_param_uint32(settings, FreeRDP_ServerPort, file->MachinePort); freerdp_target_net_addresses_free(settings); @@ -1173,16 +1177,7 @@ int freerdp_client_populate_settings_from_assistance_file(rdpAssistanceFile* fil rdpAssistanceFile* freerdp_assistance_file_new() { - rdpAssistanceFile* file; - - file = (rdpAssistanceFile*) calloc(1, sizeof(rdpAssistanceFile)); - - if (file) - { - - } - - return file; + return (rdpAssistanceFile*) calloc(1, sizeof(rdpAssistanceFile)); } void freerdp_assistance_file_free(rdpAssistanceFile* file) diff --git a/libfreerdp/common/settings.c b/libfreerdp/common/settings.c index f095d5dd2..924ceeb23 100644 --- a/libfreerdp/common/settings.c +++ b/libfreerdp/common/settings.c @@ -51,7 +51,8 @@ int freerdp_addin_set_argument(ADDIN_ARGV* args, char* argument) return -1; args->argv = new_argv; args->argc++; - args->argv[args->argc - 1] = _strdup(argument); + if (!(args->argv[args->argc - 1] = _strdup(argument))) + return -1; return 0; } @@ -66,7 +67,8 @@ int freerdp_addin_replace_argument(ADDIN_ARGV* args, char* previous, char* argum if (strcmp(args->argv[i], previous) == 0) { free(args->argv[i]); - args->argv[i] = _strdup(argument); + if (!(args->argv[i] = _strdup(argument))) + return -1; return 1; } @@ -77,7 +79,8 @@ int freerdp_addin_replace_argument(ADDIN_ARGV* args, char* previous, char* argum return -1; args->argv = new_argv; args->argc++; - args->argv[args->argc - 1] = _strdup(argument); + if (!(args->argv[args->argc - 1] = _strdup(argument))) + return -1; return 0; } @@ -381,6 +384,9 @@ void freerdp_device_collection_free(rdpSettings* settings) { device = (RDPDR_DEVICE*) settings->DeviceArray[index]; + if (!device) + continue; + free(device->Name); if (settings->DeviceArray[index]->Type == RDPDR_DTYP_FILESYSTEM) @@ -491,6 +497,9 @@ void freerdp_static_channel_collection_free(rdpSettings* settings) for (i = 0; i < settings->StaticChannelCount; i++) { + if (!settings->StaticChannelArray[i]) + continue; + for (j = 0; j < settings->StaticChannelArray[i]->argc; j++) free(settings->StaticChannelArray[i]->argv[j]); @@ -588,6 +597,9 @@ void freerdp_dynamic_channel_collection_free(rdpSettings* settings) for (i = 0; i < settings->DynamicChannelCount; i++) { + if (!settings->DynamicChannelArray[i]) + continue; + for (j = 0; j < settings->DynamicChannelArray[i]->argc; j++) free(settings->DynamicChannelArray[i]->argv[j]); @@ -2432,231 +2444,232 @@ char* freerdp_get_param_string(rdpSettings* settings, int id) int freerdp_set_param_string(rdpSettings* settings, int id, const char* param) { +#define CHECKED_STRDUP(name) if (param && !(settings->name = _strdup(param))) return -1 switch (id) { case FreeRDP_ServerHostname: free(settings->ServerHostname); - settings->ServerHostname = _strdup(param); + CHECKED_STRDUP(ServerHostname); break; case FreeRDP_Username: free(settings->Username); - settings->Username = _strdup(param); + CHECKED_STRDUP(Username); break; case FreeRDP_Password: free(settings->Password); - settings->Password = _strdup(param); + CHECKED_STRDUP(Password); break; case FreeRDP_Domain: free(settings->Domain); - settings->Domain = _strdup(param); + CHECKED_STRDUP(Domain); break; case FreeRDP_PasswordHash: free(settings->PasswordHash); - settings->PasswordHash = _strdup(param); + CHECKED_STRDUP(PasswordHash); break; case FreeRDP_ClientHostname: free(settings->ClientHostname); - settings->ClientHostname = _strdup(param); + CHECKED_STRDUP(ClientHostname); break; case FreeRDP_ClientProductId: free(settings->ClientProductId); - settings->ClientProductId = _strdup(param); + CHECKED_STRDUP(ClientProductId); break; case FreeRDP_AlternateShell: free(settings->AlternateShell); - settings->AlternateShell = _strdup(param); + CHECKED_STRDUP(AlternateShell); break; case FreeRDP_ShellWorkingDirectory: free(settings->ShellWorkingDirectory); - settings->ShellWorkingDirectory = _strdup(param); + CHECKED_STRDUP(ShellWorkingDirectory); break; case FreeRDP_ClientAddress: free(settings->ClientAddress); - settings->ClientAddress = _strdup(param); + CHECKED_STRDUP(ClientAddress); break; case FreeRDP_ClientDir: free(settings->ClientDir); - settings->ClientDir = _strdup(param); + CHECKED_STRDUP(ClientDir); break; case FreeRDP_DynamicDSTTimeZoneKeyName: free(settings->DynamicDSTTimeZoneKeyName); - settings->DynamicDSTTimeZoneKeyName = _strdup(param); + CHECKED_STRDUP(DynamicDSTTimeZoneKeyName); break; case FreeRDP_RemoteAssistanceSessionId: free(settings->RemoteAssistanceSessionId); - settings->RemoteAssistanceSessionId = _strdup(param); + CHECKED_STRDUP(RemoteAssistanceSessionId); break; case FreeRDP_RemoteAssistancePassStub: free(settings->RemoteAssistancePassStub); - settings->RemoteAssistancePassStub = _strdup(param); + CHECKED_STRDUP(RemoteAssistancePassStub); break; case FreeRDP_RemoteAssistancePassword: free(settings->RemoteAssistancePassword); - settings->RemoteAssistancePassword = _strdup(param); + CHECKED_STRDUP(RemoteAssistancePassword); break; case FreeRDP_RemoteAssistanceRCTicket: free(settings->RemoteAssistanceRCTicket); - settings->RemoteAssistanceRCTicket = _strdup(param); + CHECKED_STRDUP(RemoteAssistanceRCTicket); break; case FreeRDP_AuthenticationServiceClass: free(settings->AuthenticationServiceClass); - settings->AuthenticationServiceClass = _strdup(param); + CHECKED_STRDUP(AuthenticationServiceClass); break; case FreeRDP_PreconnectionBlob: free(settings->PreconnectionBlob); - settings->PreconnectionBlob = _strdup(param); + CHECKED_STRDUP(PreconnectionBlob); break; case FreeRDP_KerberosKdc: free(settings->KerberosKdc); - settings->KerberosKdc = _strdup(param); + CHECKED_STRDUP(KerberosKdc); break; case FreeRDP_KerberosRealm: free(settings->KerberosRealm); - settings->KerberosRealm = _strdup(param); + CHECKED_STRDUP(KerberosRealm); break; case FreeRDP_CertificateName: free(settings->CertificateName); - settings->CertificateName = _strdup(param); + CHECKED_STRDUP(CertificateName); break; case FreeRDP_CertificateFile: free(settings->CertificateFile); - settings->CertificateFile = _strdup(param); + CHECKED_STRDUP(CertificateFile); break; case FreeRDP_PrivateKeyFile: free(settings->PrivateKeyFile); - settings->PrivateKeyFile = _strdup(param); + CHECKED_STRDUP(PrivateKeyFile); break; case FreeRDP_RdpKeyFile: free(settings->RdpKeyFile); - settings->RdpKeyFile = _strdup(param); + CHECKED_STRDUP(RdpKeyFile); break; case FreeRDP_WindowTitle: free(settings->WindowTitle); - settings->WindowTitle = _strdup(param); + CHECKED_STRDUP(WindowTitle); break; case FreeRDP_ComputerName: free(settings->ComputerName); - settings->ComputerName = _strdup(param); + CHECKED_STRDUP(ComputerName); break; case FreeRDP_ConnectionFile: free(settings->ConnectionFile); - settings->ConnectionFile = _strdup(param); + CHECKED_STRDUP(ConnectionFile); break; case FreeRDP_AssistanceFile: free(settings->AssistanceFile); - settings->AssistanceFile = _strdup(param); + CHECKED_STRDUP(AssistanceFile); break; case FreeRDP_HomePath: free(settings->HomePath); - settings->HomePath = _strdup(param); + CHECKED_STRDUP(HomePath); break; case FreeRDP_ConfigPath: free(settings->ConfigPath); - settings->ConfigPath = _strdup(param); + CHECKED_STRDUP(ConfigPath); break; case FreeRDP_CurrentPath: free(settings->CurrentPath); - settings->CurrentPath = _strdup(param); + CHECKED_STRDUP(CurrentPath); break; case FreeRDP_DumpRemoteFxFile: free(settings->DumpRemoteFxFile); - settings->DumpRemoteFxFile = _strdup(param); + CHECKED_STRDUP(DumpRemoteFxFile); break; case FreeRDP_PlayRemoteFxFile: free(settings->PlayRemoteFxFile); - settings->PlayRemoteFxFile = _strdup(param); + CHECKED_STRDUP(PlayRemoteFxFile); break; case FreeRDP_GatewayHostname: free(settings->GatewayHostname); - settings->GatewayHostname = _strdup(param); + CHECKED_STRDUP(GatewayHostname); break; case FreeRDP_GatewayUsername: free(settings->GatewayUsername); - settings->GatewayUsername = _strdup(param); + CHECKED_STRDUP(GatewayUsername); break; case FreeRDP_GatewayPassword: free(settings->GatewayPassword); - settings->GatewayPassword = _strdup(param); + CHECKED_STRDUP(GatewayPassword); break; case FreeRDP_GatewayDomain: free(settings->GatewayDomain); - settings->GatewayDomain = _strdup(param); + CHECKED_STRDUP(GatewayDomain); break; case FreeRDP_RemoteApplicationName: free(settings->RemoteApplicationName); - settings->RemoteApplicationName = _strdup(param); + CHECKED_STRDUP(RemoteApplicationName); break; case FreeRDP_RemoteApplicationIcon: free(settings->RemoteApplicationIcon); - settings->RemoteApplicationIcon = _strdup(param); + CHECKED_STRDUP(RemoteApplicationIcon); break; case FreeRDP_RemoteApplicationProgram: free(settings->RemoteApplicationProgram); - settings->RemoteApplicationProgram = _strdup(param); + CHECKED_STRDUP(RemoteApplicationProgram); break; case FreeRDP_RemoteApplicationFile: free(settings->RemoteApplicationFile); - settings->RemoteApplicationFile = _strdup(param); + CHECKED_STRDUP(RemoteApplicationFile); break; case FreeRDP_RemoteApplicationGuid: free(settings->RemoteApplicationGuid); - settings->RemoteApplicationGuid = _strdup(param); + CHECKED_STRDUP(RemoteApplicationGuid); break; case FreeRDP_RemoteApplicationCmdLine: free(settings->RemoteApplicationCmdLine); - settings->RemoteApplicationCmdLine = _strdup(param); + CHECKED_STRDUP(RemoteApplicationCmdLine); break; case FreeRDP_ImeFileName: free(settings->ImeFileName); - settings->ImeFileName = _strdup(param); + CHECKED_STRDUP(ImeFileName); break; case FreeRDP_DrivesToRedirect: free(settings->DrivesToRedirect); - settings->DrivesToRedirect = _strdup(param); + CHECKED_STRDUP(DrivesToRedirect); break; default: diff --git a/libfreerdp/common/test/TestCommonAssistance.c b/libfreerdp/common/test/TestCommonAssistance.c index 411152508..9bc304281 100644 --- a/libfreerdp/common/test/TestCommonAssistance.c +++ b/libfreerdp/common/test/TestCommonAssistance.c @@ -85,6 +85,9 @@ int test_msrsc_incident_file_type1() file = freerdp_assistance_file_new(); + if (!file) + return -1; + status = freerdp_assistance_parse_file_buffer(file, TEST_MSRC_INCIDENT_FILE_TYPE1, sizeof(TEST_MSRC_INCIDENT_FILE_TYPE1)); @@ -136,6 +139,9 @@ int test_msrsc_incident_file_type2() file = freerdp_assistance_file_new(); + if (!file) + return -1; + status = freerdp_assistance_parse_file_buffer(file, TEST_MSRC_INCIDENT_FILE_TYPE2, sizeof(TEST_MSRC_INCIDENT_FILE_TYPE2)); @@ -174,9 +180,17 @@ int test_msrsc_incident_file_type2() int TestCommonAssistance(int argc, char* argv[]) { - test_msrsc_incident_file_type1(); + if (test_msrsc_incident_file_type1() != 0) + { + printf("test_msrsc_incident_file_type1 failed\n"); + return -1; + } - test_msrsc_incident_file_type2(); + if (test_msrsc_incident_file_type2() != 0) + { + printf("test_msrsc_incident_file_type1 failed\n"); + return -1; + } return 0; } diff --git a/libfreerdp/core/certificate.c b/libfreerdp/core/certificate.c index da5b8991c..7fca4ea07 100644 --- a/libfreerdp/core/certificate.c +++ b/libfreerdp/core/certificate.c @@ -791,6 +791,8 @@ rdpCertificate* certificate_clone(rdpCertificate* certificate) if (certificate->cert_info.ModulusLength) { _certificate->cert_info.Modulus = (BYTE*) malloc(certificate->cert_info.ModulusLength); + if (!_certificate->cert_info.Modulus) + goto out_fail; CopyMemory(_certificate->cert_info.Modulus, certificate->cert_info.Modulus, certificate->cert_info.ModulusLength); _certificate->cert_info.ModulusLength = certificate->cert_info.ModulusLength; } @@ -798,11 +800,15 @@ rdpCertificate* certificate_clone(rdpCertificate* certificate) if (certificate->x509_cert_chain) { _certificate->x509_cert_chain = (rdpX509CertChain*) malloc(sizeof(rdpX509CertChain)); + if (!_certificate->x509_cert_chain) + goto out_fail; CopyMemory(_certificate->x509_cert_chain, certificate->x509_cert_chain, sizeof(rdpX509CertChain)); if (certificate->x509_cert_chain->count) { _certificate->x509_cert_chain->array = (rdpCertBlob*) calloc(certificate->x509_cert_chain->count, sizeof(rdpCertBlob)); + if (!_certificate->x509_cert_chain->array) + goto out_fail; for (index = 0; index < certificate->x509_cert_chain->count; index++) { @@ -811,6 +817,15 @@ rdpCertificate* certificate_clone(rdpCertificate* certificate) if (certificate->x509_cert_chain->array[index].length) { _certificate->x509_cert_chain->array[index].data = (BYTE*) malloc(certificate->x509_cert_chain->array[index].length); + if (!_certificate->x509_cert_chain->array[index].data) + { + for (--index; index >= 0; --index) + { + if (certificate->x509_cert_chain->array[index].length) + free(_certificate->x509_cert_chain->array[index].data); + } + goto out_fail; + } CopyMemory(_certificate->x509_cert_chain->array[index].data, certificate->x509_cert_chain->array[index].data, _certificate->x509_cert_chain->array[index].length); } @@ -819,6 +834,16 @@ rdpCertificate* certificate_clone(rdpCertificate* certificate) } return _certificate; + +out_fail: + if (certificate->x509_cert_chain->count) + { + free(_certificate->x509_cert_chain->array); + } + free(_certificate->x509_cert_chain); + free(_certificate->cert_info.Modulus); + free(_certificate); + return NULL; } /** diff --git a/libfreerdp/core/connection.c b/libfreerdp/core/connection.c index a7b9eb429..0fe31317d 100644 --- a/libfreerdp/core/connection.c +++ b/libfreerdp/core/connection.c @@ -334,11 +334,13 @@ BOOL rdp_client_redirect(rdpRdp* rdp) rdpSettings* settings = rdp->settings; rdp_client_disconnect(rdp); - rdp_redirection_apply_settings(rdp); + if (rdp_redirection_apply_settings(rdp) != 0) + return FALSE; if (settings->RedirectionFlags & LB_LOAD_BALANCE_INFO) { - nego_set_routing_token(rdp->nego, settings->LoadBalanceInfo, settings->LoadBalanceInfoLength); + if (!nego_set_routing_token(rdp->nego, settings->LoadBalanceInfo, settings->LoadBalanceInfoLength)) + return FALSE; } else { @@ -346,16 +348,22 @@ BOOL rdp_client_redirect(rdpRdp* rdp) { free(settings->ServerHostname); settings->ServerHostname = _strdup(settings->RedirectionTargetFQDN); + if (!settings->ServerHostname) + return FALSE; } else if (settings->RedirectionFlags & LB_TARGET_NET_ADDRESS) { free(settings->ServerHostname); settings->ServerHostname = _strdup(settings->TargetNetAddress); + if (!settings->ServerHostname) + return FALSE; } else if (settings->RedirectionFlags & LB_TARGET_NETBIOS_NAME) { free(settings->ServerHostname); settings->ServerHostname = _strdup(settings->RedirectionTargetNetBiosName); + if (!settings->ServerHostname) + return FALSE; } } @@ -363,12 +371,16 @@ BOOL rdp_client_redirect(rdpRdp* rdp) { free(settings->Username); settings->Username = _strdup(settings->RedirectionUsername); + if (!settings->Username) + return FALSE; } if (settings->RedirectionFlags & LB_DOMAIN) { free(settings->Domain); settings->Domain = _strdup(settings->RedirectionDomain); + if (!settings->Domain) + return FALSE; } status = rdp_client_connect(rdp); diff --git a/libfreerdp/core/gateway/ncacn_http.c b/libfreerdp/core/gateway/ncacn_http.c index b67663788..175aab3c3 100644 --- a/libfreerdp/core/gateway/ncacn_http.c +++ b/libfreerdp/core/gateway/ncacn_http.c @@ -138,9 +138,24 @@ int rpc_ncacn_http_ntlm_init(rdpRpc* rpc, RpcChannel* channel) if (settings->GatewayUseSameCredentials) { - settings->Username = _strdup(settings->GatewayUsername); - settings->Domain = _strdup(settings->GatewayDomain); - settings->Password = _strdup(settings->GatewayPassword); + if (settings->GatewayUsername) + { + free(settings->Username); + if (!(settings->Username = _strdup(settings->GatewayUsername))) + return -1; + } + if (settings->GatewayDomain) + { + free(settings->Domain); + if (!(settings->Domain = _strdup(settings->GatewayDomain))) + return -1; + } + if (settings->GatewayPassword) + { + free(settings->Password); + if (!(settings->Password = _strdup(settings->GatewayPassword))) + return -1; + } } } } diff --git a/libfreerdp/core/gateway/rdg.c b/libfreerdp/core/gateway/rdg.c index 1154ebf39..60efa47f4 100644 --- a/libfreerdp/core/gateway/rdg.c +++ b/libfreerdp/core/gateway/rdg.c @@ -780,9 +780,24 @@ BOOL rdg_ncacn_http_ntlm_init(rdpRdg* rdg, rdpTls* tls) if (settings->GatewayUseSameCredentials) { - settings->Username = _strdup(settings->GatewayUsername); - settings->Domain = _strdup(settings->GatewayDomain); - settings->Password = _strdup(settings->GatewayPassword); + if (settings->GatewayUsername) + { + free(settings->Username); + if (!(settings->Username = _strdup(settings->GatewayUsername))) + return FALSE; + } + if (settings->GatewayDomain) + { + free(settings->Domain); + if (!(settings->Domain = _strdup(settings->GatewayDomain))) + return FALSE; + } + if (settings->GatewayPassword) + { + free(settings->Password); + if (!(settings->Password = _strdup(settings->GatewayPassword))) + return FALSE; + } } } } diff --git a/libfreerdp/core/nla.c b/libfreerdp/core/nla.c index 84a760c54..f83db8a11 100644 --- a/libfreerdp/core/nla.c +++ b/libfreerdp/core/nla.c @@ -1398,6 +1398,12 @@ LPTSTR nla_make_spn(const char* ServiceClass, const char* hostname) hostnameX = _strdup(hostname); ServiceClassX = _strdup(ServiceClass); #endif + if (!hostnameX || !ServiceClassX) + { + free(hostnameX); + free(ServiceClassX); + return NULL; + } if (!ServiceClass) { diff --git a/libfreerdp/core/redirection.c b/libfreerdp/core/redirection.c index 2fdcff55d..1b36b1eb9 100644 --- a/libfreerdp/core/redirection.c +++ b/libfreerdp/core/redirection.c @@ -134,28 +134,38 @@ int rdp_redirection_apply_settings(rdpRdp* rdp) { free(settings->RedirectionTargetFQDN); settings->RedirectionTargetFQDN = _strdup(redirection->TargetFQDN); + if (!settings->RedirectionTargetFQDN) + return -1; } else if (settings->RedirectionFlags & LB_TARGET_NET_ADDRESS) { free(settings->TargetNetAddress); settings->TargetNetAddress = _strdup(redirection->TargetNetAddress); + if (!settings->TargetNetAddress) + return -1; } else if (settings->RedirectionFlags & LB_TARGET_NETBIOS_NAME) { free(settings->RedirectionTargetNetBiosName); settings->RedirectionTargetNetBiosName = _strdup(redirection->TargetNetBiosName); + if (!settings->RedirectionTargetNetBiosName) + return -1; } if (settings->RedirectionFlags & LB_USERNAME) { free(settings->RedirectionUsername); settings->RedirectionUsername = _strdup(redirection->Username); + if (!settings->RedirectionUsername) + return -1; } if (settings->RedirectionFlags & LB_DOMAIN) { free(settings->RedirectionDomain); settings->RedirectionDomain = _strdup(redirection->Domain); + if (!settings->RedirectionDomain) + return -1; } if (settings->RedirectionFlags & LB_PASSWORD) @@ -164,6 +174,8 @@ int rdp_redirection_apply_settings(rdpRdp* rdp) free(settings->RedirectionPassword); settings->RedirectionPasswordLength = redirection->PasswordLength; settings->RedirectionPassword = (BYTE*) malloc(settings->RedirectionPasswordLength); + if (!settings->RedirectionPassword) + return -1; CopyMemory(settings->RedirectionPassword, redirection->Password, settings->RedirectionPasswordLength); } @@ -173,6 +185,8 @@ int rdp_redirection_apply_settings(rdpRdp* rdp) free(settings->RedirectionTsvUrl); settings->RedirectionTsvUrlLength = redirection->TsvUrlLength; settings->RedirectionTsvUrl = (BYTE*) malloc(settings->RedirectionTsvUrlLength); + if (!settings->RedirectionTsvUrl) + return -1; CopyMemory(settings->RedirectionTsvUrl, redirection->TsvUrl, settings->RedirectionTsvUrlLength); } @@ -182,10 +196,21 @@ int rdp_redirection_apply_settings(rdpRdp* rdp) freerdp_target_net_addresses_free(settings); settings->TargetNetAddressCount = redirection->TargetNetAddressesCount; settings->TargetNetAddresses = (char**) malloc(sizeof(char*) * settings->TargetNetAddressCount); + if (!settings->TargetNetAddresses) + { + settings->TargetNetAddressCount = 0; + return -1; + } for (i = 0; i < settings->TargetNetAddressCount; i++) { settings->TargetNetAddresses[i] = _strdup(redirection->TargetNetAddresses[i]); + if (!settings->TargetNetAddresses[i]) + { + for (--i; i >= 0; --i) + free(settings->TargetNetAddresses[i]); + return -1; + } } } diff --git a/libfreerdp/core/settings.c b/libfreerdp/core/settings.c index 339baa684..ab3acd9fd 100644 --- a/libfreerdp/core/settings.c +++ b/libfreerdp/core/settings.c @@ -549,59 +549,55 @@ rdpSettings* freerdp_settings_clone(rdpSettings* settings) { CopyMemory(_settings, settings, sizeof(rdpSettings)); - /** - * Generated Code - */ - /* char* values */ - - _settings->ServerHostname = _strdup(settings->ServerHostname); /* 20 */ - _settings->Username = _strdup(settings->Username); /* 21 */ - _settings->Password = _strdup(settings->Password); /* 22 */ - _settings->Domain = _strdup(settings->Domain); /* 23 */ - _settings->PasswordHash = _strdup(settings->PasswordHash); /* 24 */ +#define CHECKED_STRDUP(name) if (settings->name && !(_settings->name = _strdup(settings->name))) goto out_fail + CHECKED_STRDUP(ServerHostname); /* 20 */ + CHECKED_STRDUP(Username); /* 21 */ + CHECKED_STRDUP(Password); /* 22 */ + CHECKED_STRDUP(Domain); /* 23 */ + CHECKED_STRDUP(PasswordHash); /* 24 */ _settings->ClientHostname = NULL; /* 134 */ _settings->ClientProductId = NULL; /* 135 */ - _settings->AlternateShell = _strdup(settings->AlternateShell); /* 640 */ - _settings->ShellWorkingDirectory = _strdup(settings->ShellWorkingDirectory); /* 641 */ - _settings->ClientAddress = _strdup(settings->ClientAddress); /* 769 */ - _settings->ClientDir = _strdup(settings->ClientDir); /* 770 */ - _settings->DynamicDSTTimeZoneKeyName = _strdup(settings->DynamicDSTTimeZoneKeyName); /* 897 */ - _settings->RemoteAssistanceSessionId = _strdup(settings->RemoteAssistanceSessionId); /* 1025 */ - _settings->RemoteAssistancePassStub = _strdup(settings->RemoteAssistancePassStub); /* 1026 */ - _settings->RemoteAssistancePassword = _strdup(settings->RemoteAssistancePassword); /* 1027 */ - _settings->RemoteAssistanceRCTicket = _strdup(settings->RemoteAssistanceRCTicket); /* 1028 */ - _settings->AuthenticationServiceClass = _strdup(settings->AuthenticationServiceClass); /* 1098 */ - _settings->AllowedTlsCiphers = _strdup(settings->AllowedTlsCiphers); /* 1101 */ - _settings->PreconnectionBlob = _strdup(settings->PreconnectionBlob); /* 1155 */ - _settings->KerberosKdc = _strdup(settings->KerberosKdc); /* 1344 */ - _settings->KerberosRealm = _strdup(settings->KerberosRealm); /* 1345 */ - _settings->CertificateName = _strdup(settings->CertificateName); /* 1409 */ - _settings->CertificateFile = _strdup(settings->CertificateFile); /* 1410 */ - _settings->PrivateKeyFile = _strdup(settings->PrivateKeyFile); /* 1411 */ - _settings->RdpKeyFile = _strdup(settings->RdpKeyFile); /* 1412 */ - _settings->WindowTitle = _strdup(settings->WindowTitle); /* 1542 */ - _settings->WmClass = _strdup(settings->WmClass); /* 1549 */ - _settings->ComputerName = _strdup(settings->ComputerName); /* 1664 */ - _settings->ConnectionFile = _strdup(settings->ConnectionFile); /* 1728 */ - _settings->AssistanceFile = _strdup(settings->AssistanceFile); /* 1729 */ - _settings->HomePath = _strdup(settings->HomePath); /* 1792 */ - _settings->ConfigPath = _strdup(settings->ConfigPath); /* 1793 */ - _settings->CurrentPath = _strdup(settings->CurrentPath); /* 1794 */ - _settings->DumpRemoteFxFile = _strdup(settings->DumpRemoteFxFile); /* 1858 */ - _settings->PlayRemoteFxFile = _strdup(settings->PlayRemoteFxFile); /* 1859 */ - _settings->GatewayHostname = _strdup(settings->GatewayHostname); /* 1986 */ - _settings->GatewayUsername = _strdup(settings->GatewayUsername); /* 1987 */ - _settings->GatewayPassword = _strdup(settings->GatewayPassword); /* 1988 */ - _settings->GatewayDomain = _strdup(settings->GatewayDomain); /* 1989 */ - _settings->RemoteApplicationName = _strdup(settings->RemoteApplicationName); /* 2113 */ - _settings->RemoteApplicationIcon = _strdup(settings->RemoteApplicationIcon); /* 2114 */ - _settings->RemoteApplicationProgram = _strdup(settings->RemoteApplicationProgram); /* 2115 */ - _settings->RemoteApplicationFile = _strdup(settings->RemoteApplicationFile); /* 2116 */ - _settings->RemoteApplicationGuid = _strdup(settings->RemoteApplicationGuid); /* 2117 */ - _settings->RemoteApplicationCmdLine = _strdup(settings->RemoteApplicationCmdLine); /* 2118 */ - _settings->ImeFileName = _strdup(settings->ImeFileName); /* 2628 */ - _settings->DrivesToRedirect = _strdup(settings->DrivesToRedirect); /* 4290 */ + CHECKED_STRDUP(AlternateShell); /* 640 */ + CHECKED_STRDUP(ShellWorkingDirectory); /* 641 */ + CHECKED_STRDUP(ClientAddress); /* 769 */ + CHECKED_STRDUP(ClientDir); /* 770 */ + CHECKED_STRDUP(DynamicDSTTimeZoneKeyName); /* 897 */ + CHECKED_STRDUP(RemoteAssistanceSessionId); /* 1025 */ + CHECKED_STRDUP(RemoteAssistancePassStub); /* 1026 */ + CHECKED_STRDUP(RemoteAssistancePassword); /* 1027 */ + CHECKED_STRDUP(RemoteAssistanceRCTicket); /* 1028 */ + CHECKED_STRDUP(AuthenticationServiceClass); /* 1098 */ + CHECKED_STRDUP(AllowedTlsCiphers); /* 1101 */ + CHECKED_STRDUP(PreconnectionBlob); /* 1155 */ + CHECKED_STRDUP(KerberosKdc); /* 1344 */ + CHECKED_STRDUP(KerberosRealm); /* 1345 */ + CHECKED_STRDUP(CertificateName); /* 1409 */ + CHECKED_STRDUP(CertificateFile); /* 1410 */ + CHECKED_STRDUP(PrivateKeyFile); /* 1411 */ + CHECKED_STRDUP(RdpKeyFile); /* 1412 */ + CHECKED_STRDUP(WindowTitle); /* 1542 */ + CHECKED_STRDUP(WmClass); /* 1549 */ + CHECKED_STRDUP(ComputerName); /* 1664 */ + CHECKED_STRDUP(ConnectionFile); /* 1728 */ + CHECKED_STRDUP(AssistanceFile); /* 1729 */ + CHECKED_STRDUP(HomePath); /* 1792 */ + CHECKED_STRDUP(ConfigPath); /* 1793 */ + CHECKED_STRDUP(CurrentPath); /* 1794 */ + CHECKED_STRDUP(DumpRemoteFxFile); /* 1858 */ + CHECKED_STRDUP(PlayRemoteFxFile); /* 1859 */ + CHECKED_STRDUP(GatewayHostname); /* 1986 */ + CHECKED_STRDUP(GatewayUsername); /* 1987 */ + CHECKED_STRDUP(GatewayPassword); /* 1988 */ + CHECKED_STRDUP(GatewayDomain); /* 1989 */ + CHECKED_STRDUP(RemoteApplicationName); /* 2113 */ + CHECKED_STRDUP(RemoteApplicationIcon); /* 2114 */ + CHECKED_STRDUP(RemoteApplicationProgram); /* 2115 */ + CHECKED_STRDUP(RemoteApplicationFile); /* 2116 */ + CHECKED_STRDUP(RemoteApplicationGuid); /* 2117 */ + CHECKED_STRDUP(RemoteApplicationCmdLine); /* 2118 */ + CHECKED_STRDUP(ImeFileName); /* 2628 */ + CHECKED_STRDUP(DrivesToRedirect); /* 4290 */ /** * Manual Code @@ -666,15 +662,19 @@ rdpSettings* freerdp_settings_clone(rdpSettings* settings) _settings->ChannelCount = settings->ChannelCount; _settings->ChannelDefArraySize = settings->ChannelDefArraySize; _settings->ChannelDefArray = (CHANNEL_DEF*) malloc(sizeof(CHANNEL_DEF) * settings->ChannelDefArraySize); + if (!_settings->ChannelDefArray && _settings->ChannelDefArraySize) goto out_fail; + CopyMemory(_settings->ChannelDefArray, settings->ChannelDefArray, sizeof(CHANNEL_DEF) * settings->ChannelDefArraySize); _settings->MonitorCount = settings->MonitorCount; _settings->MonitorDefArraySize = settings->MonitorDefArraySize; _settings->MonitorDefArray = (rdpMonitor*) malloc(sizeof(rdpMonitor) * settings->MonitorDefArraySize); + if (!_settings->MonitorDefArray && _settings->MonitorDefArraySize) goto out_fail; + CopyMemory(_settings->MonitorDefArray, settings->MonitorDefArray, sizeof(rdpMonitor) * settings->MonitorDefArraySize); _settings->MonitorIds = (UINT32*) calloc(16, sizeof(UINT32)); @@ -683,11 +683,11 @@ rdpSettings* freerdp_settings_clone(rdpSettings* settings) CopyMemory(_settings->MonitorIds, settings->MonitorIds, 16 * sizeof(UINT32)); _settings->ReceivedCapabilities = malloc(32); - if (!_settings->ReceivedCapabilities) - goto out_fail; _settings->OrderSupport = malloc(32); - if (!_settings->OrderSupport) + + if (!_settings->ReceivedCapabilities || !_settings->OrderSupport) goto out_fail; + CopyMemory(_settings->ReceivedCapabilities, settings->ReceivedCapabilities, 32); CopyMemory(_settings->OrderSupport, settings->OrderSupport, 32); @@ -733,16 +733,24 @@ rdpSettings* freerdp_settings_clone(rdpSettings* settings) if (settings->TargetNetAddressCount > 0) { _settings->TargetNetAddresses = (char**) calloc(settings->TargetNetAddressCount, sizeof(char*)); - if (!_settings->TargetNetAddresses) + { + _settings->TargetNetAddressCount = 0; goto out_fail; + } for (index = 0; index < settings->TargetNetAddressCount; index++) { _settings->TargetNetAddresses[index] = _strdup(settings->TargetNetAddresses[index]); - if (!_settings->TargetNetAddresses[index]) + { + for (--index; index >= 0; --index) + free(_settings->TargetNetAddresses[index]); + free(_settings->TargetNetAddresses); + _settings->TargetNetAddresses = NULL; + _settings->TargetNetAddressCount = 0; goto out_fail; + } } if (settings->TargetNetPorts) @@ -760,37 +768,52 @@ rdpSettings* freerdp_settings_clone(rdpSettings* settings) _settings->DeviceCount = settings->DeviceCount; _settings->DeviceArraySize = settings->DeviceArraySize; _settings->DeviceArray = (RDPDR_DEVICE**) calloc(_settings->DeviceArraySize, sizeof(RDPDR_DEVICE*)); - if (!_settings->DeviceArray && _settings->DeviceArraySize) + { + _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]); + if (!_settings->DeviceArray[index]) + goto out_fail; } _settings->StaticChannelCount = settings->StaticChannelCount; _settings->StaticChannelArraySize = settings->StaticChannelArraySize; _settings->StaticChannelArray = (ADDIN_ARGV**) calloc(_settings->StaticChannelArraySize, sizeof(ADDIN_ARGV*)); - if (!_settings->StaticChannelArray && _settings->StaticChannelArraySize) + { + _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]); + if (!_settings->StaticChannelArray[index]) + goto out_fail; } _settings->DynamicChannelCount = settings->DynamicChannelCount; _settings->DynamicChannelArraySize = settings->DynamicChannelArraySize; _settings->DynamicChannelArray = (ADDIN_ARGV**) calloc(_settings->DynamicChannelArraySize, sizeof(ADDIN_ARGV*)); - if (!_settings->DynamicChannelArray && _settings->DynamicChannelArraySize) + { + _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]); + if (!_settings->DynamicChannelArray[index]) + goto out_fail; } _settings->SettingsModified = (BYTE*) calloc(1, sizeof(rdpSettings) / 8); @@ -798,10 +821,16 @@ rdpSettings* freerdp_settings_clone(rdpSettings* settings) if (!_settings->SettingsModified) goto out_fail; } - return _settings; + out_fail: - freerdp_settings_free(_settings); + /* In case any memory allocation failed during clone, some bytes might leak. + * + * freerdp_settings_free can't be reliable used at this point since it could + * free memory of pointers copied by CopyMemory and detecting and freeing + * each allocation separately is quite painful. + */ + free(_settings); return NULL; } diff --git a/libfreerdp/core/tcp.c b/libfreerdp/core/tcp.c index af92c6d79..4eb7b0c24 100644 --- a/libfreerdp/core/tcp.c +++ b/libfreerdp/core/tcp.c @@ -1283,8 +1283,9 @@ int freerdp_tcp_connect(rdpSettings* settings, const char* hostname, int port, i if (!freerdp_tcp_connect_timeout(sockfd, addr->ai_addr, addr->ai_addrlen, timeout)) { - fprintf(stderr, "failed to connect to %s\n", hostname); freeaddrinfo(result); + close(sockfd); + WLog_ERR(TAG, "failed to connect to %s", hostname); return -1; } @@ -1296,6 +1297,12 @@ int freerdp_tcp_connect(rdpSettings* settings, const char* hostname, int port, i free(settings->ClientAddress); settings->ClientAddress = freerdp_tcp_get_ip_address(sockfd); + if (!settings->ClientAddress) + { + close(sockfd); + WLog_ERR(TAG, "Couldn't get socket ip address"); + return -1; + } optval = 1; optlen = sizeof(optval); @@ -1316,6 +1323,7 @@ int freerdp_tcp_connect(rdpSettings* settings, const char* hostname, int port, i if (setsockopt(sockfd, SOL_SOCKET, SO_RCVBUF, (void*) &optval, optlen) < 0) { + close(sockfd); WLog_ERR(TAG, "unable to set receive buffer len"); return -1; } @@ -1325,7 +1333,11 @@ int freerdp_tcp_connect(rdpSettings* settings, const char* hostname, int port, i if (!ipcSocket) { if (!freerdp_tcp_set_keep_alive_mode(sockfd)) + { + close(sockfd); + WLog_ERR(TAG, "Couldn't set keep alive mode."); return -1; + } } return sockfd; diff --git a/libfreerdp/core/test/CMakeLists.txt b/libfreerdp/core/test/CMakeLists.txt index 7b9853519..1c06ec601 100644 --- a/libfreerdp/core/test/CMakeLists.txt +++ b/libfreerdp/core/test/CMakeLists.txt @@ -5,7 +5,8 @@ set(MODULE_PREFIX "TEST_CORE") set(${MODULE_PREFIX}_DRIVER ${MODULE_NAME}.c) set(${MODULE_PREFIX}_TESTS - TestVersion.c) + TestVersion.c + TestSettings.c) create_test_sourcelist(${MODULE_PREFIX}_SRCS ${${MODULE_PREFIX}_DRIVER} diff --git a/libfreerdp/core/test/TestSettings.c b/libfreerdp/core/test/TestSettings.c new file mode 100644 index 000000000..7c81f4e48 --- /dev/null +++ b/libfreerdp/core/test/TestSettings.c @@ -0,0 +1,28 @@ +#include + +int TestSettings(int argc, char* argv[]) +{ + rdpSettings *settings = NULL; + rdpSettings *cloned; + + settings = freerdp_settings_new(0); + if (!settings) + { + printf("Couldn't create settings\n"); + return -1; + } + settings->Username = strdup("abcdefg"); + settings->Password = strdup("xyz"); + cloned = freerdp_settings_clone(settings); + if (!cloned) + { + printf("Problem cloning settings\n"); + freerdp_settings_free(settings); + return -1; + } + + freerdp_settings_free(cloned); + freerdp_settings_free(settings); + return 0; +} + diff --git a/libfreerdp/locale/keyboard_layout.c b/libfreerdp/locale/keyboard_layout.c index 952c93e66..91bec80f8 100644 --- a/libfreerdp/locale/keyboard_layout.c +++ b/libfreerdp/locale/keyboard_layout.c @@ -248,6 +248,14 @@ RDP_KEYBOARD_LAYOUT* freerdp_keyboard_get_layouts(DWORD types) { layouts[num].code = RDP_KEYBOARD_LAYOUT_TABLE[i].code; layouts[num].name = _strdup(RDP_KEYBOARD_LAYOUT_TABLE[i].name); + if (!layouts[num].name) + { + for (--i; i >=0; --i) + free(layouts[num].name); + + free(layouts); + return NULL; + } } } if ((types & RDP_KEYBOARD_LAYOUT_TYPE_VARIANT) != 0) @@ -265,6 +273,14 @@ RDP_KEYBOARD_LAYOUT* freerdp_keyboard_get_layouts(DWORD types) { layouts[num].code = RDP_KEYBOARD_LAYOUT_VARIANT_TABLE[i].code; layouts[num].name = _strdup(RDP_KEYBOARD_LAYOUT_VARIANT_TABLE[i].name); + if (!layouts[num].name) + { + for (--i; i >=0; --i) + free(layouts[num].name); + + free(layouts); + return NULL; + } } } if ((types & RDP_KEYBOARD_LAYOUT_TYPE_IME) != 0) @@ -282,6 +298,14 @@ RDP_KEYBOARD_LAYOUT* freerdp_keyboard_get_layouts(DWORD types) { layouts[num].code = RDP_KEYBOARD_IME_TABLE[i].code; layouts[num].name = _strdup(RDP_KEYBOARD_IME_TABLE[i].name); + if (!layouts[num].name) + { + for (--i; i >=0; --i) + free(layouts[num].name); + + free(layouts); + return NULL; + } } } diff --git a/libfreerdp/locale/timezone.c b/libfreerdp/locale/timezone.c index 78a8cf121..c09f09703 100644 --- a/libfreerdp/locale/timezone.c +++ b/libfreerdp/locale/timezone.c @@ -1597,6 +1597,8 @@ BOOL freerdp_match_unix_timezone_identifier_with_list(const char* tzid, const ch char* list_copy; list_copy = _strdup(list); + if (!list_copy) + return FALSE; p = strtok(list_copy, " "); diff --git a/server/Mac/mf_peer.c b/server/Mac/mf_peer.c index 9a837e528..f67e2c4c1 100644 --- a/server/Mac/mf_peer.c +++ b/server/Mac/mf_peer.c @@ -24,6 +24,7 @@ #include #include #include +#include #include @@ -388,6 +389,13 @@ void* mf_peer_main_loop(void* arg) /* Initialize the real server settings here */ client->settings->CertificateFile = _strdup("server.crt"); client->settings->PrivateKeyFile = _strdup("server.key"); + if (!client->settings->CertificateFile || !client->settings->PrivateKeyFile) + { + freerdp_peer_free(client); + return NULL; + } + + client->settings->NlaSecurity = FALSE; client->settings->RemoteFxCodec = TRUE; client->settings->ColorDepth = 32; diff --git a/server/Sample/sfreerdp.c b/server/Sample/sfreerdp.c index 7d5355151..7cb018b42 100644 --- a/server/Sample/sfreerdp.c +++ b/server/Sample/sfreerdp.c @@ -167,7 +167,7 @@ static void test_peer_end_frame(freerdp_peer* client) context->frame_id++; } -static void test_peer_draw_background(freerdp_peer* client) +static BOOL test_peer_draw_background(freerdp_peer* client) { int size; wStream* s; @@ -176,9 +176,10 @@ static void test_peer_draw_background(freerdp_peer* client) rdpUpdate* update = client->update; SURFACE_BITS_COMMAND* cmd = &update->surface_bits_command; testPeerContext* context = (testPeerContext*) client->context; + BOOL ret= FALSE; if (!client->settings->RemoteFxCodec && !client->settings->NSCodec) - return; + return FALSE; s = test_peer_stream_init(context); @@ -189,7 +190,10 @@ static void test_peer_draw_background(freerdp_peer* client) size = rect.width * rect.height * 3; if (!(rgb_data = malloc(size))) - return; + { + WLog_ERR(TAG, "Problem allocating memory"); + return FALSE; + } memset(rgb_data, 0xA0, size); @@ -223,11 +227,13 @@ static void test_peer_draw_background(freerdp_peer* client) update->SurfaceBits(update->context, cmd); test_peer_end_frame(client); + ret = TRUE; out: free(rgb_data); + return ret; } -static void test_peer_load_icon(freerdp_peer* client) +static BOOL test_peer_load_icon(freerdp_peer* client) { testPeerContext* context = (testPeerContext*) client->context; FILE* fp; @@ -237,10 +243,16 @@ static void test_peer_load_icon(freerdp_peer* client) int c; if (!client->settings->RemoteFxCodec && !client->settings->NSCodec) - return; + { + WLog_ERR(TAG, "Client doesn't support RemoteFX or NSCodec"); + return FALSE; + } if ((fp = fopen("test_icon.ppm", "r")) == NULL) - return; + { + WLog_ERR(TAG, "Unable to open test icon"); + return FALSE; + } /* P3 */ fgets(line, sizeof(line), fp); @@ -248,26 +260,40 @@ static void test_peer_load_icon(freerdp_peer* client) fgets(line, sizeof(line), fp); /* width height */ fgets(line, sizeof(line), fp); - sscanf(line, "%d %d", &context->icon_width, &context->icon_height); + if (sscanf(line, "%d %d", &context->icon_width, &context->icon_height) < 2) + { + WLog_ERR(TAG, "Problem while extracting width/height from the icon file"); + goto out_fail; + } /* Max */ fgets(line, sizeof(line), fp); - rgb_data = malloc(context->icon_width * context->icon_height * 3); + if (!(rgb_data = malloc(context->icon_width * context->icon_height * 3))) + goto out_fail; for (i = 0; i < context->icon_width * context->icon_height * 3; i++) { - if (fgets(line, sizeof(line), fp)) - { - sscanf(line, "%d", &c); - rgb_data[i] = (BYTE)c; - } + if (!fgets(line, sizeof(line), fp) || (sscanf(line, "%d", &c) != 1)) + goto out_fail; + + rgb_data[i] = (BYTE)c; } - context->icon_data = rgb_data; /* background with same size, which will be used to erase the icon from old position */ - context->bg_data = malloc(context->icon_width * context->icon_height * 3); + if (!(context->bg_data = malloc(context->icon_width * context->icon_height * 3))) + goto out_fail; memset(context->bg_data, 0xA0, context->icon_width * context->icon_height * 3); + context->icon_data = rgb_data; + + fclose(fp); + return TRUE; + +out_fail: + free(rgb_data); + context->bg_data = NULL; + fclose(fp); + return FALSE; } static void test_peer_draw_icon(freerdp_peer* client, int x, int y) @@ -392,7 +418,7 @@ static BOOL test_sleep_tsdiff(UINT32 *old_sec, UINT32 *old_usec, UINT32 new_sec, return TRUE; } -void tf_peer_dump_rfx(freerdp_peer* client) +BOOL tf_peer_dump_rfx(freerdp_peer* client) { wStream* s; UINT32 prev_seconds; @@ -402,20 +428,26 @@ void tf_peer_dump_rfx(freerdp_peer* client) pcap_record record; s = Stream_New(NULL, 512); - update = client->update; - client->update->pcap_rfx = pcap_open(test_pcap_file, FALSE); - pcap_rfx = client->update->pcap_rfx; + if (!s) + return FALSE; - if (pcap_rfx == NULL) - return; + update = client->update; + if (!(pcap_rfx = pcap_open(test_pcap_file, FALSE))) + return FALSE; prev_seconds = prev_useconds = 0; while (pcap_has_next_record(pcap_rfx)) { - pcap_get_next_record_header(pcap_rfx, &record); + BYTE* tmp = NULL; + if (!pcap_get_next_record_header(pcap_rfx, &record)) + break; - Stream_Buffer(s) = realloc(Stream_Buffer(s), record.length); + tmp = realloc(Stream_Buffer(s), record.length); + if (!tmp) + break; + + Stream_Buffer(s) = tmp; record.data = Stream_Buffer(s); Stream_Capacity(s) = record.length; @@ -430,6 +462,11 @@ void tf_peer_dump_rfx(freerdp_peer* client) if (client->CheckFileDescriptor(client) != TRUE) break; } + + + Stream_Free(s, TRUE); + pcap_close(pcap_rfx); + return TRUE; } static void* tf_debug_channel_thread_func(void* arg) @@ -524,7 +561,12 @@ BOOL tf_peer_post_connect(freerdp_peer* client) #endif /* A real server should tag the peer as activated here and start sending updates in main loop. */ - test_peer_load_icon(client); + if (!test_peer_load_icon(client)) + { + WLog_DBG(TAG, "Unable to load icon"); + return FALSE; + } + if (WTSVirtualChannelManagerIsChannelJoined(context->vcm, "rdpdbg")) { @@ -584,12 +626,11 @@ BOOL tf_peer_activate(freerdp_peer* client) if (test_pcap_file != NULL) { client->update->dump_rfx = TRUE; - tf_peer_dump_rfx(client); + if (!tf_peer_dump_rfx(client)) + return FALSE; } else - { test_peer_draw_background(client); - } return TRUE; } @@ -718,6 +759,12 @@ static void* test_peer_mainloop(void* arg) client->settings->CertificateFile = _strdup("server.crt"); client->settings->PrivateKeyFile = _strdup("server.key"); client->settings->RdpKeyFile = _strdup("server.key"); + if (!client->settings->CertificateFile || !client->settings->PrivateKeyFile || !client->settings->RdpKeyFile) + { + WLog_ERR(TAG, "Memory allocation failed (strdup)"); + freerdp_peer_free(client); + return NULL; + } client->settings->RdpSecurity = TRUE; client->settings->TlsSecurity = TRUE; client->settings->NlaSecurity = FALSE; diff --git a/server/Windows/wf_peer.c b/server/Windows/wf_peer.c index 598080e68..85ea76fdd 100644 --- a/server/Windows/wf_peer.c +++ b/server/Windows/wf_peer.c @@ -37,6 +37,7 @@ #include "wf_rdpsnd.h" #include "wf_peer.h" +#include BOOL wf_peer_context_new(freerdp_peer* client, wfPeerContext* context) { @@ -211,13 +212,23 @@ DWORD WINAPI wf_peer_socket_listener(LPVOID lpParam) return 0; } -void wf_peer_read_settings(freerdp_peer* client) +BOOL wf_peer_read_settings(freerdp_peer* client) { if (!wf_settings_read_string_ascii(HKEY_LOCAL_MACHINE, _T("Software\\FreeRDP\\Server"), _T("CertificateFile"), &(client->settings->CertificateFile))) + { client->settings->CertificateFile = _strdup("server.crt"); + if (!client->settings->CertificateFile) + return FALSE; + } if (!wf_settings_read_string_ascii(HKEY_LOCAL_MACHINE, _T("Software\\FreeRDP\\Server"), _T("PrivateKeyFile"), &(client->settings->PrivateKeyFile))) + { client->settings->PrivateKeyFile = _strdup("server.key"); + if (!client->settings->PrivateKeyFile) + return FALSE; + } + + return TRUE; } DWORD WINAPI wf_peer_main_loop(LPVOID lpParam) @@ -246,7 +257,8 @@ DWORD WINAPI wf_peer_main_loop(LPVOID lpParam) settings->ColorDepth = 32; settings->NSCodec = FALSE; settings->JpegCodec = FALSE; - wf_peer_read_settings(client); + if (!wf_peer_read_settings(client)) + goto fail_peer_init; client->PostConnect = wf_peer_post_connect; client->Activate = wf_peer_activate; diff --git a/server/shadow/X11/x11_shadow.c b/server/shadow/X11/x11_shadow.c index c297683a8..bef92a5ab 100644 --- a/server/shadow/X11/x11_shadow.c +++ b/server/shadow/X11/x11_shadow.c @@ -76,16 +76,14 @@ typedef struct _SHADOW_PAM_AUTH_INFO SHADOW_PAM_AUTH_INFO; int x11_shadow_pam_conv(int num_msg, const struct pam_message** msg, struct pam_response** resp, void* appdata_ptr) { int index; - int pam_status = PAM_SUCCESS; + int pam_status = PAM_BUF_ERR; SHADOW_PAM_AUTH_DATA* appdata; struct pam_response* response; appdata = (SHADOW_PAM_AUTH_DATA*) appdata_ptr; - response = (struct pam_response*) calloc(num_msg, sizeof(struct pam_response)); - - if (!response) - return PAM_CONV_ERR; + if (!(response = (struct pam_response*) calloc(num_msg, sizeof(struct pam_response)))) + return PAM_BUF_ERR; for (index = 0; index < num_msg; index++) { @@ -93,29 +91,40 @@ int x11_shadow_pam_conv(int num_msg, const struct pam_message** msg, struct pam_ { case PAM_PROMPT_ECHO_ON: response[index].resp = _strdup(appdata->user); + if (!response[index].resp) + goto out_fail; response[index].resp_retcode = PAM_SUCCESS; break; case PAM_PROMPT_ECHO_OFF: response[index].resp = _strdup(appdata->password); + if (!response[index].resp) + goto out_fail; response[index].resp_retcode = PAM_SUCCESS; break; default: pam_status = PAM_CONV_ERR; - break; + goto out_fail; } } - if (pam_status != PAM_SUCCESS) - { - free(response); - return pam_status; - } - *resp = response; + return PAM_SUCCESS; - return pam_status; +out_fail: + for (index = 0; index < num_msg; ++index) + { + if (response[index].resp) + { + memset(response[index].resp, 0, strlen(response[index].resp)); + free(response[index].resp); + } + } + memset(response, 0, sizeof(struct pam_response) * num_msg); + free(response); + *resp = NULL; + return PAM_CONV_ERR; } int x11_shadow_pam_get_service_name(SHADOW_PAM_AUTH_INFO* info) @@ -145,6 +154,9 @@ int x11_shadow_pam_get_service_name(SHADOW_PAM_AUTH_INFO* info) return -1; } + if (!info->service_name) + return -1; + return 1; } diff --git a/server/shadow/shadow_client.c b/server/shadow/shadow_client.c index 650101400..4303d12e7 100644 --- a/server/shadow/shadow_client.c +++ b/server/shadow/shadow_client.c @@ -589,11 +589,9 @@ int shadow_client_send_bitmap_update(rdpShadowClient* client, rdpShadowSurface* totalBitmapSize = 0; bitmapUpdate.count = bitmapUpdate.number = rows * cols; - bitmapData = (BITMAP_DATA*) malloc(sizeof(BITMAP_DATA) * bitmapUpdate.number); - bitmapUpdate.rectangles = bitmapData; - - if (!bitmapData) + if (!(bitmapData = (BITMAP_DATA*) malloc(sizeof(BITMAP_DATA) * bitmapUpdate.number))) return -1; + bitmapUpdate.rectangles = bitmapData; if ((nWidth % 4) != 0) { diff --git a/server/shadow/shadow_server.c b/server/shadow/shadow_server.c index ebcd00a2a..5e71619d3 100644 --- a/server/shadow/shadow_server.c +++ b/server/shadow/shadow_server.c @@ -88,6 +88,8 @@ int shadow_server_print_command_line_help(int argc, char** argv) { length = (int) (strlen(arg->Name) + strlen(arg->Format) + 2); str = (char*) malloc(length + 1); + if (str) + return -1; sprintf_s(str, length + 1, "%s:%s", arg->Name, arg->Format); WLog_INFO(TAG, "%-20s", str); free(str); @@ -103,6 +105,8 @@ int shadow_server_print_command_line_help(int argc, char** argv) { length = (int) strlen(arg->Name) + 32; str = (char*) malloc(length + 1); + if (!str) + return -1; sprintf_s(str, length + 1, "%s (default:%s)", arg->Name, arg->Default ? "on" : "off"); @@ -132,7 +136,8 @@ int shadow_server_command_line_status_print(rdpShadowServer* server, int argc, c } else if (status < 0) { - shadow_server_print_command_line_help(argc, argv); + if (shadow_server_print_command_line_help(argc, argv) < 0) + return -1; return COMMAND_LINE_STATUS_PRINT_HELP; } @@ -174,6 +179,8 @@ int shadow_server_parse_command_line(rdpShadowServer* server, int argc, char** a CommandLineSwitchCase(arg, "ipc-socket") { server->ipcSocket = _strdup(arg->Value); + if (!server->ipcSocket) + return -1; } CommandLineSwitchCase(arg, "may-view") { @@ -189,7 +196,6 @@ int shadow_server_parse_command_line(rdpShadowServer* server, int argc, char** a char* tok[4]; int x, y, w, h; char* str = _strdup(arg->Value); - if (!str) return -1; @@ -499,10 +505,11 @@ int shadow_server_init_config_path(rdpShadowServer* server) return 1; } -int shadow_server_init_certificate(rdpShadowServer* server) +static BOOL shadow_server_init_certificate(rdpShadowServer* server) { char* filepath; - MAKECERT_CONTEXT* makecert; + MAKECERT_CONTEXT* makecert = NULL; + BOOL ret = FALSE; const char* makecert_argv[6] = { @@ -519,44 +526,58 @@ int shadow_server_init_certificate(rdpShadowServer* server) !PathMakePathA(server->ConfigPath, 0)) { WLog_ERR(TAG, "Failed to create directory '%s'", server->ConfigPath); - return -1; + return FALSE; } if (!(filepath = GetCombinedPath(server->ConfigPath, "shadow"))) - return -1; + return FALSE; if (!PathFileExistsA(filepath) && !PathMakePathA(filepath, 0)) { - WLog_ERR(TAG, "Failed to create directory '%s'", filepath); - free(filepath); - return -1; + if (!CreateDirectoryA(filepath, 0)) + { + WLog_ERR(TAG, "Failed to create directory '%s'", filepath); + goto out_fail; + } } server->CertificateFile = GetCombinedPath(filepath, "shadow.crt"); server->PrivateKeyFile = GetCombinedPath(filepath, "shadow.key"); + if (!server->CertificateFile || !server->PrivateKeyFile) + goto out_fail; if ((!PathFileExistsA(server->CertificateFile)) || (!PathFileExistsA(server->PrivateKeyFile))) { makecert = makecert_context_new(); + if (!makecert) + goto out_fail; - makecert_context_process(makecert, makecert_argc, (char**) makecert_argv); + if (makecert_context_process(makecert, makecert_argc, (char**) makecert_argv) != 1) + goto out_fail; - makecert_context_set_output_file_name(makecert, "shadow"); + if (!makecert_context_set_output_file_name(makecert, "shadow") != 1) + goto out_fail; if (!PathFileExistsA(server->CertificateFile)) - makecert_context_output_certificate_file(makecert, filepath); + { + if (makecert_context_output_certificate_file(makecert, filepath) != 1) + goto out_fail; + } if (!PathFileExistsA(server->PrivateKeyFile)) - makecert_context_output_private_key_file(makecert, filepath); - - makecert_context_free(makecert); + { + if (makecert_context_output_private_key_file(makecert, filepath) != 1) + goto out_fail; + } } - + ret = TRUE; +out_fail: + makecert_context_free(makecert); free(filepath); - return 1; + return ret; } int shadow_server_init(rdpShadowServer* server) diff --git a/winpr/include/winpr/wlog.h b/winpr/include/winpr/wlog.h index 5c5bef56b..6b20b3d76 100644 --- a/winpr/include/winpr/wlog.h +++ b/winpr/include/winpr/wlog.h @@ -324,15 +324,15 @@ WINPR_API int WLog_CloseAppender(wLog* log); WINPR_API void WLog_ConsoleAppender_SetOutputStream(wLog* log, wLogConsoleAppender* appender, int outputStream); -WINPR_API void WLog_FileAppender_SetOutputFileName(wLog* log, wLogFileAppender* appender, const char* filename); -WINPR_API void WLog_FileAppender_SetOutputFilePath(wLog* log, wLogFileAppender* appender, const char* filepath); +WINPR_API BOOL WLog_FileAppender_SetOutputFileName(wLog* log, wLogFileAppender* appender, const char* filename); +WINPR_API BOOL WLog_FileAppender_SetOutputFilePath(wLog* log, wLogFileAppender* appender, const char* filepath); WINPR_API void WLog_CallbackAppender_SetCallbacks(wLog* log, wLogCallbackAppender* appender, CallbackAppenderMessage_t msg, CallbackAppenderImage_t img, CallbackAppenderPackage_t pkg, CallbackAppenderData_t data); WINPR_API wLogLayout* WLog_GetLogLayout(wLog* log); -WINPR_API void WLog_Layout_SetPrefixFormat(wLog* log, wLogLayout* layout, const char* format); +WINPR_API BOOL WLog_Layout_SetPrefixFormat(wLog* log, wLogLayout* layout, const char* format); WINPR_API wLog* WLog_GetRoot(void); WINPR_API wLog* WLog_Get(LPCSTR name); diff --git a/winpr/libwinpr/clipboard/clipboard.c b/winpr/libwinpr/clipboard/clipboard.c index e363e7692..3e6a524a9 100644 --- a/winpr/libwinpr/clipboard/clipboard.c +++ b/winpr/libwinpr/clipboard/clipboard.c @@ -373,7 +373,12 @@ BOOL ClipboardInitFormats(wClipboard* clipboard) format->formatName = _strdup(CF_STANDARD_STRINGS[formatId]); if (!format->formatName) + { + for (--formatId; formatId >= 0; --formatId) + free((void *)format->formatName); + clipboard->numFormats = 0; return FALSE; + } } ClipboardInitSynthesizers(clipboard); @@ -511,7 +516,11 @@ wClipboard* ClipboardCreate() clipboard->nextFormatId = 0xC000; clipboard->sequenceNumber = 0; - InitializeCriticalSectionAndSpinCount(&(clipboard->lock), 4000); + if (!InitializeCriticalSectionAndSpinCount(&(clipboard->lock), 4000)) + { + free(clipboard); + return NULL; + } clipboard->numFormats = 0; clipboard->maxFormats = 64; @@ -519,11 +528,17 @@ wClipboard* ClipboardCreate() if (!clipboard->formats) { + DeleteCriticalSection(&(clipboard->lock)); free(clipboard); return NULL; } - ClipboardInitFormats(clipboard); + if(!ClipboardInitFormats(clipboard)) + { + DeleteCriticalSection(&(clipboard->lock)); + free(clipboard); + return NULL; + } } return clipboard; diff --git a/winpr/libwinpr/clipboard/test/TestClipboardFormats.c b/winpr/libwinpr/clipboard/test/TestClipboardFormats.c index 2242bcd2a..474eb37e1 100644 --- a/winpr/libwinpr/clipboard/test/TestClipboardFormats.c +++ b/winpr/libwinpr/clipboard/test/TestClipboardFormats.c @@ -42,6 +42,11 @@ int TestClipboardFormats(int argc, char* argv[]) char* pDstData; pSrcData = _strdup("this is a test string"); + if (!pSrcData) + { + fprintf(stderr, "Memory allocation failed\n"); + return -1; + } SrcSize = (UINT32) (strlen(pSrcData) + 1); bSuccess = ClipboardSetData(clipboard, utf8StringFormatId, (void*) pSrcData, SrcSize); diff --git a/winpr/libwinpr/file/test/TestFileFindFirstFile.c b/winpr/libwinpr/file/test/TestFileFindFirstFile.c index 9b6922db5..917cf3930 100644 --- a/winpr/libwinpr/file/test/TestFileFindFirstFile.c +++ b/winpr/libwinpr/file/test/TestFileFindFirstFile.c @@ -31,6 +31,11 @@ int TestFileFindFirstFile(int argc, char* argv[]) BasePath[length] = 0; #else BasePath = _strdup(str); + if (!BasePath) + { + printf("Unable to allocate memory\n"); + return -1; + } length = strlen(BasePath); #endif diff --git a/winpr/libwinpr/file/test/TestFileFindNextFile.c b/winpr/libwinpr/file/test/TestFileFindNextFile.c index cc8dd2a88..23b9fcc80 100644 --- a/winpr/libwinpr/file/test/TestFileFindNextFile.c +++ b/winpr/libwinpr/file/test/TestFileFindNextFile.c @@ -33,6 +33,11 @@ int TestFileFindNextFile(int argc, char* argv[]) BasePath[length] = 0; #else BasePath = _strdup(str); + if (!BasePath) + { + printf("Unable to allocate memory"); + return -1; + } length = strlen(BasePath); #endif diff --git a/winpr/libwinpr/io/device.c b/winpr/libwinpr/io/device.c index 6e26ccaf7..0e13092d7 100644 --- a/winpr/libwinpr/io/device.c +++ b/winpr/libwinpr/io/device.c @@ -30,6 +30,7 @@ #include #include #include +#include #ifdef HAVE_UNISTD_H #include @@ -95,13 +96,21 @@ char* GetDeviceFileUnixDomainSocketBaseFilePathA() char* GetDeviceFileUnixDomainSocketFilePathA(LPCSTR lpName) { - char* lpPipePath; - char* lpFileName; - char* lpFilePath; + char* lpPipePath = NULL; + char* lpFileName = NULL; + char* lpFilePath = NULL; lpPipePath = GetDeviceFileUnixDomainSocketBaseFilePathA(); + if (!lpPipePath) + return NULL; lpFileName = GetDeviceFileNameWithoutPrefixA(lpName); + if (!lpFileName) + { + free(lpFilePath); + return NULL; + } + lpFilePath = GetCombinedPath(lpPipePath, (char*) lpFileName); free(lpPipePath); @@ -124,6 +133,9 @@ NTSTATUS _IoCreateDeviceEx(PDRIVER_OBJECT_EX DriverObject, ULONG DeviceExtension DeviceBasePath = GetDeviceFileUnixDomainSocketBaseFilePathA(); + if (!DeviceBasePath) + return STATUS_NO_MEMORY; + if (!PathFileExistsA(DeviceBasePath)) { if (!mkdir(DeviceBasePath, S_IRUSR | S_IWUSR | S_IXUSR)) @@ -132,26 +144,63 @@ NTSTATUS _IoCreateDeviceEx(PDRIVER_OBJECT_EX DriverObject, ULONG DeviceExtension return STATUS_ACCESS_DENIED; } } + free(DeviceBasePath); - pDeviceObjectEx = (DEVICE_OBJECT_EX*) malloc(sizeof(DEVICE_OBJECT_EX)); + pDeviceObjectEx = (DEVICE_OBJECT_EX*) calloc(1, sizeof(DEVICE_OBJECT_EX)); if (!pDeviceObjectEx) + return STATUS_NO_MEMORY; + + ConvertFromUnicode(CP_UTF8, 0, DeviceName->Buffer, DeviceName->Length / 2, &(pDeviceObjectEx->DeviceName), 0, NULL, NULL); + if (!pDeviceObjectEx->DeviceName) { + free(pDeviceObjectEx); return STATUS_NO_MEMORY; } - ZeroMemory(pDeviceObjectEx, sizeof(DEVICE_OBJECT_EX)); - - ConvertFromUnicode(CP_UTF8, 0, DeviceName->Buffer, DeviceName->Length / 2, &(pDeviceObjectEx->DeviceName), 0, NULL, NULL); - pDeviceObjectEx->DeviceFileName = GetDeviceFileUnixDomainSocketFilePathA(pDeviceObjectEx->DeviceName); + if (!pDeviceObjectEx->DeviceFileName) + { + free(pDeviceObjectEx->DeviceName); + free(pDeviceObjectEx); + return STATUS_NO_MEMORY; + } if (PathFileExistsA(pDeviceObjectEx->DeviceFileName)) { - unlink(pDeviceObjectEx->DeviceFileName); + if (unlink(pDeviceObjectEx->DeviceFileName) == -1) + { + free(pDeviceObjectEx->DeviceName); + free(pDeviceObjectEx->DeviceFileName); + free(pDeviceObjectEx); + return STATUS_ACCESS_DENIED; + } + } status = mkfifo(pDeviceObjectEx->DeviceFileName, 0666); + if (status != 0) + { + free(pDeviceObjectEx->DeviceName); + free(pDeviceObjectEx->DeviceFileName); + free(pDeviceObjectEx); + switch (errno) + { + case EACCES: + return STATUS_ACCESS_DENIED; + case EEXIST: + return STATUS_OBJECT_NAME_EXISTS; + case ENAMETOOLONG: + return STATUS_NAME_TOO_LONG; + case ENOENT: + case ENOTDIR: + return STATUS_NOT_A_DIRECTORY; + case ENOSPC: + return STATUS_DISK_FULL; + default: + return STATUS_INTERNAL_ERROR; + } + } *((ULONG_PTR*) (DeviceObject)) = (ULONG_PTR) pDeviceObjectEx; diff --git a/winpr/libwinpr/io/test/TestIoDevice.c b/winpr/libwinpr/io/test/TestIoDevice.c index 97d8ea80e..d6feb633b 100644 --- a/winpr/libwinpr/io/test/TestIoDevice.c +++ b/winpr/libwinpr/io/test/TestIoDevice.c @@ -19,6 +19,9 @@ int TestIoDevice(int argc, char* argv[]) &uString, FILE_DEVICE_UNKNOWN, 0, FALSE, &pDeviceObject); + if (NtStatus != STATUS_SUCCESS) + return -1; + _IoDeleteDeviceEx(pDeviceObject); _RtlFreeUnicodeString(&uString); diff --git a/winpr/libwinpr/library/test/TestLibraryFreeLibrary.c b/winpr/libwinpr/library/test/TestLibraryFreeLibrary.c index a1df01638..a90988449 100644 --- a/winpr/libwinpr/library/test/TestLibraryFreeLibrary.c +++ b/winpr/libwinpr/library/test/TestLibraryFreeLibrary.c @@ -29,6 +29,11 @@ int TestLibraryFreeLibrary(int argc, char* argv[]) BasePath[length] = 0; #else BasePath = _strdup(str); + if (!BasePath) + { + printf("Memory allocation failed"); + return -1; + } length = strlen(BasePath); #endif diff --git a/winpr/libwinpr/library/test/TestLibraryGetProcAddress.c b/winpr/libwinpr/library/test/TestLibraryGetProcAddress.c index 52e3f6d24..ff0303f0a 100644 --- a/winpr/libwinpr/library/test/TestLibraryGetProcAddress.c +++ b/winpr/libwinpr/library/test/TestLibraryGetProcAddress.c @@ -34,6 +34,11 @@ int TestLibraryGetProcAddress(int argc, char* argv[]) BasePath[length] = 0; #else BasePath = _strdup(str); + if (!BasePath) + { + printf("Memory allocation failed"); + return -1; + } length = strlen(BasePath); #endif diff --git a/winpr/libwinpr/library/test/TestLibraryLoadLibrary.c b/winpr/libwinpr/library/test/TestLibraryLoadLibrary.c index 0b5cac353..9256a38d1 100644 --- a/winpr/libwinpr/library/test/TestLibraryLoadLibrary.c +++ b/winpr/libwinpr/library/test/TestLibraryLoadLibrary.c @@ -29,6 +29,12 @@ int TestLibraryLoadLibrary(int argc, char* argv[]) BasePath[length] = 0; #else BasePath = _strdup(str); + if (!BasePath) + { + printf("Memory allocation failed"); + return -1; + } + length = strlen(BasePath); #endif diff --git a/winpr/libwinpr/path/shell.c b/winpr/libwinpr/path/shell.c index 69841a1c1..769bea9e5 100644 --- a/winpr/libwinpr/path/shell.c +++ b/winpr/libwinpr/path/shell.c @@ -307,6 +307,7 @@ char* GetKnownPath(int id) case KNOWN_PATH_TEMP: path = GetPath_TEMP(); + break; case KNOWN_PATH_XDG_DATA_HOME: @@ -406,19 +407,39 @@ char* GetCombinedPath(const char* basePath, const char* subPath) CopyMemory(path, basePath, basePathLength); path[basePathLength] = '\0'; - PathCchConvertStyleA(path, basePathLength, PATH_STYLE_NATIVE); + if (PathCchConvertStyleA(path, basePathLength, PATH_STYLE_NATIVE) != S_OK) + { + free(path); + return NULL; + } if (!subPath) return path; subPathCpy = _strdup(subPath); - PathCchConvertStyleA(subPathCpy, subPathLength, PATH_STYLE_NATIVE); + if (!subPathCpy) + { + free(path); + return NULL; + } + if (PathCchConvertStyleA(subPathCpy, subPathLength, PATH_STYLE_NATIVE) != S_OK) + { + free(path); + free(subPathCpy); + return NULL; + } status = NativePathCchAppendA(path, length + 1, subPathCpy); free(subPathCpy); - return path; + if (status != S_OK) + { + free(path); + return NULL; + } + else + return path; } BOOL PathMakePathA(LPCSTR path, LPSECURITY_ATTRIBUTES lpAttributes) diff --git a/winpr/libwinpr/path/test/TestPathShell.c b/winpr/libwinpr/path/test/TestPathShell.c index 9887a8adf..e311f1ba8 100644 --- a/winpr/libwinpr/path/test/TestPathShell.c +++ b/winpr/libwinpr/path/test/TestPathShell.c @@ -10,24 +10,38 @@ int TestPathShell(int argc, char* argv[]) char* path; path = GetKnownPath(KNOWN_PATH_HOME); + if (!path) + return -1; printf("KNOWN_PATH_HOME: %s\n", path); path = GetKnownPath(KNOWN_PATH_TEMP); + if (!path) + return -1; printf("KNOWN_PATH_TEMP: %s\n", path); path = GetKnownPath(KNOWN_PATH_XDG_DATA_HOME); + if (!path) + return -1; printf("KNOWN_PATH_DATA: %s\n", path); path = GetKnownPath(KNOWN_PATH_XDG_CONFIG_HOME); + if (!path) + return -1; printf("KNOWN_PATH_CONFIG: %s\n", path); path = GetKnownPath(KNOWN_PATH_XDG_CACHE_HOME); + if (!path) + return -1; printf("KNOWN_PATH_CACHE: %s\n", path); path = GetKnownPath(KNOWN_PATH_XDG_RUNTIME_DIR); + if (!path) + return -1; printf("KNOWN_PATH_RUNTIME: %s\n", path); path = GetKnownSubPath(KNOWN_PATH_XDG_CONFIG_HOME, "freerdp"); + if (!path) + return -1; printf("KNOWN_PATH_CONFIG SubPath: %s\n", path); return 0; diff --git a/winpr/libwinpr/registry/registry_reg.c b/winpr/libwinpr/registry/registry_reg.c index 79e072011..3b4e00b28 100644 --- a/winpr/libwinpr/registry/registry_reg.c +++ b/winpr/libwinpr/registry/registry_reg.c @@ -169,6 +169,12 @@ static RegVal* reg_load_value(Reg* reg, RegKey* key) p[4] = strchr(data, '"'); p[4][0] = '\0'; value->data.string = _strdup(data); + if (!value->data.string) + { + free(value); + free(name); + return NULL; + } } else { @@ -226,6 +232,8 @@ static void reg_insert_key(Reg* reg, RegKey* key, RegKey* subkey) char* save; int length; path = _strdup(subkey->name); + if (!path) + return; name = strtok_s(path, "\\", &save); while (name != NULL) @@ -235,6 +243,12 @@ static void reg_insert_key(Reg* reg, RegKey* key, RegKey* subkey) length = strlen(name); name += length + 1; subkey->subname = _strdup(name); + /* TODO: free allocated memory in error case */ + if (!subkey->subname) + { + free(path); + return; + } } name = strtok_s(NULL, "\\", &save); diff --git a/winpr/libwinpr/smartcard/smartcard_pcsc.c b/winpr/libwinpr/smartcard/smartcard_pcsc.c index cd98f9b3b..61c5160b3 100644 --- a/winpr/libwinpr/smartcard/smartcard_pcsc.c +++ b/winpr/libwinpr/smartcard/smartcard_pcsc.c @@ -651,7 +651,7 @@ BOOL PCSC_AddReaderNameAlias(char* namePCSC, char* nameWinSCard) reader = (PCSC_READER*) calloc(1, sizeof(PCSC_READER)); if (!reader) - goto error_reader; + return FALSE; reader->namePCSC = _strdup(namePCSC); if (!reader->namePCSC) @@ -670,9 +670,7 @@ error_nameWinSCard: free(reader->namePCSC); error_namePSC: free(reader); -error_reader: return FALSE; - } static int PCSC_AtoiWithLength(const char* str, int length) diff --git a/winpr/libwinpr/sspi/sspi_winpr.c b/winpr/libwinpr/sspi/sspi_winpr.c index 70af02644..3b3618317 100644 --- a/winpr/libwinpr/sspi/sspi_winpr.c +++ b/winpr/libwinpr/sspi/sspi_winpr.c @@ -632,6 +632,11 @@ SECURITY_STATUS SEC_ENTRY winpr_EnumerateSecurityPackagesA(ULONG* pcPackages, PS pPackageInfo[index].cbMaxToken = SecPkgInfoA_LIST[index]->cbMaxToken; pPackageInfo[index].Name = _strdup(SecPkgInfoA_LIST[index]->Name); pPackageInfo[index].Comment = _strdup(SecPkgInfoA_LIST[index]->Comment); + if (!pPackageInfo[index].Name || !pPackageInfo[index].Comment) + { + sspi_ContextBufferFree(pPackageInfo); + return SEC_E_INSUFFICIENT_MEMORY; + } } *(pcPackages) = cPackages; @@ -729,6 +734,11 @@ SECURITY_STATUS SEC_ENTRY winpr_QuerySecurityPackageInfoA(SEC_CHAR* pszPackageNa pPackageInfo->cbMaxToken = SecPkgInfoA_LIST[index]->cbMaxToken; pPackageInfo->Name = _strdup(SecPkgInfoA_LIST[index]->Name); pPackageInfo->Comment = _strdup(SecPkgInfoA_LIST[index]->Comment); + if (!pPackageInfo->Name || !pPackageInfo->Comment) + { + sspi_ContextBufferFree(pPackageInfo); + return SEC_E_INSUFFICIENT_MEMORY; + } *(ppPackageInfo) = pPackageInfo; diff --git a/winpr/libwinpr/sspi/test/TestAcquireCredentialsHandle.c b/winpr/libwinpr/sspi/test/TestAcquireCredentialsHandle.c index e9d60ea3f..fcba1c1c4 100644 --- a/winpr/libwinpr/sspi/test/TestAcquireCredentialsHandle.c +++ b/winpr/libwinpr/sspi/test/TestAcquireCredentialsHandle.c @@ -22,10 +22,18 @@ int TestAcquireCredentialsHandle(int argc, char* argv[]) table = InitSecurityInterface(); identity.User = (UINT16*) _strdup(test_User); - identity.UserLength = sizeof(test_User); identity.Domain = (UINT16*) _strdup(test_Domain); - identity.DomainLength = sizeof(test_Domain); identity.Password = (UINT16*) _strdup(test_Password); + if (!identity.User || !identity.Domain || !identity.Password) + { + free(identity.User); + free(identity.Domain); + free(identity.Password); + fprintf(stderr, "Memory allocation failed\n"); + return -1; + } + identity.UserLength = sizeof(test_User); + identity.DomainLength = sizeof(test_Domain); identity.PasswordLength = sizeof(test_Password); identity.Flags = SEC_WINNT_AUTH_IDENTITY_ANSI; diff --git a/winpr/libwinpr/sspi/test/TestInitializeSecurityContext.c b/winpr/libwinpr/sspi/test/TestInitializeSecurityContext.c index e250a36dc..d9aa4220e 100644 --- a/winpr/libwinpr/sspi/test/TestInitializeSecurityContext.c +++ b/winpr/libwinpr/sspi/test/TestInitializeSecurityContext.c @@ -40,10 +40,19 @@ int TestInitializeSecurityContext(int argc, char* argv[]) cbMaxLen = pPackageInfo->cbMaxToken; identity.User = (UINT16*) _strdup(test_User); - identity.UserLength = sizeof(test_User); identity.Domain = (UINT16*) _strdup(test_Domain); - identity.DomainLength = sizeof(test_Domain); identity.Password = (UINT16*) _strdup(test_Password); + if (!identity.User || !identity.Domain || !identity.Password) + { + free(identity.User); + free(identity.Domain); + free(identity.Password); + fprintf(stderr, "Memory allocation failed\n"); + return -1; + } + + identity.UserLength = sizeof(test_User); + identity.DomainLength = sizeof(test_Domain); identity.PasswordLength = sizeof(test_Password); identity.Flags = SEC_WINNT_AUTH_IDENTITY_ANSI; diff --git a/winpr/libwinpr/sspicli/sspicli.c b/winpr/libwinpr/sspicli/sspicli.c index 1ea1c55c2..19cc2ed30 100644 --- a/winpr/libwinpr/sspicli/sspicli.c +++ b/winpr/libwinpr/sspicli/sspicli.c @@ -136,9 +136,22 @@ BOOL LogonUserA(LPCSTR lpszUsername, LPCSTR lpszDomain, LPCSTR lpszPassword, token->ops = &ops; token->Username = _strdup(lpszUsername); + if (!token->Username) + { + free(token); + return FALSE; + } if (lpszDomain) + { token->Domain = _strdup(lpszDomain); + if (!token->Domain) + { + free(token->Username); + free(token); + return FALSE; + } + } pw = getpwnam(lpszUsername); diff --git a/winpr/libwinpr/utils/ini.c b/winpr/libwinpr/utils/ini.c index f46e41d7b..716b8ea26 100644 --- a/winpr/libwinpr/utils/ini.c +++ b/winpr/libwinpr/utils/ini.c @@ -151,6 +151,13 @@ wIniFileKey* IniFile_Key_New(const char* name, const char* value) { key->name = _strdup(name); key->value = _strdup(value); + if (!key->name || !key->value) + { + free(key->name); + free(key->value); + free(key); + return NULL; + } } return key; @@ -278,12 +285,9 @@ wIniFileKey* IniFile_AddKey(wIniFile* ini, wIniFileSection* section, const char* { wIniFileKey* key; - if (!section) + if (!section || !name) return NULL; - if (!name) - return NULL; - key = IniFile_GetKey(ini, section, name); if (!key) @@ -302,6 +306,8 @@ wIniFileKey* IniFile_AddKey(wIniFile* ini, wIniFileSection* section, const char* } key = IniFile_Key_New(name, value); + if (!key) + return NULL; section->keys[section->nKeys] = key; section->nKeys++; } @@ -309,6 +315,8 @@ wIniFileKey* IniFile_AddKey(wIniFile* ini, wIniFileSection* section, const char* { free(key->value); key->value = _strdup(value); + if (!key->value) + return NULL; } return key; @@ -375,7 +383,11 @@ int IniFile_Load(wIniFile* ini) value = beg; - IniFile_AddKey(ini, section, name, value); + if (!IniFile_AddKey(ini, section, name, value)) + { + return -1; + } + key = NULL; if (section && section->keys) key = section->keys[section->nKeys - 1]; @@ -412,6 +424,8 @@ int IniFile_ReadFile(wIniFile* ini, const char* filename) free(ini->filename); ini->filename = _strdup(filename); + if (!ini->filename) + return -1; status = IniFile_Load_File(ini, filename); diff --git a/winpr/libwinpr/utils/wlog/BinaryAppender.c b/winpr/libwinpr/utils/wlog/BinaryAppender.c index 4bc64270d..aee2368b1 100644 --- a/winpr/libwinpr/utils/wlog/BinaryAppender.c +++ b/winpr/libwinpr/utils/wlog/BinaryAppender.c @@ -41,32 +41,32 @@ * Binary Appender */ -void WLog_BinaryAppender_SetOutputFileName(wLog* log, wLogBinaryAppender* appender, const char* filename) +BOOL WLog_BinaryAppender_SetOutputFileName(wLog* log, wLogBinaryAppender* appender, const char* filename) { - if (!appender) - return; + if (!appender || !filename) + return FALSE; if (appender->Type != WLOG_APPENDER_BINARY) - return; - - if (!filename) - return; + return FALSE; appender->FileName = _strdup(filename); + if (!appender->FileName) + return FALSE; + return TRUE; } -void WLog_BinaryAppender_SetOutputFilePath(wLog* log, wLogBinaryAppender* appender, const char* filepath) +BOOL WLog_BinaryAppender_SetOutputFilePath(wLog* log, wLogBinaryAppender* appender, const char* filepath) { - if (!appender) - return; + if (!appender || !filepath) + return FALSE; if (appender->Type != WLOG_APPENDER_BINARY) - return; - - if (!filepath) - return; + return FALSE; appender->FilePath = _strdup(filepath); + if (!appender->FilePath) + return FALSE; + return TRUE; } int WLog_BinaryAppender_Open(wLog* log, wLogBinaryAppender* appender) diff --git a/winpr/libwinpr/utils/wlog/FileAppender.c b/winpr/libwinpr/utils/wlog/FileAppender.c index 64a9239be..5b165cd12 100644 --- a/winpr/libwinpr/utils/wlog/FileAppender.c +++ b/winpr/libwinpr/utils/wlog/FileAppender.c @@ -37,32 +37,34 @@ * File Appender */ -void WLog_FileAppender_SetOutputFileName(wLog* log, wLogFileAppender* appender, const char* filename) +BOOL WLog_FileAppender_SetOutputFileName(wLog* log, wLogFileAppender* appender, const char* filename) { - if (!appender) - return; + if (!appender || !filename) + return FALSE; if (appender->Type != WLOG_APPENDER_FILE) - return; - - if (!filename) - return; + return FALSE; appender->FileName = _strdup(filename); + if (!appender->FileName) + return FALSE; + + return TRUE; } -void WLog_FileAppender_SetOutputFilePath(wLog* log, wLogFileAppender* appender, const char* filepath) +BOOL WLog_FileAppender_SetOutputFilePath(wLog* log, wLogFileAppender* appender, const char* filepath) { - if (!appender) - return; + if (!appender || !filepath) + return FALSE; if (appender->Type != WLOG_APPENDER_FILE) - return; - - if (!filepath) - return; + return FALSE; appender->FilePath = _strdup(filepath); + if (!appender->FilePath) + return FALSE; + + return TRUE; } int WLog_FileAppender_Open(wLog* log, wLogFileAppender* appender) diff --git a/winpr/libwinpr/utils/wlog/Layout.c b/winpr/libwinpr/utils/wlog/Layout.c index 8183c16bc..0df79a43f 100644 --- a/winpr/libwinpr/utils/wlog/Layout.c +++ b/winpr/libwinpr/utils/wlog/Layout.c @@ -338,13 +338,18 @@ wLogLayout* WLog_GetLogLayout(wLog* log) return appender->Layout; } -void WLog_Layout_SetPrefixFormat(wLog* log, wLogLayout* layout, const char* format) +BOOL WLog_Layout_SetPrefixFormat(wLog* log, wLogLayout* layout, const char* format) { free(layout->FormatString); layout->FormatString = NULL; if (format) + { layout->FormatString = _strdup(format); + if (!layout->FormatString) + return FALSE; + } + return TRUE; } wLogLayout* WLog_Layout_New(wLog* log) diff --git a/winpr/libwinpr/wnd/wnd.c b/winpr/libwinpr/wnd/wnd.c index e94ebc018..480ea07b5 100644 --- a/winpr/libwinpr/wnd/wnd.c +++ b/winpr/libwinpr/wnd/wnd.c @@ -58,6 +58,13 @@ WNDCLASSEXA* CloneWindowClass(CONST WNDCLASSEXA* lpwcx) _lpwcx->lpszClassName = _strdup(lpwcx->lpszClassName); _lpwcx->lpszMenuName = _strdup(lpwcx->lpszMenuName); + if (!_lpwcx->lpszClassName || !_lpwcx->lpszMenuName) + { + free((LPSTR)_lpwcx->lpszClassName); + free((LPSTR)_lpwcx->lpszMenuName); + free(_lpwcx); + return NULL; + } return _lpwcx; } @@ -233,9 +240,15 @@ HWND WINAPI CreateWindowExA(DWORD dwExStyle, LPCSTR lpClassName, pWnd->nWidth = nWidth; pWnd->nHeight = nHeight; pWnd->lpClassName = _strdup(lpClassName); + if (!pWnd->lpClassName) + goto out_fail; if (lpWindowName) + { pWnd->lpWindowName = _strdup(lpWindowName); + if (!pWnd->lpWindowName) + goto out_fail; + } pWnd->hWndParent = hWndParent; pWnd->hMenu = hMenu; @@ -244,6 +257,12 @@ HWND WINAPI CreateWindowExA(DWORD dwExStyle, LPCSTR lpClassName, pWnd->lpwcx = lpwcx; return hWnd; + +out_fail: + free(pWnd->lpClassName); + free(pWnd->lpWindowName); + free(pWnd); + return NULL; } HWND WINAPI CreateWindowExW(DWORD dwExStyle, LPCWSTR lpClassName, diff --git a/winpr/libwinpr/wtsapi/wtsapi_win32.c b/winpr/libwinpr/wtsapi/wtsapi_win32.c index b114b41bb..998ccfb18 100644 --- a/winpr/libwinpr/wtsapi/wtsapi_win32.c +++ b/winpr/libwinpr/wtsapi/wtsapi_win32.c @@ -157,6 +157,13 @@ HANDLE WINAPI Win32_WTSVirtualChannelOpen_Internal(HANDLE hServer, DWORD Session pChannel->SessionId = SessionId; pChannel->hFile = hFile; pChannel->VirtualName = _strdup(pVirtualName); + if (!pChannel->VirtualName) + { + CloseHandle(hFile); + SetLastError(ERROR_NOT_ENOUGH_MEMORY); + free(pChannel); + return NULL; + } pChannel->flags = flags; pChannel->dynamic = (flags & WTS_CHANNEL_OPTION_DYNAMIC) ? TRUE : FALSE; diff --git a/winpr/tools/makecert/cli/main.c b/winpr/tools/makecert/cli/main.c index 446442ca2..553a20cde 100644 --- a/winpr/tools/makecert/cli/main.c +++ b/winpr/tools/makecert/cli/main.c @@ -30,12 +30,16 @@ int main(int argc, char* argv[]) { MAKECERT_CONTEXT* context; + int ret = 0; context = makecert_context_new(); + if (!context) + return 1; - makecert_context_process(context, argc, argv); + if (makecert_context_process(context, argc, argv) < 0) + ret = 1; makecert_context_free(context); - return 0; + return ret; } diff --git a/winpr/tools/makecert/makecert.c b/winpr/tools/makecert/makecert.c index 17f56dc4f..476649f20 100644 --- a/winpr/tools/makecert/makecert.c +++ b/winpr/tools/makecert/makecert.c @@ -396,6 +396,8 @@ int makecert_context_parse_arguments(MAKECERT_CONTEXT* context, int argc, char** context->pemFormat = FALSE; context->pfxFormat = TRUE; } + else + return -1; } CommandLineSwitchCase(arg, "path") { @@ -403,6 +405,8 @@ int makecert_context_parse_arguments(MAKECERT_CONTEXT* context, int argc, char** continue; context->output_path = _strdup(arg->Value); + if (!context->output_path) + return -1; } CommandLineSwitchCase(arg, "p") { @@ -410,6 +414,8 @@ int makecert_context_parse_arguments(MAKECERT_CONTEXT* context, int argc, char** continue; context->password = _strdup(arg->Value); + if (!context->password) + return -1; } CommandLineSwitchCase(arg, "n") { @@ -417,6 +423,8 @@ int makecert_context_parse_arguments(MAKECERT_CONTEXT* context, int argc, char** continue; context->common_name = _strdup(arg->Value); + if (!context->common_name) + return -1; } CommandLineSwitchCase(arg, "y") { @@ -450,17 +458,22 @@ int makecert_context_set_output_file_name(MAKECERT_CONTEXT* context, char* name) { free(context->output_file); context->output_file = _strdup(name); + if (!context->output_file) + return -1; return 1; } int makecert_context_output_certificate_file(MAKECERT_CONTEXT* context, char* path) { - FILE* fp; + FILE* fp = NULL; int status; int length; int offset; - char* filename; - char* fullpath; + char* filename = NULL; + char* fullpath = NULL; + int ret = -1; + BIO* bio = NULL; + BYTE* x509_str = NULL; if (!context->output_file) { @@ -491,18 +504,21 @@ int makecert_context_output_certificate_file(MAKECERT_CONTEXT* context, char* pa else fullpath = _strdup(filename); + if (!fullpath) + goto out_fail; + fp = fopen(fullpath, "w+"); if (fp) { - BIO* bio; - BYTE* x509_str = NULL; if (context->pfxFormat) { if (!context->password) { context->password = _strdup("password"); + if (!context->password) + goto out_fail; printf("Using default export password \"password\"\n"); } @@ -512,41 +528,29 @@ int makecert_context_output_certificate_file(MAKECERT_CONTEXT* context, char* pa context->pkcs12 = PKCS12_create(context->password, context->default_name, context->pkey, context->x509, NULL, 0, 0, 0, 0, 0); + if (!context->pkcs12) + goto out_fail; bio = BIO_new(BIO_s_mem()); if (!bio) - { - free(filename); - free(fullpath); - fclose (fp); - return -1; - } + goto out_fail; status = i2d_PKCS12_bio(bio, context->pkcs12); + if (status != 1) + goto out_fail; offset = 0; length = 2048; x509_str = (BYTE*) malloc(length); if (!x509_str) - { - free(filename); - free(fullpath); - fclose (fp); - return -1; - } + goto out_fail; status = BIO_read(bio, x509_str, length); if (status < 0) - { - free(x509_str); - free(filename); - free(fullpath); - fclose (fp); - return -1; - } - + goto out_fail; + offset += status; while (offset >= length) @@ -574,57 +578,33 @@ int makecert_context_output_certificate_file(MAKECERT_CONTEXT* context, char* pa } if (status < 0) - { - free(x509_str); - free(filename); - free(fullpath); - fclose (fp); - return -1; - } + goto out_fail; length = offset; fwrite((void*) x509_str, length, 1, fp); - free(x509_str); - BIO_free(bio); } else { bio = BIO_new(BIO_s_mem()); if (!bio) - { - free(filename); - free(fullpath); - fclose (fp); - return -1; - } + goto out_fail; - status = PEM_write_bio_X509(bio, context->x509); + if (!PEM_write_bio_X509(bio, context->x509)) + goto out_fail; offset = 0; length = 2048; x509_str = (BYTE*) malloc(length); if (!x509_str) - { - BIO_free(bio); - free(filename); - free(fullpath); - fclose (fp); - return -1; - } + goto out_fail; status = BIO_read(bio, x509_str, length); if (status < 0) - { - BIO_free(bio); - free(filename); - free(fullpath); - fclose (fp); - return -1; - } - + goto out_fail; + offset += status; while (offset >= length) @@ -652,58 +632,36 @@ int makecert_context_output_certificate_file(MAKECERT_CONTEXT* context, char* pa } if (status < 0) - { - free (x509_str); - free(filename); - free(fullpath); - fclose (fp); - return -1; - } - + goto out_fail; + length = offset; fwrite((void*) x509_str, length, 1, fp); free(x509_str); + x509_str = NULL; BIO_free(bio); + bio = NULL; if (context->pemFormat) { bio = BIO_new(BIO_s_mem()); if (!bio) - { - free(filename); - free(fullpath); - fclose (fp); - return -1; - } + goto out_fail; status = PEM_write_bio_PrivateKey(bio, context->pkey, NULL, NULL, 0, NULL, NULL); offset = 0; length = 2048; if (!(x509_str = (BYTE*) malloc(length))) - { - BIO_free(bio); - free(filename); - free(fullpath); - fclose (fp); - return -1; - - } + goto out_fail; status = BIO_read(bio, x509_str, length); if (status < 0) - { - free(x509_str); - free(filename); - free(fullpath); - fclose (fp); - return -1; - } - + goto out_fail; + offset += status; while (offset >= length) @@ -731,30 +689,27 @@ int makecert_context_output_certificate_file(MAKECERT_CONTEXT* context, char* pa } if (status < 0) - { - free(x509_str); - free(filename); - free(fullpath); - fclose (fp); - return -1; - } - + goto out_fail; + length = offset; fwrite((void*) x509_str, length, 1, fp); - - free(x509_str); - BIO_free(bio); } } - fclose(fp); } + ret = 1; +out_fail: + if (bio) + BIO_free(bio); + if (fp) + fclose(fp); + free(x509_str); free(filename); free(fullpath); - return 1; + return ret; } int makecert_context_output_private_key_file(MAKECERT_CONTEXT* context, char* path) @@ -789,6 +744,12 @@ int makecert_context_output_private_key_file(MAKECERT_CONTEXT* context, char* pa else fullpath = _strdup(filename); + if (!fullpath) + { + free(filename); + return -1; + } + fp = fopen(fullpath, "w+"); if (fp) @@ -888,17 +849,31 @@ int makecert_context_process(MAKECERT_CONTEXT* context, int argc, char** argv) X509_NAME* name = NULL; const EVP_MD* md = NULL; COMMAND_LINE_ARGUMENT_A* arg; + int ret; - if (makecert_context_parse_arguments(context, argc, argv) < 1) - return 0; + ret = makecert_context_parse_arguments(context, argc, argv); + if (ret < 1) + return ret; if (!context->default_name && !context->common_name) + { context->default_name = x509_get_default_name(); + if (!context->default_name) + return -1; + } else + { context->default_name = _strdup(context->common_name); + if (!context->default_name) + return -1; + } if (!context->common_name) + { context->common_name = _strdup(context->default_name); + if (!context->common_name) + return -1; + } if (!context->pkey) context->pkey = EVP_PKEY_new(); @@ -1137,6 +1112,8 @@ void makecert_context_free(MAKECERT_CONTEXT* context) free(context->default_name); free(context->common_name); + free(context->output_file); + free(context->output_path); CRYPTO_cleanup_all_ex_data();