ConvertFromUnicode fixes and misc hardening
authorNorbert Federa <norbert.federa@thincast.com>
Thu, 3 Mar 2016 15:21:12 +0000 (16:21 +0100)
committerNorbert Federa <norbert.federa@thincast.com>
Thu, 3 Mar 2016 15:56:19 +0000 (16:56 +0100)
- Added missing ConvertFromUnicode checks
- If ConvertToUnicode allocates memory, guarantee the null termination
  similar to ConvertFromUnicode's implementation
- Fixed some TestUnicodeConversion.c CTest return values
- Added some CTests for ConvertFromUnicode and ConvertToUnicode
- Misc code and protocol hardening fixes in the surrounding code regions
  that have been touched

15 files changed:
channels/cliprdr/client/cliprdr_format.c
channels/cliprdr/server/cliprdr_main.c
channels/echo/server/echo_main.c
channels/rdpdr/server/rdpdr_main.c
channels/smartcard/client/smartcard_pack.c
client/Windows/wf_rail.c
client/X11/xf_rail.c
libfreerdp/core/gcc.c
libfreerdp/core/info.c
libfreerdp/core/redirection.c
libfreerdp/core/window.c
server/shadow/Win/win_wds.c
winpr/libwinpr/crt/test/TestUnicodeConversion.c
winpr/libwinpr/crt/unicode.c
winpr/libwinpr/file/generic.c

