diff --git a/libfreerdp/core/info.c b/libfreerdp/core/info.c index 5e57e9601..31d92924d 100644 --- a/libfreerdp/core/info.c +++ b/libfreerdp/core/info.c @@ -73,8 +73,8 @@ static const struct info_flags_t info_flags[] = { { INFO_HIDEF_RAIL_SUPPORTED, "INFO_HIDEF_RAIL_SUPPORTED" }, }; -static BOOL rdp_read_info_null_string(UINT32 flags, wStream* s, size_t cbLen, CHAR** dst, - size_t max) +static BOOL rdp_read_info_null_string(const char* what, UINT32 flags, wStream* s, size_t cbLen, + CHAR** dst, size_t max, BOOL isNullTerminated) { CHAR* ret = NULL; @@ -86,13 +86,12 @@ static BOOL rdp_read_info_null_string(UINT32 flags, wStream* s, size_t cbLen, CH if (cbLen > 0) { - /* 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 ((cbLen % 2) || (cbLen > (max - nullSize))) + if (isNullTerminated && (max > 0)) + max -= nullSize; + + if ((cbLen > max) || (unicode && ((cbLen % 2) != 0))) { - WLog_ERR(TAG, "protocol error: invalid value: %" PRIuz "", cbLen); + WLog_ERR(TAG, "protocol error: %s has invalid value: %" PRIuz "", what, cbLen); return FALSE; } @@ -101,13 +100,21 @@ static BOOL rdp_read_info_null_string(UINT32 flags, wStream* s, size_t cbLen, CH size_t len = 0; ret = Stream_Read_UTF16_String_As_UTF8(s, cbLen / sizeof(WCHAR), &len); if (!ret && (cbLen > 0)) + { + WLog_ERR(TAG, "protocol error: no data to read for %s [expected %" PRIuz "]", what, + cbLen); return FALSE; + } } else { const char* domain = (const char*)Stream_Pointer(s); if (!Stream_SafeSeek(s, cbLen)) + { + WLog_ERR(TAG, "protocol error: no data to read for %s [expected %" PRIuz "]", what, + cbLen); return FALSE; + } ret = calloc(cbLen + 1, nullSize); if (!ret) @@ -327,24 +334,11 @@ static 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 - * Note: Although according to [MS-RDPBCGR 2.2.1.11.1.1.1] the null terminator - * is mandatory, connections via Microsoft's TS Gateway set cbClientAddress to 0. - */ - - if ((cbClientAddress % 2) || (cbClientAddress > rdp_get_client_address_max_size(rdp))) - { - WLog_ERR(TAG, "protocol error: invalid cbClientAddress value: %" PRIu16 "", - cbClientAddress); - return FALSE; - } - settings->IPv6Enabled = (clientAddressFamily == ADDRESS_FAMILY_INET6 ? TRUE : FALSE); - if (!rdp_read_info_null_string(INFO_UNICODE, s, cbClientAddress, &settings->ClientAddress, - (settings->RdpVersion < RDP_VERSION_10_0) ? 64 : 80)) + if (!rdp_read_info_null_string("cbClientAddress", INFO_UNICODE, s, cbClientAddress, + &settings->ClientAddress, rdp_get_client_address_max_size(rdp), + TRUE)) return FALSE; if (!Stream_CheckAndLogRequiredLength(TAG, s, 2)) @@ -360,7 +354,8 @@ static BOOL rdp_read_extended_info_packet(rdpRdp* rdp, wStream* s) * sets cbClientDir to 0. */ - if (!rdp_read_info_null_string(INFO_UNICODE, s, cbClientDir, &settings->ClientDir, 512)) + if (!rdp_read_info_null_string("cbClientDir", INFO_UNICODE, s, cbClientDir, + &settings->ClientDir, 512, TRUE)) return FALSE; /** @@ -433,8 +428,9 @@ static BOOL rdp_read_extended_info_packet(rdpRdp* rdp, wStream* s) Stream_Read_UINT16(s, cbDynamicDSTTimeZoneKeyName); - if (!rdp_read_info_null_string(INFO_UNICODE, s, cbDynamicDSTTimeZoneKeyName, - &settings->DynamicDSTTimeZoneKeyName, 254)) + if (!rdp_read_info_null_string("cbDynamicDSTTimeZoneKeyName", INFO_UNICODE, s, + cbDynamicDSTTimeZoneKeyName, + &settings->DynamicDSTTimeZoneKeyName, 254, FALSE)) return FALSE; if (Stream_GetRemainingLength(s) == 0)