Merge pull request #2509 from bmiklautz/security_fixes_v2

NULL dereference fixes
This commit is contained in:
Norbert Federa 2015-03-31 15:37:16 +02:00
commit 89b7eebaf8
13 changed files with 108 additions and 57 deletions

View File

@ -46,9 +46,9 @@ void cliprdr_process_format_list(cliprdrPlugin* cliprdr, wStream* s, UINT32 data
if (!context->custom)
return;
asciiNames = (msgFlags & CB_ASCII_NAMES) ? TRUE : FALSE;
formatList.msgType = CB_FORMAT_LIST;
formatList.msgFlags = msgFlags;
formatList.dataLen = dataLen;
@ -56,7 +56,7 @@ void cliprdr_process_format_list(cliprdrPlugin* cliprdr, wStream* s, UINT32 data
index = 0;
formatList.numFormats = 0;
position = Stream_GetPosition(s);
if (!formatList.dataLen)
{
/* empty format list */
@ -66,32 +66,32 @@ void cliprdr_process_format_list(cliprdrPlugin* cliprdr, wStream* s, UINT32 data
else if (!cliprdr->useLongFormatNames)
{
formatList.numFormats = (dataLen / 36);
if ((formatList.numFormats * 36) != dataLen)
{
WLog_ERR(TAG, "Invalid short format list length: %d", dataLen);
return;
}
if (formatList.numFormats)
formats = (CLIPRDR_FORMAT*) calloc(formatList.numFormats, sizeof(CLIPRDR_FORMAT));
if (!formats)
return;
formatList.formats = formats;
while (dataLen)
{
Stream_Read_UINT32(s, formats[index].formatId); /* formatId (4 bytes) */
dataLen -= 4;
formats[index].formatName = NULL;
if (asciiNames)
{
szFormatName = (char*) Stream_Pointer(s);
if (szFormatName[0])
{
formats[index].formatName = (char*) malloc(32 + 1);
@ -102,14 +102,14 @@ void cliprdr_process_format_list(cliprdrPlugin* cliprdr, wStream* s, UINT32 data
else
{
wszFormatName = (WCHAR*) Stream_Pointer(s);
if (wszFormatName[0])
{
ConvertFromUnicode(CP_UTF8, 0, wszFormatName,
16, &(formats[index].formatName), 0, NULL, NULL);
}
}
Stream_Seek(s, 32);
dataLen -= 32;
index++;
@ -121,17 +121,17 @@ void cliprdr_process_format_list(cliprdrPlugin* cliprdr, wStream* s, UINT32 data
{
Stream_Seek(s, 4); /* formatId (4 bytes) */
dataLen -= 4;
wszFormatName = (WCHAR*) Stream_Pointer(s);
if (!wszFormatName[0])
formatNameLength = 0;
else
formatNameLength = _wcslen(wszFormatName);
Stream_Seek(s, (formatNameLength + 1) * 2);
dataLen -= ((formatNameLength + 1) * 2);
formatList.numFormats++;
}
@ -140,7 +140,7 @@ void cliprdr_process_format_list(cliprdrPlugin* cliprdr, wStream* s, UINT32 data
if (formatList.numFormats)
formats = (CLIPRDR_FORMAT*) calloc(formatList.numFormats, sizeof(CLIPRDR_FORMAT));
if (!formats)
return;
@ -152,9 +152,9 @@ void cliprdr_process_format_list(cliprdrPlugin* cliprdr, wStream* s, UINT32 data
dataLen -= 4;
formats[index].formatName = NULL;
wszFormatName = (WCHAR*) Stream_Pointer(s);
if (!wszFormatName[0])
formatNameLength = 0;
else
@ -165,7 +165,7 @@ void cliprdr_process_format_list(cliprdrPlugin* cliprdr, wStream* s, UINT32 data
ConvertFromUnicode(CP_UTF8, 0, wszFormatName,
-1, &(formats[index].formatName), 0, NULL, NULL);
}
Stream_Seek(s, (formatNameLength + 1) * 2);
dataLen -= ((formatNameLength + 1) * 2);
@ -179,13 +179,15 @@ void cliprdr_process_format_list(cliprdrPlugin* cliprdr, wStream* s, UINT32 data
if (context->ServerFormatList)
context->ServerFormatList(context, &formatList);
for (index = 0; index < formatList.numFormats; index++)
if (formats)
{
if (formats[index].formatName)
for (index = 0; index < formatList.numFormats; index++)
{
free(formats[index].formatName);
}
}
free(formats);
free(formats);
}
}
void cliprdr_process_format_list_response(cliprdrPlugin* cliprdr, wStream* s, UINT32 dataLen, UINT16 msgFlags)
@ -230,25 +232,25 @@ void cliprdr_process_format_data_response(cliprdrPlugin* cliprdr, wStream* s, UI
{
CLIPRDR_FORMAT_DATA_RESPONSE formatDataResponse;
CliprdrClientContext* context = cliprdr_get_client_interface(cliprdr);
WLog_Print(cliprdr->log, WLOG_DEBUG, "ServerFormatDataResponse");
if (!context->custom)
return;
formatDataResponse.msgType = CB_FORMAT_DATA_RESPONSE;
formatDataResponse.msgFlags = msgFlags;
formatDataResponse.dataLen = dataLen;
formatDataResponse.requestedFormatData = NULL;
if (dataLen)
{
formatDataResponse.requestedFormatData = (BYTE*) malloc(dataLen);
Stream_Read(s, formatDataResponse.requestedFormatData, dataLen);
}
if (context->ServerFormatDataResponse)
context->ServerFormatDataResponse(context, &formatDataResponse);
free(formatDataResponse.requestedFormatData);
}

View File

@ -215,7 +215,7 @@ int freerdp_client_print_command_line_help(int argc, char** argv)
if (arg->Format)
{
length = (int)(strlen(arg->Name) + strlen(arg->Format) + 2);
str = (char*) malloc(length + 1);
str = (char*) calloc(length + 1UL, sizeof(char));
sprintf_s(str, length + 1, "%s:%s", arg->Name, arg->Format);
printf("%-20s", str);
free(str);
@ -230,7 +230,7 @@ int freerdp_client_print_command_line_help(int argc, char** argv)
else if (arg->Flags & COMMAND_LINE_VALUE_BOOL)
{
length = (int) strlen(arg->Name) + 32;
str = (char*) malloc(length + 1);
str = (char*) calloc(length + 1UL, sizeof(char));
sprintf_s(str, length + 1, "%s (default:%s)", arg->Name,
arg->Default ? "on" : "off");
@ -473,7 +473,7 @@ int freerdp_client_add_static_channel(rdpSettings* settings, int count, char** p
args = (ADDIN_ARGV*) malloc(sizeof(ADDIN_ARGV));
args->argc = count;
args->argv = (char**) malloc(sizeof(char*) * args->argc);
args->argv = (char**) calloc(args->argc, sizeof(char*));
for (index = 0; index < args->argc; index++)
args->argv[index] = _strdup(params[index]);
@ -491,7 +491,7 @@ int freerdp_client_add_dynamic_channel(rdpSettings* settings, int count, char**
args = (ADDIN_ARGV*) malloc(sizeof(ADDIN_ARGV));
args->argc = count;
args->argv = (char**) malloc(sizeof(char*) * args->argc);
args->argv = (char**) calloc(args->argc, sizeof(char*));
for (index = 0; index < args->argc; index++)
args->argv[index] = _strdup(params[index]);
@ -521,8 +521,7 @@ static char** freerdp_command_line_parse_comma_separated_values(char* list, int*
nCommas += (list[index] == ',') ? 1 : 0;
nArgs = nCommas + 1;
p = (char**) malloc(sizeof(char*) * (nArgs + 1));
ZeroMemory(p, sizeof(char*) * (nArgs + 1));
p = (char**) calloc((nArgs + 1UL), sizeof(char*));
str = (char*) list;
@ -804,7 +803,7 @@ int freerdp_parse_username(char* username, char** user, char** domain)
if (p)
{
length = (int) (p - username);
*domain = (char*) malloc(length + 1);
*domain = (char*) calloc(length + 1UL, sizeof(char));
strncpy(*domain, username, length);
(*domain)[length] = '\0';
*user = _strdup(&p[1]);
@ -832,7 +831,7 @@ int freerdp_parse_hostname(char* hostname, char** host, int* port)
if (p)
{
length = (p - hostname);
*host = (char*) malloc(length + 1);
*host = (char*) calloc(length + 1UL, sizeof(char));
if (!(*host))
return -1;
@ -1235,7 +1234,7 @@ int freerdp_client_settings_parse_command_line_arguments(rdpSettings* settings,
{
length = (int) (p - arg->Value);
settings->ServerPort = atoi(&p[1]);
settings->ServerHostname = (char*) malloc(length + 1);
settings->ServerHostname = (char*) calloc(length + 1UL, sizeof(char));
strncpy(settings->ServerHostname, arg->Value, length);
settings->ServerHostname[length] = '\0';
}
@ -1471,7 +1470,7 @@ int freerdp_client_settings_parse_command_line_arguments(rdpSettings* settings,
{
length = (int) (p - arg->Value);
settings->GatewayPort = atoi(&p[1]);
settings->GatewayHostname = (char*) malloc(length + 1);
settings->GatewayHostname = (char*) calloc(length + 1UL, sizeof(char));
strncpy(settings->GatewayHostname, arg->Value, length);
settings->GatewayHostname[length] = '\0';
}

View File

@ -45,7 +45,11 @@ static BOOL rail_read_unicode_string(wStream* s, RAIL_UNICODE_STRING* unicode_st
new_str = (BYTE*) realloc(unicode_string->string, new_len);
if (!new_str)
{
free (unicode_string->string);
unicode_string->string = NULL;
return FALSE;
}
unicode_string->string = new_str;
unicode_string->length = new_len;
@ -99,7 +103,11 @@ BOOL update_read_icon_info(wStream* s, ICON_INFO* iconInfo)
/* bitsMask */
newBitMask = (BYTE*) realloc(iconInfo->bitsMask, iconInfo->cbBitsMask);
if (!newBitMask)
{
free (iconInfo->bitsMask);
iconInfo->bitsMask = NULL;
return FALSE;
}
iconInfo->bitsMask = newBitMask;
Stream_Read(s, iconInfo->bitsMask, iconInfo->cbBitsMask);
@ -116,7 +124,11 @@ BOOL update_read_icon_info(wStream* s, ICON_INFO* iconInfo)
new_tab = (BYTE*) realloc(iconInfo->colorTable, iconInfo->cbColorTable);
if (!new_tab)
{
free (iconInfo->colorTable);
iconInfo->colorTable = NULL;
return FALSE;
}
iconInfo->colorTable = new_tab;
}
else
@ -131,7 +143,11 @@ BOOL update_read_icon_info(wStream* s, ICON_INFO* iconInfo)
/* bitsColor */
newBitMask = (BYTE *)realloc(iconInfo->bitsColor, iconInfo->cbBitsColor);
if (!newBitMask)
{
free (iconInfo->bitsColor);
iconInfo->bitsColor = NULL;
return FALSE;
}
iconInfo->bitsColor = newBitMask;
Stream_Read(s, iconInfo->bitsColor, iconInfo->cbBitsColor);
@ -499,7 +515,12 @@ BOOL update_read_desktop_actively_monitored_order(wStream* s, WINDOW_ORDER_INFO*
newid = (UINT32*) realloc(monitored_desktop->windowIds, size);
if (!newid)
{
free (monitored_desktop->windowIds);
monitored_desktop->windowIds = NULL;
return FALSE;
}
monitored_desktop->windowIds = newid;
/* windowIds */
for (i = 0; i < (int) monitored_desktop->numWindowIds; i++)

View File

@ -1451,7 +1451,7 @@ BOOL CommIsHandled(HANDLE handle)
pComm = (WINPR_COMM*)handle;
if (!pComm || pComm->Type != HANDLE_TYPE_COMM)
if (!pComm || (pComm->Type != HANDLE_TYPE_COMM) || (pComm == INVALID_HANDLE_VALUE))
{
SetLastError(ERROR_INVALID_HANDLE);
return FALSE;

View File

@ -208,7 +208,7 @@ static BOOL FileIsHandled(HANDLE handle)
{
WINPR_NAMED_PIPE* pFile = (WINPR_NAMED_PIPE*) handle;
if (!pFile || pFile->Type != HANDLE_TYPE_NAMED_PIPE)
if (!pFile || (pFile->Type != HANDLE_TYPE_NAMED_PIPE) || (pFile == INVALID_HANDLE_VALUE))
{
SetLastError(ERROR_INVALID_HANDLE);
return FALSE;

View File

@ -86,7 +86,7 @@ static BOOL PipeIsHandled(HANDLE handle)
{
WINPR_PIPE* pPipe = (WINPR_PIPE*) handle;
if (!pPipe || pPipe->Type != HANDLE_TYPE_ANONYMOUS_PIPE)
if (!pPipe || (pPipe->Type != HANDLE_TYPE_ANONYMOUS_PIPE))
{
SetLastError(ERROR_INVALID_HANDLE);
return FALSE;
@ -190,7 +190,7 @@ static BOOL NamedPipeIsHandled(HANDLE handle)
{
WINPR_NAMED_PIPE* pPipe = (WINPR_NAMED_PIPE*) handle;
if (!pPipe || pPipe->Type != HANDLE_TYPE_NAMED_PIPE)
if (!pPipe || (pPipe->Type != HANDLE_TYPE_NAMED_PIPE) || (pPipe == INVALID_HANDLE_VALUE))
{
SetLastError(ERROR_INVALID_HANDLE);
return FALSE;

View File

@ -75,7 +75,7 @@ static BOOL LogonUserIsHandled(HANDLE handle)
{
WINPR_ACCESS_TOKEN* pLogonUser = (WINPR_ACCESS_TOKEN*) handle;
if (!pLogonUser || pLogonUser->Type != HANDLE_TYPE_ACCESS_TOKEN)
if (!pLogonUser || (pLogonUser->Type != HANDLE_TYPE_ACCESS_TOKEN))
{
SetLastError(ERROR_INVALID_HANDLE);
return FALSE;

View File

@ -54,7 +54,7 @@ static BOOL EventIsHandled(HANDLE handle)
{
WINPR_TIMER* pEvent = (WINPR_TIMER*) handle;
if (!pEvent || pEvent->Type != HANDLE_TYPE_EVENT)
if (!pEvent || (pEvent->Type != HANDLE_TYPE_EVENT))
{
SetLastError(ERROR_INVALID_HANDLE);
return FALSE;

View File

@ -34,7 +34,7 @@ static BOOL MutexIsHandled(HANDLE handle)
{
WINPR_TIMER* pMutex = (WINPR_TIMER*) handle;
if (!pMutex || pMutex->Type != HANDLE_TYPE_MUTEX)
if (!pMutex || (pMutex->Type != HANDLE_TYPE_MUTEX))
{
SetLastError(ERROR_INVALID_HANDLE);
return FALSE;

View File

@ -42,7 +42,7 @@ static BOOL SemaphoreIsHandled(HANDLE handle)
{
WINPR_TIMER* pSemaphore = (WINPR_TIMER*) handle;
if (!pSemaphore || pSemaphore->Type != HANDLE_TYPE_SEMAPHORE)
if (!pSemaphore || (pSemaphore->Type != HANDLE_TYPE_SEMAPHORE))
{
SetLastError(ERROR_INVALID_HANDLE);
return FALSE;

View File

@ -49,7 +49,7 @@ static BOOL TimerIsHandled(HANDLE handle)
{
WINPR_TIMER* pTimer = (WINPR_TIMER*) handle;
if (!pTimer || pTimer->Type != HANDLE_TYPE_TIMER)
if (!pTimer || (pTimer->Type != HANDLE_TYPE_TIMER))
{
SetLastError(ERROR_INVALID_HANDLE);
return FALSE;

View File

@ -102,7 +102,7 @@ static BOOL ThreadIsHandled(HANDLE handle)
{
WINPR_THREAD* pThread = (WINPR_THREAD*) handle;
if (!pThread || pThread->Type != HANDLE_TYPE_THREAD)
if (!pThread || (pThread->Type != HANDLE_TYPE_THREAD))
{
SetLastError(ERROR_INVALID_HANDLE);
return FALSE;

View File

@ -319,6 +319,10 @@ int WLog_ParseFilter(wLogFilter* filter, LPCSTR name)
LPSTR names;
int iLevel;
count = 1;
if(!name)
return -1;
p = (char*) name;
if (p)
@ -331,8 +335,16 @@ int WLog_ParseFilter(wLogFilter* filter, LPCSTR name)
}
names = _strdup(name);
if (!names)
return -1;
filter->NameCount = count;
filter->Names = (LPSTR*) malloc(sizeof(LPSTR) * (count + 1));
filter->Names = (LPSTR*) calloc((count + 1UL), sizeof(LPSTR));
if(!filter->Names)
{
free(names);
filter->NameCount = 0;
return -1;
}
filter->Names[count] = NULL;
count = 0;
p = (char*) names;
@ -340,20 +352,33 @@ int WLog_ParseFilter(wLogFilter* filter, LPCSTR name)
q = strrchr(p, ':');
if (!q)
{
free(names);
free(filter->Names);
filter->Names = NULL;
filter->NameCount = 0;
return -1;
}
*q = '\0';
q++;
iLevel = WLog_ParseLogLevel(q);
if (iLevel < 0)
{
free(names);
free(filter->Names);
filter->Names = NULL;
filter->NameCount = 0;
return -1;
}
filter->Level = (DWORD) iLevel;
while ((p = strchr(p, '.')) != NULL)
{
filter->Names[count++] = p + 1;
if (count < filter->NameCount)
filter->Names[count++] = p + 1;
*p = '\0';
p++;
}
@ -368,7 +393,7 @@ int WLog_ParseFilters()
DWORD count;
DWORD nSize;
int status;
char** strs;
LPCSTR* strs;
nSize = GetEnvironmentVariableA("WLOG_FILTER", NULL, 0);
@ -392,8 +417,9 @@ int WLog_ParseFilters()
g_FilterCount = count;
p = env;
count = 0;
strs = (char**) calloc(g_FilterCount, sizeof(char*));
strs = (LPCSTR*) calloc(g_FilterCount, sizeof(LPCSTR));
if (!strs)
{
@ -405,7 +431,8 @@ int WLog_ParseFilters()
while ((p = strchr(p, ',')) != NULL)
{
strs[count++] = p + 1;
if (count < g_FilterCount)
strs[count++] = p + 1;
*p = '\0';
p++;
}
@ -496,7 +523,8 @@ int WLog_ParseName(wLog* log, LPCSTR name)
if (!names)
return -1;
log->NameCount = count;
if(!(log->Names = (LPSTR*) malloc(sizeof(LPSTR) * (count + 1))))
log->Names = (LPSTR*) calloc((count + 1UL), sizeof(LPSTR));
if(!log->Names)
{
free(names);
return -1;
@ -508,7 +536,8 @@ int WLog_ParseName(wLog* log, LPCSTR name)
while ((p = strchr(p, '.')) != NULL)
{
log->Names[count++] = p + 1;
if (count < log->NameCount)
log->Names[count++] = p + 1;
*p = '\0';
p++;
}