[core,info] improve rdp_read_info_null_string

* Removed comments with invalid assumptions
* Added arguments to rdp_read_info_null_string to indicate if the string
  is expected to be '\0' terminated and what is actually read for error
  logs
This commit is contained in:
Armin Novak 2022-12-14 11:31:10 +01:00 committed by Pascal Nowack
parent 0a7d19ee7a
commit 74f273e593

View File

@ -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)