index e686a7c..a7afae5 100644 (file)
@@ -102,16 +102,25 @@ UINT cliprdr_process_format_list(cliprdrPlugin* cliprdr, wStream* s, UINT32 data
 
                        formats[index].formatName = NULL;
 
+                       /* According to MS-RDPECLIP 2.2.3.1.1.1 formatName is "a 32-byte block containing
+                        * the *null-terminated* name assigned to the Clipboard Format: (32 ASCII 8 characters
+                        * or 16 Unicode characters)"
+                        * However, both Windows RDSH and mstsc violate this specs as seen in the following
+                        * example of a transferred short format name string: [R.i.c.h. .T.e.x.t. .F.o.r.m.a.t.]
+                        * These are 16 unicode charaters - *without* terminating null !
+                        */
+
                        if (asciiNames)
                        {
                                szFormatName = (char*) Stream_Pointer(s);
 
                                if (szFormatName[0])
                                {
+                                       /* ensure null termination */
                                        formats[index].formatName = (char*) malloc(32 + 1);
                                        if (!formats[index].formatName)
                                        {
-                                               WLog_ERR(TAG, "calloc failed!");
+                                               WLog_ERR(TAG, "malloc failed!");
                                                error = CHANNEL_RC_NO_MEMORY;
                                                goto error_out;
                                        }
@@ -125,8 +134,16 @@ UINT cliprdr_process_format_list(cliprdrPlugin* cliprdr, wStream* s, UINT32 data
 
                                if (wszFormatName[0])
                                {
-                                       ConvertFromUnicode(CP_UTF8, 0, wszFormatName,
-                                               16, &(formats[index].formatName), 0, NULL, NULL);
+                                       /* ConvertFromUnicode always returns a null-terminated
+                                        * string on success, even if the source string isn't.
+                                        */
+                                       if (ConvertFromUnicode(CP_UTF8, 0, wszFormatName, 16,
+                                               &(formats[index].formatName), 0, NULL, NULL) < 1)
+                                       {
+                                               WLog_ERR(TAG, "failed to convert short clipboard format name");
+                                               error = ERROR_INTERNAL_ERROR;
+                                               goto error_out;
+                                       }
                                }
                        }
 
@@ -185,8 +202,13 @@ UINT cliprdr_process_format_list(cliprdrPlugin* cliprdr, wStream* s, UINT32 data
 
                        if (formatNameLength)
                        {
-                               ConvertFromUnicode(CP_UTF8, 0, wszFormatName,
-                                       -1, &(formats[index].formatName), 0, NULL, NULL);
+                               if (ConvertFromUnicode(CP_UTF8, 0, wszFormatName, -1,
+                                       &(formats[index].formatName), 0, NULL, NULL) < 1)
+                               {
+                                       WLog_ERR(TAG, "failed to convert long clipboard format name");
+                                       error = ERROR_INTERNAL_ERROR;
+                                       goto error_out;
+                               }
                        }
 
                        Stream_Seek(s, (formatNameLength + 1) * 2);
index 24a34c5..ed98170 100644 (file)
@@ -597,8 +597,12 @@ static UINT cliprdr_server_receive_temporary_directory(CliprdrServerContext* con
        free(cliprdr->temporaryDirectory);
        cliprdr->temporaryDirectory = NULL;
 
-       ConvertFromUnicode(CP_UTF8, 0, wszTempDir, -1,
-                       &(cliprdr->temporaryDirectory), 0, NULL, NULL);
+       if (ConvertFromUnicode(CP_UTF8, 0, wszTempDir, -1,
+               &(cliprdr->temporaryDirectory), 0, NULL, NULL) < 1)
+       {
+               WLog_ERR(TAG, "failed to convert temporary directory name");
+               return ERROR_INVALID_DATA;
+       }
 
        length = strlen(cliprdr->temporaryDirectory);
 
@@ -680,12 +684,21 @@ static UINT cliprdr_server_receive_format_list(CliprdrServerContext* context, wS
 
                        formats[index].formatName = NULL;
 
+                       /* According to MS-RDPECLIP 2.2.3.1.1.1 formatName is "a 32-byte block containing
+                        * the *null-terminated* name assigned to the Clipboard Format: (32 ASCII 8 characters
+                        * or 16 Unicode characters)"
+                        * However, both Windows RDSH and mstsc violate this specs as seen in the following
+                        * example of a transferred short format name string: [R.i.c.h. .T.e.x.t. .F.o.r.m.a.t.]
+                        * These are 16 unicode charaters - *without* terminating null !
+                        */
+
                        if (asciiNames)
                        {
                                szFormatName = (char*) Stream_Pointer(s);
 
                                if (szFormatName[0])
                                {
+                                       /* ensure null termination */
                                        formats[index].formatName = (char*) malloc(32 + 1);
                                        CopyMemory(formats[index].formatName, szFormatName, 32);
                                        formats[index].formatName[32] = '\0';
@@ -697,8 +710,16 @@ static UINT cliprdr_server_receive_format_list(CliprdrServerContext* context, wS
 
                                if (wszFormatName[0])
                                {
-                                       ConvertFromUnicode(CP_UTF8, 0, wszFormatName,
-                                               16, &(formats[index].formatName), 0, NULL, NULL);
+                                       /* ConvertFromUnicode always returns a null-terminated
+                                        * string on success, even if the source string isn't.
+                                        */
+                                       if (ConvertFromUnicode(CP_UTF8, 0, wszFormatName, 16,
+                                               &(formats[index].formatName), 0, NULL, NULL) < 1)
+                                       {
+                                               WLog_ERR(TAG, "failed to convert short clipboard format name");
+                                               error = ERROR_INVALID_DATA;
+                                               goto out;
+                                       }
                                }
                        }
 
@@ -757,8 +778,13 @@ static UINT cliprdr_server_receive_format_list(CliprdrServerContext* context, wS
 
                        if (formatNameLength)
                        {
-                               ConvertFromUnicode(CP_UTF8, 0, wszFormatName,
-                                       -1, &(formats[index].formatName), 0, NULL, NULL);
+                               if (ConvertFromUnicode(CP_UTF8, 0, wszFormatName, -1,
+                                       &(formats[index].formatName), 0, NULL, NULL) < 1)
+                               {
+                                       WLog_ERR(TAG, "failed to convert long clipboard format name");
+                                       error = ERROR_INVALID_DATA;
+                                       goto out;
+                               }
                        }
 
                        Stream_Seek(s, (formatNameLength + 1) * 2);
@@ -775,6 +801,7 @@ static UINT cliprdr_server_receive_format_list(CliprdrServerContext* context, wS
        if (error)
                WLog_ERR(TAG, "ClientFormatList failed with error %lu!", error);
 
+out:
        for (index = 0; index < formatList.numFormats; index++)
        {
                free(formatList.formats[index].formatName);
index 58cc812..8c7f921 100644 (file)
@@ -116,12 +116,15 @@ static void* echo_server_thread_func(void* arg)
        DWORD BytesReturned = 0;
        echo_server* echo = (echo_server*) arg;
        UINT error;
-    DWORD status;
+       DWORD status;
 
        if ((error = echo_server_open_channel(echo)))
        {
-               IFCALLRET(echo->context.OpenResult, error, &echo->context, ECHO_SERVER_OPEN_RESULT_NOTSUPPORTED);
+               UINT error2 = 0;
                WLog_ERR(TAG, "echo_server_open_channel failed with error %lu!", error);
+               IFCALLRET(echo->context.OpenResult, error2, &echo->context, ECHO_SERVER_OPEN_RESULT_NOTSUPPORTED);
+               if (error2)
+                       WLog_ERR(TAG, "echo server's OpenResult callback failed with error %lu", error2);
                goto out;
        }
 
@@ -145,14 +148,14 @@ static void* echo_server_thread_func(void* arg)
 
        while (1)
        {
-        status = WaitForMultipleObjects(nCount, events, FALSE, 100);
+               status = WaitForMultipleObjects(nCount, events, FALSE, 100);
 
-        if (status == WAIT_FAILED)
-        {
-            error = GetLastError();
-            WLog_ERR(TAG, "WaitForMultipleObjects failed with error %lu", error);
-            break;
-        }
+               if (status == WAIT_FAILED)
+               {
+                       error = GetLastError();
+                       WLog_ERR(TAG, "WaitForMultipleObjects failed with error %lu", error);
+                       break;
+               }
 
                if (status == WAIT_OBJECT_0)
                {
index 074d3e4..b29182f 100644 (file)
@@ -152,9 +152,10 @@ static UINT rdpdr_server_receive_client_name_request(RdpdrServerContext* context
        Stream_Seek_UINT32(s); /* CodePage (4 bytes), MUST be set to zero */
        Stream_Read_UINT32(s, ComputerNameLen); /* ComputerNameLen (4 bytes) */
 
-       if (Stream_GetRemainingLength(s) < ComputerNameLen)
+
+       if (UnicodeFlag > 1) /* must be 0x00000000 or 0x00000001 */
        {
-               WLog_ERR(TAG, "not enough data in stream!");
+               WLog_ERR(TAG, "invalid UnicodeFlag value: 0x%08X", UnicodeFlag);
                return ERROR_INVALID_DATA;
        }
 
@@ -164,6 +165,39 @@ static UINT rdpdr_server_receive_client_name_request(RdpdrServerContext* context
         * not in characters, including the NULL terminator!
         */
 
+       if (UnicodeFlag)
+       {
+               if ((ComputerNameLen % 2) || ComputerNameLen > 512 || ComputerNameLen < 2)
+               {
+                       WLog_ERR(TAG, "invalid unicode computer name length: %u", ComputerNameLen);
+                       return ERROR_INVALID_DATA;
+               }
+       }
+       else
+       {
+               if (ComputerNameLen > 256 || ComputerNameLen < 1)
+               {
+                       WLog_ERR(TAG, "invalid ascii computer name length: %u", ComputerNameLen);
+                       return ERROR_INVALID_DATA;
+               }
+       }
+
+       if (Stream_GetRemainingLength(s) < ComputerNameLen)
+       {
+               WLog_ERR(TAG, "not enough data in stream!");
+               return ERROR_INVALID_DATA;
+       }
+
+       /* ComputerName must be null terminated, check if it really is */
+
+       if (Stream_Pointer(s)[ComputerNameLen-1] ||
+               (UnicodeFlag && Stream_Pointer(s)[ComputerNameLen-2]))
+       {
+               WLog_ERR(TAG, "computer name must be null terminated");
+               return ERROR_INVALID_DATA;
+       }
+
+
        if (context->priv->ClientComputerName)
        {
                free(context->priv->ClientComputerName);
@@ -172,12 +206,21 @@ static UINT rdpdr_server_receive_client_name_request(RdpdrServerContext* context
 
        if (UnicodeFlag)
        {
-               ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) Stream_Pointer(s),
-                               -1, &(context->priv->ClientComputerName), 0, NULL, NULL);
+               if (ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) Stream_Pointer(s), -1,
+                       &(context->priv->ClientComputerName), 0, NULL, NULL) < 1)
+               {
+                       WLog_ERR(TAG, "failed to convert client computer name");
+                       return ERROR_INVALID_DATA;
+               }
        }
        else
        {
                context->priv->ClientComputerName = _strdup((char*) Stream_Pointer(s));
+               if (!context->priv->ClientComputerName)
+               {
+                       WLog_ERR(TAG, "failed to duplicate client computer name");
+                       return CHANNEL_RC_NO_MEMORY;
+               }
        }
 
        Stream_Seek(s, ComputerNameLen);
index 421cc61..766648b 100644 (file)
@@ -707,7 +707,12 @@ void smartcard_trace_list_readers_return(SMARTCARD_DEVICE* smartcard, ListReader
        if (unicode)
        {
                length = ret->cBytes / 2;
-               ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) ret->msz, (int)length, &mszA, 0, NULL, NULL);
+               if (ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) ret->msz, (int)length,
+                       &mszA, 0, NULL, NULL) < 1)
+               {
+                       WLog_ERR(TAG, "ConvertFromUnicode failed");
+                       return;
+               }
        }
        else
        {
@@ -1767,7 +1772,12 @@ void smartcard_trace_status_return(SMARTCARD_DEVICE* smartcard, Status_Return* r
        if (unicode)
        {
                length = ret->cBytes / 2;
-               ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) ret->mszReaderNames, (int)length, &mszReaderNamesA, 0, NULL, NULL);
+               if (ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) ret->mszReaderNames, (int)length,
+                       &mszReaderNamesA, 0, NULL, NULL) < 1)
+               {
+                       WLog_ERR(TAG, "ConvertFromUnicode failed");
+                       return;
+               }
        }
        else
        {
index c00a465..9ee65ce 100644 (file)
@@ -456,14 +456,33 @@ static BOOL wf_rail_window_common(rdpContext* context, WINDOW_ORDER_INFO* orderI
                {
                        char* title = NULL;
 
-                       ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) windowState->titleInfo.string,
-                                  windowState->titleInfo.length / 2, &title, 0, NULL, NULL);
+                       if (windowState->titleInfo.length == 0)
+                       {
+                               if (!(title = _strdup("")))
+                               {
+                                       WLog_ERR(TAG, "failed to duplicate empty window title string");
+                                       /* error handled below */
+                               }
+                       }
+                       else if (ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) windowState->titleInfo.string,
+                                  windowState->titleInfo.length / 2, &title, 0, NULL, NULL) < 1)
+                       {
+                               WLog_ERR(TAG, "failed to convert window title");
+                               /* error handled below */
+                       }
 
                        railWindow->title = title;
                }
                else
                {
-                       railWindow->title = _strdup("RdpRailWindow");
+                       if (!(railWindow->title = _strdup("RdpRailWindow")))
+                               WLog_ERR(TAG, "failed to duplicate default window title string");
+               }
+
+               if (!railWindow->title)
+               {
+                       free(railWindow);
+                       return FALSE;
                }
 
                ConvertToUnicode(CP_UTF8, 0, railWindow->title, -1, &titleW, 0);
@@ -569,8 +588,20 @@ static BOOL wf_rail_window_common(rdpContext* context, WINDOW_ORDER_INFO* orderI
                char* title = NULL;
                WCHAR* titleW = NULL;
 
-               ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) windowState->titleInfo.string,
-                          windowState->titleInfo.length / 2, &title, 0, NULL, NULL);
+               if (windowState->titleInfo.length == 0)
+               {
+                       if (!(title = _strdup("")))
+                       {
+                               WLog_ERR(TAG, "failed to duplicate empty window title string");
+                               return FALSE;
+                       }
+               }
+               else if (ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) windowState->titleInfo.string,
+                       windowState->titleInfo.length / 2, &title, 0, NULL, NULL) < 1)
+               {
+                       WLog_ERR(TAG, "failed to convert window title");
+                       return FALSE;
+               }
 
                free(railWindow->title);
                railWindow->title = title;
index 22a941c..6b49d56 100644 (file)
@@ -290,15 +290,29 @@ static BOOL xf_rail_window_common(rdpContext* context, WINDOW_ORDER_INFO* orderI
                {
                        char* title = NULL;
 
-                       ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) windowState->titleInfo.string,
-                                  windowState->titleInfo.length / 2, &title, 0, NULL, NULL);
+                       if (windowState->titleInfo.length == 0)
+                       {
+                               if (!(title = _strdup("")))
+                               {
+                                       WLog_ERR(TAG, "failed to duplicate empty window title string");
+                                       /* error handled below */
+                               }
+                       }
+                       else if (ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) windowState->titleInfo.string,
+                               windowState->titleInfo.length / 2, &title, 0, NULL, NULL) < 1)
+                       {
+                               WLog_ERR(TAG, "failed to convert window title");
+                               /* error handled below */
+                       }
 
                        appWindow->title = title;
                }
                else
                {
-                       appWindow->title = _strdup("RdpRailWindow");
+                       if (!(appWindow->title = _strdup("RdpRailWindow")))
+                               WLog_ERR(TAG, "failed to duplicate default window title string");
                }
+
                if (!appWindow->title)
                {
                        free(appWindow);
@@ -365,9 +379,20 @@ static BOOL xf_rail_window_common(rdpContext* context, WINDOW_ORDER_INFO* orderI
        {
                char* title = NULL;
 
-               ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) windowState->titleInfo.string,
-                          windowState->titleInfo.length / 2, &title, 0, NULL, NULL);
-
+               if (windowState->titleInfo.length == 0)
+               {
+                       if (!(title = _strdup("")))
+                       {
+                               WLog_ERR(TAG, "failed to duplicate empty window title string");
+                               return FALSE;
+                       }
+               }
+               else if (ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) windowState->titleInfo.string,
+                       windowState->titleInfo.length / 2, &title, 0, NULL, NULL) < 1)
+               {
+                       WLog_ERR(TAG, "failed to convert window title");
+                       return FALSE;
+               }
                free(appWindow->title);
                appWindow->title = title;
        }
index 7078a1d..e19e019 100644 (file)
@@ -590,7 +590,12 @@ BOOL gcc_read_client_core_data(wStream* s, rdpMcs* mcs, UINT16 blockLength)
        Stream_Read_UINT32(s, settings->ClientBuild); /* ClientBuild (4 bytes) */
 
        /* clientName (32 bytes, null-terminated unicode, truncated to 15 characters) */
-       ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) Stream_Pointer(s), 32 / 2, &str, 0, NULL, NULL);
+       if (ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) Stream_Pointer(s), 32 / 2,
+               &str, 0, NULL, NULL) < 1)
+       {
+               WLog_ERR(TAG, "failed to convert client host name");
+               return FALSE;
+       }
        Stream_Seek(s, 32);
        free(settings->ClientHostname);
        settings->ClientHostname = str;
@@ -644,10 +649,17 @@ BOOL gcc_read_client_core_data(wStream* s, rdpMcs* mcs, UINT16 blockLength)
                settings->EarlyCapabilityFlags = (UINT32) earlyCapabilityFlags;
                blockLength -= 2;
 
+               /* clientDigProductId (64 bytes): Contains a value that uniquely identifies the client */
+
                if (blockLength < 64)
                        break;
 
-               ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) Stream_Pointer(s), 64 / 2, &str, 0, NULL, NULL);
+               if (ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) Stream_Pointer(s), 64 / 2,
+                       &str, 0, NULL, NULL) < 1)
+               {
+                       WLog_ERR(TAG, "failed to convert the client product identifier");
+                       return FALSE;
+               }
                Stream_Seek(s, 64); /* clientDigProductId (64 bytes) */
                free(settings->ClientProductId);
                settings->ClientProductId = str;
index 25fe940..6a918c4 100644 (file)
@@ -199,6 +199,7 @@ BOOL rdp_read_extended_info_packet(rdpRdp* rdp, wStream* s)
        UINT16 cbClientDir;
        UINT16 cbAutoReconnectLen;
        rdpSettings* settings = rdp->settings;
+       WCHAR* wstr;
 
        if (Stream_GetRemainingLength(s) < 4)
                return FALSE;
@@ -206,6 +207,17 @@ BOOL rdp_read_extended_info_packet(rdpRdp* rdp, wStream* s)
        Stream_Read_UINT16(s, clientAddressFamily); /* clientAddressFamily (2 bytes) */
        Stream_Read_UINT16(s, cbClientAddress); /* cbClientAddress (2 bytes) */
 
+       /* cbClientAddress is the size in bytes of the character data in the clientAddress field.
+        * This size includes the length of the mandatory null terminator.
+        * The maximum allowed value is 80 bytes
+        */
+
+       if ((cbClientAddress % 2) || cbClientAddress < 2 || cbClientAddress > 80)
+       {
+               WLog_ERR(TAG, "protocol error: invalid cbClientAddress value: %u", cbClientAddress);
+               return FALSE;
+       }
+
        settings->IPv6Enabled = (clientAddressFamily == ADDRESS_FAMILY_INET6 ? TRUE : FALSE);
 
        if (Stream_GetRemainingLength(s) < cbClientAddress)
@@ -217,7 +229,17 @@ BOOL rdp_read_extended_info_packet(rdpRdp* rdp, wStream* s)
                settings->ClientAddress = NULL;
        }
 
-       ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) Stream_Pointer(s), cbClientAddress / 2, &settings->ClientAddress, 0, NULL, NULL);
+       wstr = (WCHAR*) Stream_Pointer(s);
+       if (wstr[cbClientAddress / 2 - 1])
+       {
+               WLog_ERR(TAG, "protocol error: clientAddress must be null terminated");
+               return FALSE;
+       }
+       if (ConvertFromUnicode(CP_UTF8, 0, wstr, -1, &settings->ClientAddress, 0, NULL, NULL) < 1)
+       {
+               WLog_ERR(TAG, "failed to convert client address");
+               return FALSE;
+       }
        Stream_Seek(s, cbClientAddress);
 
        if (Stream_GetRemainingLength(s) < 2)
@@ -225,6 +247,17 @@ BOOL rdp_read_extended_info_packet(rdpRdp* rdp, wStream* s)
 
        Stream_Read_UINT16(s, cbClientDir); /* cbClientDir (2 bytes) */
 
+       /* cbClientDir is the size in bytes of the character data in the clientDir field.
+        * This size includes the length of the mandatory null terminator.
+        * The maximum allowed value is 512 bytes
+        */
+
+       if ((cbClientDir % 2) || cbClientDir < 2 || cbClientDir > 512)
+       {
+               WLog_ERR(TAG, "protocol error: invalid cbClientDir value: %u", cbClientDir);
+               return FALSE;
+       }
+
        if (Stream_GetRemainingLength(s) < cbClientDir)
                return FALSE;
 
@@ -234,7 +267,17 @@ BOOL rdp_read_extended_info_packet(rdpRdp* rdp, wStream* s)
                settings->ClientDir = NULL;
        }
 
-       ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) Stream_Pointer(s), cbClientDir / 2, &settings->ClientDir, 0, NULL, NULL);
+       wstr = (WCHAR*) Stream_Pointer(s);
+       if (wstr[cbClientDir / 2 - 1])
+       {
+               WLog_ERR(TAG, "protocol error: clientDir must be null terminated");
+               return FALSE;
+       }
+       if (ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) Stream_Pointer(s), -1, &settings->ClientDir, 0, NULL, NULL) < 1)
+       {
+               WLog_ERR(TAG, "failed to convert client directory");
+               return FALSE;
+       }
        Stream_Seek(s, cbClientDir);
 
        if (!rdp_read_client_time_zone(s, settings))
@@ -337,6 +380,7 @@ BOOL rdp_read_info_packet(rdpRdp* rdp, wStream* s)
        UINT16 cbWorkingDir;
        UINT32 CompressionLevel;
        rdpSettings* settings = rdp->settings;
+       WCHAR* wstr;
 
        if (Stream_GetRemainingLength(s) < 18)
                return FALSE;
@@ -357,6 +401,12 @@ BOOL rdp_read_info_packet(rdpRdp* rdp, wStream* s)
                settings->CompressionLevel = CompressionLevel;
        }
 
+       if (!(flags & INFO_UNICODE))
+       {
+               WLog_ERR(TAG, "Client without INFO_UNICODE flag: this is currently not supported");
+               return FALSE;
+       }
+
        Stream_Read_UINT16(s, cbDomain); /* cbDomain (2 bytes) */
        Stream_Read_UINT16(s, cbUserName); /* cbUserName (2 bytes) */
        Stream_Read_UINT16(s, cbPassword); /* cbPassword (2 bytes) */
@@ -368,7 +418,26 @@ BOOL rdp_read_info_packet(rdpRdp* rdp, wStream* s)
 
        if (cbDomain > 0)
        {
-               ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) Stream_Pointer(s), cbDomain / 2, &settings->Domain, 0, NULL, NULL);
+               /* cbDomain is the size in bytes of the character data in the Domain field.
+                * This size excludes (!) the length of the mandatory null terminator.
+                * Maximum value including the mandatory null terminator: 512
+                */
+               if ((cbDomain % 2) || cbDomain > 512)
+               {
+                       WLog_ERR(TAG, "protocol error: invalid cbDomain value: %u", cbDomain);
+                       return FALSE;
+               }
+               wstr = (WCHAR*) Stream_Pointer(s);
+               if (wstr[cbDomain / 2])
+               {
+                       WLog_ERR(TAG, "protocol error: Domain must be null terminated");
+                       return FALSE;
+               }
+               if (ConvertFromUnicode(CP_UTF8, 0, wstr, -1, &settings->Domain, 0, NULL, NULL) < 1)
+               {
+                       WLog_ERR(TAG, "failed to convert Domain string");
+                       return FALSE;
+               }
                Stream_Seek(s, cbDomain);
        }
        Stream_Seek(s, 2);
@@ -378,7 +447,26 @@ BOOL rdp_read_info_packet(rdpRdp* rdp, wStream* s)
 
        if (cbUserName > 0)
        {
-               ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) Stream_Pointer(s), cbUserName / 2, &settings->Username, 0, NULL, NULL);
+               /* cbUserName is the size in bytes of the character data in the UserName field.
+                * This size excludes (!) the length of the mandatory null terminator.
+                * Maximum value including the mandatory null terminator: 512
+                */
+               if ((cbUserName % 2) || cbUserName > 512)
+               {
+                       WLog_ERR(TAG, "protocol error: invalid cbUserName value: %u", cbUserName);
+                       return FALSE;
+               }
+               wstr = (WCHAR*) Stream_Pointer(s);
+               if (wstr[cbUserName / 2])
+               {
+                       WLog_ERR(TAG, "protocol error: UserName must be null terminated");
+                       return FALSE;
+               }
+               if (ConvertFromUnicode(CP_UTF8, 0, wstr, -1, &settings->Username, 0, NULL, NULL) < 1)
+               {
+                       WLog_ERR(TAG, "failed to convert UserName string");
+                       return FALSE;
+               }
                Stream_Seek(s, cbUserName);
        }
        Stream_Seek(s, 2);
@@ -388,7 +476,26 @@ BOOL rdp_read_info_packet(rdpRdp* rdp, wStream* s)
 
        if (cbPassword > 0)
        {
-               ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) Stream_Pointer(s), cbPassword / 2, &settings->Password, 0, NULL, NULL);
+               /* cbPassword is the size in bytes of the character data in the Password field.
+                * This size excludes (!) the length of the mandatory null terminator.
+                * Maximum value including the mandatory null terminator: 512
+                */
+               if ((cbPassword % 2) || cbPassword > 512)
+               {
+                       WLog_ERR(TAG, "protocol error: invalid cbPassword value: %u", cbPassword);
+                       return FALSE;
+               }
+               wstr = (WCHAR*) Stream_Pointer(s);
+               if (wstr[cbPassword / 2])
+               {
+                       WLog_ERR(TAG, "protocol error: Password must be null terminated");
+                       return FALSE;
+               }
+               if (ConvertFromUnicode(CP_UTF8, 0, wstr, -1, &settings->Password, 0, NULL, NULL) < 1)
+               {
+                       WLog_ERR(TAG, "failed to convert Password string");
+                       return FALSE;
+               }
                Stream_Seek(s, cbPassword);
        }
        Stream_Seek(s, 2);
@@ -398,7 +505,26 @@ BOOL rdp_read_info_packet(rdpRdp* rdp, wStream* s)
 
        if (cbAlternateShell > 0)
        {
-               ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) Stream_Pointer(s), cbAlternateShell / 2, &settings->AlternateShell, 0, NULL, NULL);
+               /* cbAlternateShell is the size in bytes of the character data in the AlternateShell field.
+                * This size excludes (!) the length of the mandatory null terminator.
+                * Maximum value including the mandatory null terminator: 512
+                */
+               if ((cbAlternateShell % 2) || cbAlternateShell > 512)
+               {
+                       WLog_ERR(TAG, "protocol error: invalid cbAlternateShell value: %u", cbAlternateShell);
+                       return FALSE;
+               }
+               wstr = (WCHAR*) Stream_Pointer(s);
+               if (wstr[cbAlternateShell / 2])
+               {
+                       WLog_ERR(TAG, "protocol error: AlternateShell must be null terminated");
+                       return FALSE;
+               }
+               if (ConvertFromUnicode(CP_UTF8, 0, wstr, -1, &settings->AlternateShell, 0, NULL, NULL) < 1)
+               {
+                       WLog_ERR(TAG, "failed to convert AlternateShell string");
+                       return FALSE;
+               }
                Stream_Seek(s, cbAlternateShell);
        }
        Stream_Seek(s, 2);
@@ -408,7 +534,26 @@ BOOL rdp_read_info_packet(rdpRdp* rdp, wStream* s)
 
        if (cbWorkingDir > 0)
        {
-               ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) Stream_Pointer(s), cbWorkingDir / 2, &settings->ShellWorkingDirectory, 0, NULL, NULL);
+               /* cbWorkingDir is the size in bytes of the character data in the WorkingDir field.
+                * This size excludes (!) the length of the mandatory null terminator.
+                * Maximum value including the mandatory null terminator: 512
+                */
+               if ((cbWorkingDir % 2) || cbWorkingDir > 512)
+               {
+                       WLog_ERR(TAG, "protocol error: invalid cbWorkingDir value: %u", cbWorkingDir);
+                       return FALSE;
+               }
+               wstr = (WCHAR*) Stream_Pointer(s);
+               if (wstr[cbWorkingDir / 2])
+               {
+                       WLog_ERR(TAG, "protocol error: WorkingDir must be null terminated");
+                       return FALSE;
+               }
+               if (ConvertFromUnicode(CP_UTF8, 0, wstr, -1, &settings->ShellWorkingDirectory, 0, NULL, NULL) < 1)
+               {
+                       WLog_ERR(TAG, "failed to convert AlternateShell string");
+                       return FALSE;
+               }
                Stream_Seek(s, cbWorkingDir);
        }
        Stream_Seek(s, 2);
@@ -664,6 +809,7 @@ BOOL rdp_recv_logon_info_v1(rdpRdp* rdp, wStream* s, logon_info *info)
 {
        UINT32 cbDomain;
        UINT32 cbUserName;
+       WCHAR* wstr;
 
        ZeroMemory(info, sizeof(*info));
 
@@ -671,35 +817,63 @@ BOOL rdp_recv_logon_info_v1(rdpRdp* rdp, wStream* s, logon_info *info)
                return FALSE;
 
        Stream_Read_UINT32(s, cbDomain); /* cbDomain (4 bytes) */
-       if (cbDomain > 52)
-               return FALSE;
-       if (cbDomain)
+
+       /* cbDomain is the size of the Unicode character data (including the mandatory
+        * null terminator) in bytes present in the fixed-length (52 bytes) Domain field
+        */
+       if ((cbDomain % 2) || cbDomain < 2 || cbDomain > 52)
        {
-               ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) Stream_Pointer(s), cbDomain, &info->domain, 0, NULL, FALSE);
-               if (!info->domain)
-                       return FALSE;
+               WLog_ERR(TAG, "protocol error: invalid cbDomain value: %lu", cbDomain);
+               goto fail;
+       }
+       wstr = (WCHAR*) Stream_Pointer(s);
+       if (wstr[cbDomain / 2 - 1])
+       {
+               WLog_ERR(TAG, "protocol error: Domain must be null terminated");
+               goto fail;
+       }
+       if (ConvertFromUnicode(CP_UTF8, 0, wstr, -1, &info->domain, 0, NULL, FALSE) < 1)
+       {
+               WLog_ERR(TAG, "failed to convert the Domain string");
+               goto fail;
        }
-
        Stream_Seek(s, 52); /* domain (52 bytes) */
 
+
        Stream_Read_UINT32(s, cbUserName); /* cbUserName (4 bytes) */
-       if (cbUserName > 512)
-               goto error_username;
-       if (cbUserName)
+
+       /* cbUserName is the size of the Unicode character data (including the mandatory
+        * null terminator) in bytes present in the fixed-length (512 bytes) UserName field.
+        */
+
+       if ((cbUserName % 2) || cbUserName < 2 || cbUserName > 512)
        {
-               ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) Stream_Pointer(s), cbUserName, &info->username, 0, NULL, FALSE);
-               if (!info->username)
-                       goto error_username;
+               WLog_ERR(TAG, "protocol error: invalid cbUserName value: %lu", cbUserName);
+               goto fail;
+       }
+       wstr = (WCHAR*) Stream_Pointer(s);
+       if (wstr[cbUserName / 2 - 1])
+       {
+               WLog_ERR(TAG, "protocol error: UserName must be null terminated");
+               goto fail;
+       }
+       if (ConvertFromUnicode(CP_UTF8, 0, wstr, -1, &info->username, 0, NULL, FALSE) < 1)
+       {
+               WLog_ERR(TAG, "failed to convert the UserName string");
+               goto fail;
        }
        Stream_Seek(s, 512); /* userName (512 bytes) */
 
        Stream_Read_UINT32(s, info->sessionId); /* SessionId (4 bytes) */
 
-       WLog_DBG(TAG, "LogonInfoV1: SessionId: 0x%04X", info->sessionId);
+       WLog_DBG(TAG, "LogonInfoV1: SessionId: 0x%04X UserName: [%s] Domain: [%s]",
+                info->sessionId, info->username, info->domain);
 
        return TRUE;
 
-error_username:
+fail:
+       free(info->username);
+       info->username = NULL;
        free(info->domain);
        info->domain = NULL;
        return FALSE;
@@ -711,6 +885,7 @@ BOOL rdp_recv_logon_info_v2(rdpRdp* rdp, wStream* s, logon_info *info)
        UINT32 Size;
        UINT32 cbDomain;
        UINT32 cbUserName;
+       WCHAR* wstr;
 
        ZeroMemory(info, sizeof(*info));
 
@@ -724,32 +899,87 @@ BOOL rdp_recv_logon_info_v2(rdpRdp* rdp, wStream* s, logon_info *info)
        Stream_Read_UINT32(s, cbUserName); /* cbUserName (4 bytes) */
        Stream_Seek(s, 558); /* pad (558 bytes) */
 
-       if (Stream_GetRemainingLength(s) < (cbDomain + cbUserName))
-               return FALSE;
 
-       if (cbDomain)
+       /* cbDomain is the size in bytes of the Unicode character data in the Domain field.
+        * The size of the mandatory null terminator is include in this value.
+        * Note: Since MS-RDPBCGR 2.2.10.1.1.2 does not mention any size limits we assume
+        *       that the maximum value is 52 bytes, according to the fixed size of the
+        *       Domain field in the Logon Info Version 1 (TS_LOGON_INFO) structure.
+        */
+
+       if ((cbDomain % 2) || cbDomain < 2 || cbDomain > 52)
        {
-               ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) Stream_Pointer(s), cbDomain, &info->domain, 0, NULL, FALSE);
-               if (!info->domain)
-                       return FALSE;
-               Stream_Seek(s, cbDomain); /* domain */
+               WLog_ERR(TAG, "protocol error: invalid cbDomain value: %lu", cbDomain);
+               goto fail;
        }
 
-       if (cbUserName)
+       if (Stream_GetRemainingLength(s) < (size_t) cbDomain)
        {
-               ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) Stream_Pointer(s), cbUserName, &info->username, 0, NULL, FALSE);
-               if (!info->username)
-               {
-                       free(info->domain);
-                       info->domain = NULL;
-                       return FALSE;
-               }
-               Stream_Seek(s, cbUserName); /* userName */
+               WLog_ERR(TAG, "insufficient remaining stream length");
+               goto fail;
+       }
+
+       wstr = (WCHAR*) Stream_Pointer(s);
+       if (wstr[cbDomain / 2 - 1])
+       {
+               WLog_ERR(TAG, "protocol error: Domain field must be null terminated");
+               goto fail;
        }
 
-       WLog_DBG(TAG, "LogonInfoV2: SessionId: 0x%04X", info->sessionId);
+       if (ConvertFromUnicode(CP_UTF8, 0, wstr, -1, &info->domain, 0, NULL, FALSE) < 1)
+       {
+               WLog_ERR(TAG, "failed to convert the Domain string");
+               goto fail;
+       }
+
+       Stream_Seek(s, cbDomain); /* domain */
+
+
+       /* cbUserName is the size in bytes of the Unicode character data in the UserName field.
+        * The size of the mandatory null terminator is include in this value.
+        * Note: Since MS-RDPBCGR 2.2.10.1.1.2 does not mention any size limits we assume
+        *       that the maximum value is 512 bytes, according to the fixed size of the
+        *       Username field in the Logon Info Version 1 (TS_LOGON_INFO) structure.
+        */
+
+       if ((cbUserName % 2) || cbUserName < 2 || cbUserName > 512)
+       {
+               WLog_ERR(TAG, "protocol error: invalid cbUserName value: %lu", cbUserName);
+               goto fail;
+       }
+
+       if (Stream_GetRemainingLength(s) < (size_t) cbUserName)
+       {
+               WLog_ERR(TAG, "insufficient remaining stream length");
+               goto fail;
+       }
+
+       wstr = (WCHAR*) Stream_Pointer(s);
+       if (wstr[cbUserName / 2 - 1])
+       {
+               WLog_ERR(TAG, "protocol error: UserName field must be null terminated");
+               goto fail;
+       }
+
+       if (ConvertFromUnicode(CP_UTF8, 0, wstr, -1, &info->username, 0, NULL, FALSE) < 1)
+       {
+               WLog_ERR(TAG, "failed to convert the Domain string");
+               goto fail;
+       }
+
+       Stream_Seek(s, cbUserName); /* userName */
+
+       WLog_DBG(TAG, "LogonInfoV2: SessionId: 0x%04X UserName: [%s] Domain: [%s]",
+                info->sessionId, info->username, info->domain);
 
        return TRUE;
+
+fail:
+       free(info->username);
+       info->username = NULL;
+       free(info->domain);
+       info->domain = NULL;
+       return FALSE;
 }
 
 BOOL rdp_recv_logon_plain_notify(rdpRdp* rdp, wStream* s)
index 6d31257..0922902 100644 (file)
@@ -30,7 +30,7 @@
 
 #define TAG FREERDP_TAG("core.redirection")
 
-void rdp_print_redirection_flags(UINT32 flags)
+static void rdp_print_redirection_flags(UINT32 flags)
 {
        WLog_DBG(TAG, "redirectionFlags = {");
 
@@ -76,9 +76,10 @@ void rdp_print_redirection_flags(UINT32 flags)
        WLog_DBG(TAG, "}");
 }
 
-BOOL rdp_redirection_read_string(wStream* s, char** str)
+static BOOL rdp_redirection_read_unicode_string(wStream* s, char** str, size_t maxLength)
 {
        UINT32 length;
+       WCHAR* wstr = NULL;
 
        if (Stream_GetRemainingLength(s) < 4)
        {
@@ -88,13 +89,31 @@ BOOL rdp_redirection_read_string(wStream* s, char** str)
 
        Stream_Read_UINT32(s, length);
 
+       if ((length % 2) || length < 2 || length > maxLength)
+       {
+               WLog_ERR(TAG,  "rdp_redirection_read_string failure: invalid unicode string length: %lu", length);
+               return FALSE;
+       }
+
        if (Stream_GetRemainingLength(s) < length)
        {
-               WLog_ERR(TAG,  "rdp_redirection_read_string failure: incorrect length %d", length);
+               WLog_ERR(TAG,  "rdp_redirection_read_string failure: insufficient stream length (%lu bytes required)", length);
+               return FALSE;
+       }
+
+       wstr = (WCHAR*) Stream_Pointer(s);
+
+       if (wstr[length / 2 - 1])
+       {
+               WLog_ERR(TAG,  "rdp_redirection_read_string failure: unterminated unicode string");
                return FALSE;
        }
 
-       ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) Stream_Pointer(s), length / 2, str, 0, NULL, NULL);
+       if (ConvertFromUnicode(CP_UTF8, 0, wstr, -1, str, 0, NULL, NULL) < 1)
+       {
+               WLog_ERR(TAG,  "rdp_redirection_read_string failure: string conversion failed");
+               return FALSE;
+       }
        Stream_Seek(s, length);
        return TRUE;
 }
@@ -173,7 +192,10 @@ int rdp_redirection_apply_settings(rdpRdp* rdp)
                /* Password may be a cookie without a null terminator */
                free(settings->RedirectionPassword);
                settings->RedirectionPasswordLength = redirection->PasswordLength;
-               settings->RedirectionPassword = (BYTE*) malloc(settings->RedirectionPasswordLength);
+               /* For security reasons we'll allocate an additional zero WCHAR at the
+                * end of the buffer that is not included in RedirectionPasswordLength
+                */
+               settings->RedirectionPassword = (BYTE*) calloc(1, settings->RedirectionPasswordLength + sizeof(WCHAR));
                if (!settings->RedirectionPassword)
                        return -1;
                CopyMemory(settings->RedirectionPassword, redirection->Password, settings->RedirectionPasswordLength);
@@ -219,7 +241,7 @@ int rdp_redirection_apply_settings(rdpRdp* rdp)
        return 0;
 }
 
-BOOL rdp_recv_server_redirection_pdu(rdpRdp* rdp, wStream* s)
+static BOOL rdp_recv_server_redirection_pdu(rdpRdp* rdp, wStream* s)
 {
        UINT16 flags;
        UINT16 length;
@@ -238,14 +260,33 @@ BOOL rdp_recv_server_redirection_pdu(rdpRdp* rdp, wStream* s)
 
        rdp_print_redirection_flags(redirection->flags);
 
+       /* Although MS-RDPBCGR does not mention any length constraints limits for the
+        * variable length null-terminated unicode strings in the RDP_SERVER_REDIRECTION_PACKET
+        * structure we will use the following limits in bytes including the null terminator:
+        *
+        * TargetNetAddress:     80 bytes
+        * UserName:            512 bytes
+        * Domain:               52 bytes
+        * Password(Cookie):    512 bytes
+        * TargetFQDN:          512 bytes
+        * TargetNetBiosName:    32 bytes
+        */
+
        if (redirection->flags & LB_TARGET_NET_ADDRESS)
        {
-               if (!rdp_redirection_read_string(s, &(redirection->TargetNetAddress)))
+               if (!rdp_redirection_read_unicode_string(s, &(redirection->TargetNetAddress), 80))
                        return -1;
        }
 
        if (redirection->flags & LB_LOAD_BALANCE_INFO)
        {
+               /* See [MSFT-SDLBTS] (a.k.a. TS_Session_Directory.doc)
+                * load balance info example data:
+                * 0000  43 6f 6f 6b 69 65 3a 20 6d 73 74 73 3d 32 31 33  Cookie: msts=213
+                * 0010  34 30 32 36 34 33 32 2e 31 35 36 32 39 2e 30 30  4026432.15629.00
+                 * 0020  30 30 0d 0a                                      00..
+                */
+
                if (Stream_GetRemainingLength(s) < 4)
                        return -1;
 
@@ -265,7 +306,7 @@ BOOL rdp_recv_server_redirection_pdu(rdpRdp* rdp, wStream* s)
 
        if (redirection->flags & LB_USERNAME)
        {
-               if (!rdp_redirection_read_string(s, &(redirection->Username)))
+               if (!rdp_redirection_read_unicode_string(s, &(redirection->Username), 512))
                        return -1;
 
                WLog_DBG(TAG, "Username: %s", redirection->Username);
@@ -273,7 +314,7 @@ BOOL rdp_recv_server_redirection_pdu(rdpRdp* rdp, wStream* s)
 
        if (redirection->flags & LB_DOMAIN)
        {
-               if (!rdp_redirection_read_string(s, &(redirection->Domain)))
+               if (!rdp_redirection_read_unicode_string(s, &(redirection->Domain), 52))
                        return FALSE;
 
                WLog_DBG(TAG, "Domain: %s", redirection->Domain);
@@ -281,12 +322,45 @@ BOOL rdp_recv_server_redirection_pdu(rdpRdp* rdp, wStream* s)
 
        if (redirection->flags & LB_PASSWORD)
        {
-               /* Note: length (hopefully) includes double zero termination */
+               /* Note: Password is a variable-length array of bytes containing the
+                * password used by the user in Unicode format, including a null-terminator
+                * or (!) or a cookie value that MUST be passed to the target server on
+                * successful connection.
+                * Since the format of the password cookie (probably some salted hash) is
+                * currently unknown we'll treat it as opaque data. All cookies seen so far
+                * are 120 bytes including \0\0 termination.
+                * Here is an observed example of a redirection password cookie:
+                *
+                * 0000  02 00 00 80 44 53 48 4c 60 ab 69 2f 07 d6 9e 2d  ....DSHL`.i/...-
+                * 0010  f0 3a 97 3b a9 c5 ec 7e 66 bd b3 84 6c b1 ef b9  .:.;...~f...l...
+                * 0020  b6 82 4e cc 3a df 64 b7 7b 25 04 54 c2 58 98 f8  ..N.:.d.{%.T.X..
+                * 0030  97 87 d4 93 c7 c1 e1 5b c2 85 f8 22 49 1f 81 88  .......[..."I...
+                * 0040  43 44 83 f6 9a 72 40 24 dc 4d 43 cb d9 92 3c 8f  CD...r@$.MC...<.
+                * 0050  3a 37 5c 77 13 a0 72 3c 72 08 64 2a 29 fb dc eb  :7\w..r<r.d*)...
+                * 0060  0d 2b 06 b4 c6 08 b4 73 34 16 93 62 6d 24 e9 93  .+.....s4..bm$..
+                * 0070  97 27 7b dd 9a 72 00 00                          .'{..r..
+                *
+                * Notwithstanding the above, we'll allocated an additional zero WCHAR at the
+                * end of the buffer which won't get counted in PasswordLength.
+                */
+
                if (Stream_GetRemainingLength(s) < 4)
                        return -1;
 
                Stream_Read_UINT32(s, redirection->PasswordLength);
-               redirection->Password = (BYTE*) malloc(redirection->PasswordLength);
+
+               /* [MS-RDPBCGR] specifies 512 bytes as the upper limit for the password length
+                * including the null terminatior(s). This should also be enough for the unknown
+                * password cookie format (see previous comment).
+                */
+
+               if (Stream_GetRemainingLength(s) < redirection->PasswordLength)
+                       return -1;
+
+               if (redirection->PasswordLength > 512)
+                       return -1;
+
+               redirection->Password = (BYTE*) calloc(1, redirection->PasswordLength + sizeof(WCHAR));
                if (!redirection->Password)
                        return -1;
                Stream_Read(s, redirection->Password, redirection->PasswordLength);
@@ -297,7 +371,7 @@ BOOL rdp_recv_server_redirection_pdu(rdpRdp* rdp, wStream* s)
 
        if (redirection->flags & LB_TARGET_FQDN)
        {
-               if (!rdp_redirection_read_string(s, &(redirection->TargetFQDN)))
+               if (!rdp_redirection_read_unicode_string(s, &(redirection->TargetFQDN), 512))
                        return -1;
 
                WLog_DBG(TAG, "TargetFQDN: %s", redirection->TargetFQDN);
@@ -305,7 +379,7 @@ BOOL rdp_recv_server_redirection_pdu(rdpRdp* rdp, wStream* s)
 
        if (redirection->flags & LB_TARGET_NETBIOS_NAME)
        {
-               if (!rdp_redirection_read_string(s, &(redirection->TargetNetBiosName)))
+               if (!rdp_redirection_read_unicode_string(s, &(redirection->TargetNetBiosName), 32))
                        return -1;
 
                WLog_DBG(TAG, "TargetNetBiosName: %s", redirection->TargetNetBiosName);
@@ -352,7 +426,7 @@ BOOL rdp_recv_server_redirection_pdu(rdpRdp* rdp, wStream* s)
 
                for (i = 0; i < (int) count; i++)
                {
-                       if (!rdp_redirection_read_string(s, &(redirection->TargetNetAddresses[i])))
+                       if (!rdp_redirection_read_unicode_string(s, &(redirection->TargetNetAddresses[i]), 80))
                                return FALSE;
 
                        WLog_DBG(TAG, "TargetNetAddresses[%d]: %s", i, redirection->TargetNetAddresses[i]);
@@ -371,13 +445,6 @@ BOOL rdp_recv_server_redirection_pdu(rdpRdp* rdp, wStream* s)
        return 1;
 }
 
-int rdp_recv_redirection_packet(rdpRdp* rdp, wStream* s)
-{
-       int status = 0;
-       status = rdp_recv_server_redirection_pdu(rdp, s);
-       return status;
-}
-
 int rdp_recv_enhanced_security_redirection_packet(rdpRdp* rdp, wStream* s)
 {
        int status = 0;
index e31fe45..e197b76 100644 (file)
@@ -371,6 +371,7 @@ BOOL update_recv_window_info_order(rdpUpdate* update, wStream* s, WINDOW_ORDER_I
 {
        rdpContext* context = update->context;
        rdpWindowUpdate* window = update->window;
+       BOOL result = TRUE;
 
        if (Stream_GetRemainingLength(s) < 4)
                return FALSE;
@@ -382,20 +383,20 @@ BOOL update_recv_window_info_order(rdpUpdate* update, wStream* s, WINDOW_ORDER_I
                if (!update_read_window_icon_order(s, orderInfo, &window->window_icon))
                        return FALSE;
                WLog_Print(update->log, WLOG_DEBUG, "WindowIcon");
-               IFCALL(window->WindowIcon, context, orderInfo, &window->window_icon);
+               IFCALLRET(window->WindowIcon, result, context, orderInfo, &window->window_icon);
        }
        else if (orderInfo->fieldFlags & WINDOW_ORDER_CACHED_ICON)
        {
                if (!update_read_window_cached_icon_order(s, orderInfo, &window->window_cached_icon))
                        return FALSE;
                WLog_Print(update->log, WLOG_DEBUG, "WindowCachedIcon");
-               IFCALL(window->WindowCachedIcon, context, orderInfo, &window->window_cached_icon);
+               IFCALLRET(window->WindowCachedIcon, result, context, orderInfo, &window->window_cached_icon);
        }
        else if (orderInfo->fieldFlags & WINDOW_ORDER_STATE_DELETED)
        {
                update_read_window_delete_order(s, orderInfo);
                WLog_Print(update->log, WLOG_DEBUG, "WindowDelete");
-               IFCALL(window->WindowDelete, context, orderInfo);
+               IFCALLRET(window->WindowDelete, result, context, orderInfo);
        }
        else
        {
@@ -405,16 +406,16 @@ BOOL update_recv_window_info_order(rdpUpdate* update, wStream* s, WINDOW_ORDER_I
                if (orderInfo->fieldFlags & WINDOW_ORDER_STATE_NEW)
                {
                        WLog_Print(update->log, WLOG_DEBUG, "WindowCreate");
-                       IFCALL(window->WindowCreate, context, orderInfo, &window->window_state);
+                       IFCALLRET(window->WindowCreate, result, context, orderInfo, &window->window_state);
                }
                else
                {
                        WLog_Print(update->log, WLOG_DEBUG, "WindowUpdate");
-                       IFCALL(window->WindowUpdate, context, orderInfo, &window->window_state);
+                       IFCALLRET(window->WindowUpdate, result, context, orderInfo, &window->window_state);
                }
        }
 
-       return TRUE;
+       return result;
 }
 
 BOOL update_read_notification_icon_state_order(wStream* s, WINDOW_ORDER_INFO* orderInfo, NOTIFY_ICON_STATE_ORDER* notify_icon_state)
@@ -470,6 +471,7 @@ BOOL update_recv_notification_icon_info_order(rdpUpdate* update, wStream* s, WIN
 {
        rdpContext* context = update->context;
        rdpWindowUpdate* window = update->window;
+       BOOL result = TRUE;
 
        if (Stream_GetRemainingLength(s) < 8)
                return FALSE;
@@ -481,7 +483,7 @@ BOOL update_recv_notification_icon_info_order(rdpUpdate* update, wStream* s, WIN
        {
                update_read_notification_icon_delete_order(s, orderInfo);
                WLog_Print(update->log, WLOG_DEBUG, "NotifyIconDelete");
-               IFCALL(window->NotifyIconDelete, context, orderInfo);
+               IFCALLRET(window->NotifyIconDelete, result, context, orderInfo);
        }
        else
        {
@@ -491,16 +493,16 @@ BOOL update_recv_notification_icon_info_order(rdpUpdate* update, wStream* s, WIN
                if (orderInfo->fieldFlags & WINDOW_ORDER_STATE_NEW)
                {
                        WLog_Print(update->log, WLOG_DEBUG, "NotifyIconCreate");
-                       IFCALL(window->NotifyIconCreate, context, orderInfo, &window->notify_icon_state);
+                       IFCALLRET(window->NotifyIconCreate, result, context, orderInfo, &window->notify_icon_state);
                }
                else
                {
                        WLog_Print(update->log, WLOG_DEBUG, "NotifyIconUpdate");
-                       IFCALL(window->NotifyIconUpdate, context, orderInfo, &window->notify_icon_state);
+                       IFCALLRET(window->NotifyIconUpdate, result, context, orderInfo, &window->notify_icon_state);
                }
        }
 
-       return TRUE;
+       return result;
 }
 
 BOOL update_read_desktop_actively_monitored_order(wStream* s, WINDOW_ORDER_INFO* orderInfo, MONITORED_DESKTOP_ORDER* monitored_desktop)
@@ -560,22 +562,23 @@ BOOL update_recv_desktop_info_order(rdpUpdate* update, wStream* s, WINDOW_ORDER_
 {
        rdpContext* context = update->context;
        rdpWindowUpdate* window = update->window;
+       BOOL result = TRUE;
 
        if (orderInfo->fieldFlags & WINDOW_ORDER_FIELD_DESKTOP_NONE)
        {
                update_read_desktop_non_monitored_order(s, orderInfo);
                WLog_Print(update->log, WLOG_DEBUG, "NonMonitoredDesktop");
-               IFCALL(window->NonMonitoredDesktop, context, orderInfo);
+               IFCALLRET(window->NonMonitoredDesktop, result, context, orderInfo);
        }
        else
        {
                if (!update_read_desktop_actively_monitored_order(s, orderInfo, &window->monitored_desktop))
                        return FALSE;
                WLog_Print(update->log, WLOG_DEBUG, "ActivelyMonitoredDesktop");
-               IFCALL(window->MonitoredDesktop, context, orderInfo, &window->monitored_desktop);
+               IFCALLRET(window->MonitoredDesktop, result, context, orderInfo, &window->monitored_desktop);
        }
 
-       return TRUE;
+       return result;
 }
 
 BOOL update_recv_altsec_window_order(rdpUpdate* update, wStream* s)
index 44a12d2..2bfc423 100644 (file)
@@ -744,7 +744,7 @@ int win_shadow_wds_init(winShadowSubsystem* subsystem)
                return -1;
        }
 
-       subsystem->pInvitation->lpVtbl->get_ConnectionString(subsystem->pInvitation, &bstrConnectionString);
+       hr = subsystem->pInvitation->lpVtbl->get_ConnectionString(subsystem->pInvitation, &bstrConnectionString);
 
        if (FAILED(hr))
        {
@@ -752,10 +752,24 @@ int win_shadow_wds_init(winShadowSubsystem* subsystem)
                return -1;
        }
 
+       status = ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) bstrConnectionString,
+               ((UINT32*) bstrConnectionString)[-1], &(file->ConnectionString2), 0, NULL, NULL);
+
+       SysFreeString(bstrConnectionString);
+
+       if (status < 1)
+       {
+               WLog_ERR(TAG, "failed to convert connection string");
+               return -1;
+       }
+
        file = subsystem->pAssistanceFile = freerdp_assistance_file_new();
 
-       ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) bstrConnectionString,
-               ((UINT32*) bstrConnectionString)[-1], &(file->ConnectionString2), 0, NULL, NULL);
+       if (!file)
+       {
+               WLog_ERR(TAG, "freerdp_assistance_file_new() failed");
+               return -1;
+       }
 
        status = freerdp_assistance_parse_connection_string2(file);
 
index 559c919..08497bf 100644 (file)
@@ -260,7 +260,7 @@ int convert_utf16_to_utf8(BYTE* lpWideCharStr, BYTE* expected_lpMultiByteStr, in
        return length;
 }
 
-int test_unicode_uppercasing(BYTE* lower, BYTE* upper)
+BOOL test_unicode_uppercasing(BYTE* lower, BYTE* upper)
 {
        WCHAR* lowerW = NULL;
        int lowerLength;
@@ -280,13 +280,182 @@ int test_unicode_uppercasing(BYTE* lower, BYTE* upper)
                printf("Uppercase String:\n");
                string_hexdump((BYTE*) upperW, upperLength * 2);
 
-               return -1;
+               return FALSE;
        }
 
        free(lowerW);
        free(upperW);
 
-       return 0;
+       printf("success\n\n");
+       return TRUE;
+}
+
+BOOL test_ConvertFromUnicode_wrapper()
+{
+       /*               00  01  02  03  04  05  06  07  08  09  10  11  12  13  14  15  16  17  18 */
+       WCHAR src1[] = { 'R','I','C','H',' ','T','E','X','T',' ','F','O','R','M','A','T','@','@','@' };
+       WCHAR src2[] = { 'R','I','C','H',' ','T','E','X','T',' ','F','O','R','M','A','T', 0 };
+       CHAR  cmp0[] = { 'R','I','C','H',' ','T','E','X','T',' ','F','O','R','M','A','T', 0 };
+       CHAR* dst = NULL;
+       int i;
+
+       /* Test unterminated unicode string:
+        * ConvertFromUnicode must always null-terminate, even if the src string isn't
+        */
+
+       printf("Input UTF16 String:\n");
+       string_hexdump((BYTE*) src1, 19 * sizeof(WCHAR));
+
+       i = ConvertFromUnicode(CP_UTF8, 0, src1, 16, &dst, 0, NULL, NULL);
+       if (i != 16)
+       {
+               fprintf(stderr, "ConvertFromUnicode failure A1: unexpectedly returned %d instead of 16\n", i);
+               goto fail;
+       }
+       if (dst == NULL)
+       {
+               fprintf(stderr, "ConvertFromUnicode failure A2: destination ist NULL\n");
+               goto fail;
+       }
+       if ((i = strlen(dst)) != 16)
+       {
+               fprintf(stderr, "ConvertFromUnicode failure A3: dst length is %d instead of 16\n", i);
+               goto fail;
+       }
+       if (strcmp(dst, cmp0))
+       {
+               fprintf(stderr, "ConvertFromUnicode failure A4: data mismatch\n");
+               goto fail;
+       }
+       printf("Output UTF8 String:\n");
+       string_hexdump((BYTE*) dst, i + 1);
+
+       free(dst);
+       dst = NULL;
+
+       /* Test null-terminated string */
+
+       printf("Input UTF16 String:\n");
+       string_hexdump((BYTE*) src2, (_wcslen(src2) + 1 ) * sizeof(WCHAR));
+
+       i = ConvertFromUnicode(CP_UTF8, 0, src2, -1, &dst, 0, NULL, NULL);
+       if (i != 17)
+       {
+               fprintf(stderr, "ConvertFromUnicode failure B1: unexpectedly returned %d instead of 17\n", i);
+               goto fail;
+       }
+       if (dst == NULL)
+       {
+               fprintf(stderr, "ConvertFromUnicode failure B2: destination ist NULL\n");
+               goto fail;
+       }
+       if ((i = strlen(dst)) != 16)
+       {
+               fprintf(stderr, "ConvertFromUnicode failure B3: dst length is %d instead of 16\n", i);
+               goto fail;
+       }
+       if (strcmp(dst, cmp0))
+       {
+               fprintf(stderr, "ConvertFromUnicode failure B: data mismatch\n");
+               goto fail;
+       }
+       printf("Output UTF8 String:\n");
+       string_hexdump((BYTE*) dst, i + 1);
+
+       free(dst);
+       dst = NULL;
+
+       printf("success\n\n");
+
+       return TRUE;
+
+fail:
+       free(dst);
+       return FALSE;
+}
+
+BOOL test_ConvertToUnicode_wrapper()
+{
+       /*               00  01  02  03  04  05  06  07  08  09  10  11  12  13  14  15  16  17  18 */
+       CHAR  src1[] = { 'R','I','C','H',' ','T','E','X','T',' ','F','O','R','M','A','T','@','@','@' };
+       CHAR  src2[] = { 'R','I','C','H',' ','T','E','X','T',' ','F','O','R','M','A','T', 0 };
+       WCHAR cmp0[] = { 'R','I','C','H',' ','T','E','X','T',' ','F','O','R','M','A','T', 0 };
+       WCHAR* dst = NULL;
+       int i;
+
+       /* Test unterminated unicode string:
+        * ConvertToUnicode must always null-terminate, even if the src string isn't
+        */
+
+       printf("Input UTF8 String:\n");
+       string_hexdump((BYTE*) src1, 19);
+
+       i = ConvertToUnicode(CP_UTF8, 0, src1, 16, &dst, 0);
+       if (i != 16)
+       {
+               fprintf(stderr, "ConvertToUnicode failure A1: unexpectedly returned %d instead of 16\n", i);
+               goto fail;
+       }
+       if (dst == NULL)
+       {
+               fprintf(stderr, "ConvertToUnicode failure A2: destination ist NULL\n");
+               goto fail;
+       }
+       if ((i = _wcslen(dst)) != 16)
+       {
+               fprintf(stderr, "ConvertToUnicode failure A3: dst length is %d instead of 16\n", i);
+               goto fail;
+       }
+       if (_wcscmp(dst, cmp0))
+       {
+               fprintf(stderr, "ConvertToUnicode failure A4: data mismatch\n");
+               goto fail;
+       }
+       printf("Output UTF16 String:\n");
+       string_hexdump((BYTE*) dst, (i + 1) * sizeof(WCHAR));
+
+       free(dst);
+       dst = NULL;
+
+       /* Test null-terminated string */
+
+       printf("Input UTF8 String:\n");
+       string_hexdump((BYTE*) src2, strlen(src2) + 1);
+
+       i = ConvertToUnicode(CP_UTF8, 0, src2, -1, &dst, 0);
+       if (i != 17)
+       {
+               fprintf(stderr, "ConvertToUnicode failure B1: unexpectedly returned %d instead of 17\n", i);
+               goto fail;
+       }
+       if (dst == NULL)
+       {
+               fprintf(stderr, "ConvertToUnicode failure B2: destination ist NULL\n");
+               goto fail;
+       }
+       if ((i = _wcslen(dst)) != 16)
+       {
+               fprintf(stderr, "ConvertToUnicode failure B3: dst length is %d instead of 16\n", i);
+               goto fail;
+       }
+       if (_wcscmp(dst, cmp0))
+       {
+               fprintf(stderr, "ConvertToUnicode failure B: data mismatch\n");
+               goto fail;
+       }
+       printf("Output UTF16 String:\n");
+       string_hexdump((BYTE*) dst, (i + 1) * 2);
+
+       free(dst);
+       dst = NULL;
+
+       printf("success\n\n");
+
+       return TRUE;
+
+fail:
+       free(dst);
+       return FALSE;
 }
 
 int TestUnicodeConversion(int argc, char* argv[])
@@ -375,7 +544,47 @@ int TestUnicodeConversion(int argc, char* argv[])
 
        printf("Uppercasing\n");
 
-       test_unicode_uppercasing(ru_Administrator_lower, ru_Administrator_upper);
+       if (!test_unicode_uppercasing(ru_Administrator_lower, ru_Administrator_upper))
+               return -1;
+
+       /* ConvertFromUnicode */
+
+       printf("ConvertFromUnicode\n");
+
+       if (!test_ConvertFromUnicode_wrapper())
+               return -1;
+
+       /* ConvertToUnicode */
+
+       printf("ConvertToUnicode\n");
 
+       if (!test_ConvertToUnicode_wrapper())
+               return -1;
+/*
+
+       printf("----------------------------------------------------------\n\n");
+
+       if (0)
+       {
+               BYTE src[] = { 'R',0,'I',0,'C',0,'H',0,' ',0, 'T',0,'E',0,'X',0,'T',0,' ',0,'F',0,'O',0,'R',0,'M',0,'A',0,'T',0,'@',0,'@',0 };
+               //BYTE src[] = { 'R',0,'I',0,'C',0,'H',0,' ',0,  0,0,  'T',0,'E',0,'X',0,'T',0,' ',0,'F',0,'O',0,'R',0,'M',0,'A',0,'T',0,'@',0,'@',0 };
+               //BYTE src[] = { 0,0,'R',0,'I',0,'C',0,'H',0,' ',0, 'T',0,'E',0,'X',0,'T',0,' ',0,'F',0,'O',0,'R',0,'M',0,'A',0,'T',0,'@',0,'@',0 };
+               char* dst = NULL;
+               int num;
+               num = ConvertFromUnicode(CP_UTF8, 0, (WCHAR*) src, 16, &dst, 0, NULL, NULL);
+               printf("ConvertFromUnicode returned %d dst=[%s]\n", num, dst);
+               string_hexdump((BYTE*)dst, num+1);
+       }
+       if (1)
+       {
+               char src[] = "RICH TEXT FORMAT@@@@@@";
+               WCHAR *dst = NULL;
+               int num;
+               num = ConvertToUnicode(CP_UTF8, 0, src, 16, &dst, 0);
+               printf("ConvertToUnicode returned %d dst=%p\n", num, dst);
+               string_hexdump((BYTE*)dst, num * 2 + 2);
+
+       }
+*/
        return 0;
 }
index fc5134f..0816fe0 100644 (file)
@@ -276,6 +276,19 @@ int WideCharToMultiByte(UINT CodePage, DWORD dwFlags, LPCWSTR lpWideCharStr, int
 
 #endif
 
+/**
+ * ConvertToUnicode is a convenience wrapper for MultiByteToWideChar:
+ *
+ * If the lpWideCharStr prarameter for the converted string points to NULL
+ * or if the cchWideChar parameter is set to 0 this function will automatically
+ * allocate the required memory which is guaranteed to be null-terminated
+ * after the conversion, even if the source c string isn't.
+ *
+ * If the cbMultiByte parameter is set to -1 the passed lpMultiByteStr must
+ * be null-terminated and the required length for the converted string will be
+ * calculated accordingly.
+ */
+
 int ConvertToUnicode(UINT CodePage, DWORD dwFlags, LPCSTR lpMultiByteStr,
                int cbMultiByte, LPWSTR* lpWideCharStr, int cchWideChar)
 {
@@ -305,7 +318,7 @@ int ConvertToUnicode(UINT CodePage, DWORD dwFlags, LPCSTR lpMultiByteStr,
 
        if (allocate)
        {
-               *lpWideCharStr = (LPWSTR) calloc(cchWideChar, sizeof(WCHAR));
+               *lpWideCharStr = (LPWSTR) calloc(cchWideChar + 1, sizeof(WCHAR));
 
                if (!(*lpWideCharStr))
                {
@@ -317,11 +330,31 @@ int ConvertToUnicode(UINT CodePage, DWORD dwFlags, LPCSTR lpMultiByteStr,
        status = MultiByteToWideChar(CodePage, dwFlags, lpMultiByteStr, cbMultiByte, *lpWideCharStr, cchWideChar);
 
        if (status != cchWideChar)
+       {
+               if (allocate)
+               {
+                       free(*lpWideCharStr);
+                       *lpWideCharStr = NULL;
+               }
                status = 0;
+       }
 
        return status;
 }
 
+/**
+ * ConvertFromUnicode is a convenience wrapper for WideCharToMultiByte:
+ *
+ * If the lpMultiByteStr parameter for the converted string points to NULL
+ * or if the cbMultiByte parameter is set to 0 this function will automatically
+ * allocate the required memory which is guaranteed to be null-terminated
+ * after the conversion, even if the source unicode string isn't.
+ *
+ * If the cchWideChar parameter is set to -1 the passed lpWideCharStr must
+ * be null-terminated and the required length for the converted string will be
+ * calculated accordingly.
+ */
+
 int ConvertFromUnicode(UINT CodePage, DWORD dwFlags, LPCWSTR lpWideCharStr, int cchWideChar,
                LPSTR* lpMultiByteStr, int cbMultiByte, LPCSTR lpDefaultChar, LPBOOL lpUsedDefaultChar)
 {
index 367d282..71d4756 100644 (file)
@@ -274,7 +274,7 @@ HANDLE CreateFileW(LPCWSTR lpFileName, DWORD dwDesiredAccess, DWORD dwShareMode,
        LPSTR lpFileNameA = NULL;
        HANDLE hdl;
 
-       if (ConvertFromUnicode(CP_UTF8, 0, lpFileName, -1, &lpFileNameA, 0, NULL, NULL))
+       if (ConvertFromUnicode(CP_UTF8, 0, lpFileName, -1, &lpFileNameA, 0, NULL, NULL) < 1)
                return NULL;
 
        hdl= CreateFileA(lpFileNameA, dwDesiredAccess, dwShareMode, lpSecurityAttributes,
@@ -296,7 +296,7 @@ BOOL DeleteFileW(LPCWSTR lpFileName)
        LPSTR lpFileNameA = NULL;
        BOOL rc = FALSE;
 
-       if (ConvertFromUnicode(CP_UTF8, 0, lpFileName, -1, &lpFileNameA, 0, NULL, NULL))
+       if (ConvertFromUnicode(CP_UTF8, 0, lpFileName, -1, &lpFileNameA, 0, NULL, NULL) < 1)
                return FALSE;
        rc = DeleteFileA(lpFileNameA);
        free (lpFileNameA);