From ec699f6c75b179409221cbc215a97eaa930bf2a1 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 28 Apr 2022 08:00:39 +0200 Subject: [PATCH] scanbuild fixes --- channels/client/addin.c | 7 +- channels/disp/client/disp_main.c | 4 +- channels/printer/client/cups/printer_cups.c | 7 +- channels/rdpei/server/rdpei_main.c | 2 +- channels/video/client/video_main.c | 3 + client/X11/xf_cliprdr.c | 20 ++- client/common/cmdline.c | 4 + libfreerdp/cache/offscreen.c | 28 ++-- libfreerdp/cache/pointer.c | 9 +- libfreerdp/codec/h264.c | 44 +++-- libfreerdp/codec/progressive.c | 72 +++++--- libfreerdp/codec/progressive.h | 1 + libfreerdp/common/settings.c | 156 +++++++++++++----- libfreerdp/core/certificate.c | 9 +- libfreerdp/core/display.c | 16 +- libfreerdp/core/gateway/rts.c | 6 +- libfreerdp/core/nla.c | 7 +- libfreerdp/core/orders.c | 5 +- libfreerdp/core/server.c | 16 +- libfreerdp/core/settings.c | 149 ++++++----------- libfreerdp/utils/smartcard_call.c | 8 +- libfreerdp/utils/smartcard_pack.c | 4 +- server/shadow/shadow_server.c | 1 + winpr/libwinpr/sspi/Kerberos/kerberos.c | 2 + winpr/libwinpr/thread/apc.c | 51 ++++-- winpr/libwinpr/thread/argv.c | 2 +- winpr/libwinpr/utils/collections/BufferPool.c | 5 +- 27 files changed, 394 insertions(+), 244 deletions(-) diff --git a/channels/client/addin.c b/channels/client/addin.c index 0b5d8d65b..016cd460a 100644 --- a/channels/client/addin.c +++ b/channels/client/addin.c @@ -508,10 +508,9 @@ static DWORD WINAPI channel_client_thread_proc(LPVOID userdata) wStream* data; wMessage message; msg_proc_internals* internals = userdata; - if (!internals) - { - /* TODO: return some error */ - } + + WINPR_ASSERT(internals); + while (1) { if (!MessageQueue_Wait(internals->queue)) diff --git a/channels/disp/client/disp_main.c b/channels/disp/client/disp_main.c index 138ea9fd7..3bd8ec640 100644 --- a/channels/disp/client/disp_main.c +++ b/channels/disp/client/disp_main.c @@ -343,7 +343,9 @@ static UINT disp_plugin_terminated(IWTSPlugin* pPlugin) { DISP_PLUGIN* disp = (DISP_PLUGIN*)pPlugin; - if (disp && disp->listener_callback) + WINPR_ASSERT(disp); + + if (disp->listener_callback) { IWTSVirtualChannelManager* mgr = disp->listener_callback->channel_mgr; if (mgr) diff --git a/channels/printer/client/cups/printer_cups.c b/channels/printer/client/cups/printer_cups.c index 87e96add0..6e65f6560 100644 --- a/channels/printer/client/cups/printer_cups.c +++ b/channels/printer/client/cups/printer_cups.c @@ -342,8 +342,11 @@ static rdpPrinter** printer_cups_enum_printers(rdpPrinterDriver* driver) } cupsFreeDests(num_dests, dests); - if (!haveDefault && (num_dests > 0)) - printers[0]->is_default = TRUE; + if (!haveDefault && (num_dests > 0) && printers) + { + if (printers[0]) + printers[0]->is_default = TRUE; + } return printers; } diff --git a/channels/rdpei/server/rdpei_main.c b/channels/rdpei/server/rdpei_main.c index a8f376fee..5048d2c70 100644 --- a/channels/rdpei/server/rdpei_main.c +++ b/channels/rdpei/server/rdpei_main.c @@ -341,7 +341,7 @@ static UINT read_pen_frame(RdpeiServerContext* context, wStream* s, RDPINPUT_PEN return ERROR_INTERNAL_ERROR; } - frame->contacts = contact = calloc(frame->contactCount, sizeof(RDPINPUT_CONTACT_DATA)); + frame->contacts = contact = calloc(frame->contactCount, sizeof(RDPINPUT_PEN_CONTACT)); if (!frame->contacts) { WLog_ERR(TAG, "calloc failed!"); diff --git a/channels/video/client/video_main.c b/channels/video/client/video_main.c index 373e56ed4..79d49e2a1 100644 --- a/channels/video/client/video_main.c +++ b/channels/video/client/video_main.c @@ -901,7 +901,10 @@ static UINT video_VideoData(VideoClientContext* context, const TSMM_VIDEO_DATA* frame->surfaceData, surface->format, surface->scanline, surface->alignedWidth, surface->alignedHeight, &rect, 1); if (status < 0) + { + VideoFrame_free(&frame); return CHANNEL_RC_OK; + } InterlockedIncrement(&presentation->refCounter); diff --git a/client/X11/xf_cliprdr.c b/client/X11/xf_cliprdr.c index 117d51052..25679f1af 100644 --- a/client/X11/xf_cliprdr.c +++ b/client/X11/xf_cliprdr.c @@ -600,11 +600,15 @@ static void xf_cliprdr_provide_server_format_list(xfClipboard* clipboard) static BOOL xf_clipboard_format_equal(const CLIPRDR_FORMAT* a, const CLIPRDR_FORMAT* b) { + WINPR_ASSERT(a); + WINPR_ASSERT(b); + if (a->formatId != b->formatId) return FALSE; if (!a->formatName && !b->formatName) return TRUE; - + if (!a->formatName || !b->formatName) + return FALSE; return strcmp(a->formatName, b->formatName) == 0; } static BOOL xf_clipboard_changed(xfClipboard* clipboard, const CLIPRDR_FORMAT* formats, @@ -1772,7 +1776,7 @@ static char* xf_cliprdr_fuse_split_basename(char* name, int len) return NULL; } -static xfCliprdrFuseInode* xf_cliprdr_fuse_create_root_node() +static xfCliprdrFuseInode* xf_cliprdr_fuse_create_root_node(void) { xfCliprdrFuseInode* rootNode = (xfCliprdrFuseInode*)calloc(1, sizeof(xfCliprdrFuseInode)); if (!rootNode) @@ -1810,7 +1814,7 @@ static BOOL xf_cliprdr_fuse_check_stream(wStream* s, size_t count) } static BOOL xf_cliprdr_fuse_create_nodes(xfClipboard* clipboard, wStream* s, size_t count, - xfCliprdrFuseInode* rootNode) + const xfCliprdrFuseInode* rootNode) { BOOL status = FALSE; size_t lindex = 0; @@ -1818,11 +1822,17 @@ static BOOL xf_cliprdr_fuse_create_nodes(xfClipboard* clipboard, wStream* s, siz char* dirName = NULL; char* baseName = NULL; xfCliprdrFuseInode* inode = NULL; - wHashTable* mapDir = HashTable_New(TRUE); + wHashTable* mapDir; + + WINPR_ASSERT(clipboard); + WINPR_ASSERT(s); + WINPR_ASSERT(rootNode); + + mapDir = HashTable_New(TRUE); if (!mapDir) { WLog_ERR(TAG, "fail to alloc hashtable"); - return FALSE; + goto error; } if (!HashTable_SetupForStringData(mapDir, FALSE)) goto error; diff --git a/client/common/cmdline.c b/client/common/cmdline.c index 16efa1733..5e2465e90 100644 --- a/client/common/cmdline.c +++ b/client/common/cmdline.c @@ -2197,6 +2197,10 @@ int freerdp_client_settings_parse_command_line_arguments(rdpSettings* settings, } CommandLineSwitchCase(arg, "gt") { + if ((arg->Flags & COMMAND_LINE_VALUE_PRESENT) == 0) + return COMMAND_LINE_ERROR_UNEXPECTED_VALUE; + + WINPR_ASSERT(arg->Value); if (_stricmp(arg->Value, "rpc") == 0) { if (!freerdp_settings_set_bool(settings, FreeRDP_GatewayRpcTransport, TRUE) || diff --git a/libfreerdp/cache/offscreen.c b/libfreerdp/cache/offscreen.c index fadfccc0d..75f1eee8c 100644 --- a/libfreerdp/cache/offscreen.c +++ b/libfreerdp/cache/offscreen.c @@ -205,30 +205,34 @@ rdpOffscreenCache* offscreen_cache_new(rdpContext* context) offscreenCache->currentSurface = SCREEN_BITMAP_SURFACE; offscreenCache->maxSize = 7680; offscreenCache->maxEntries = 2000; - settings->OffscreenCacheSize = offscreenCache->maxSize; - settings->OffscreenCacheEntries = offscreenCache->maxEntries; + if (!freerdp_settings_set_uint32(settings, FreeRDP_OffscreenCacheSize, offscreenCache->maxSize)) + goto fail; + if (!freerdp_settings_set_uint32(settings, FreeRDP_OffscreenCacheEntries, + offscreenCache->maxEntries)) + goto fail; offscreenCache->entries = (rdpBitmap**)calloc(offscreenCache->maxEntries, sizeof(rdpBitmap*)); if (!offscreenCache->entries) - { - free(offscreenCache); - return NULL; - } + goto fail; return offscreenCache; +fail: + offscreen_cache_free(offscreenCache); + return NULL; } void offscreen_cache_free(rdpOffscreenCache* offscreenCache) { - size_t i; - rdpBitmap* bitmap; - if (offscreenCache) { - for (i = 0; i < offscreenCache->maxEntries; i++) + size_t i; + if (offscreenCache->entries) { - bitmap = offscreenCache->entries[i]; - Bitmap_Free(offscreenCache->context, bitmap); + for (i = 0; i < offscreenCache->maxEntries; i++) + { + rdpBitmap* bitmap = offscreenCache->entries[i]; + Bitmap_Free(offscreenCache->context, bitmap); + } } free(offscreenCache->entries); diff --git a/libfreerdp/cache/pointer.c b/libfreerdp/cache/pointer.c index b71f8193f..68b782c8f 100644 --- a/libfreerdp/cache/pointer.c +++ b/libfreerdp/cache/pointer.c @@ -362,10 +362,13 @@ void pointer_cache_free(rdpPointerCache* pointer_cache) UINT32 i; rdpPointer* pointer; - for (i = 0; i < pointer_cache->cacheSize; i++) + if (pointer_cache->entries) { - pointer = pointer_cache->entries[i]; - pointer_free(pointer_cache->context, pointer); + for (i = 0; i < pointer_cache->cacheSize; i++) + { + pointer = pointer_cache->entries[i]; + pointer_free(pointer_cache->context, pointer); + } } free(pointer_cache->entries); diff --git a/libfreerdp/codec/h264.c b/libfreerdp/codec/h264.c index fa9df2dba..044350da8 100644 --- a/libfreerdp/codec/h264.c +++ b/libfreerdp/codec/h264.c @@ -229,10 +229,10 @@ INT32 avc420_compress(H264_CONTEXT* h264, const BYTE* pSrcData, DWORD SrcFormat, BYTE** ppDstData, UINT32* pDstSize, RDPGFX_H264_METABLOCK* meta) { size_t x; - INT32 rc; - BYTE* pYUVData[3]; - const BYTE* pcYUVData[3]; - BYTE* pOldYUVData[3]; + INT32 rc = -1; + BYTE* pYUVData[3] = { 0 }; + const BYTE* pcYUVData[3] = { 0 }; + BYTE* pOldYUVData[3] = { 0 }; if (!h264 || !regionRect || !meta || !h264->Compressor) return -1; @@ -263,14 +263,17 @@ INT32 avc420_compress(H264_CONTEXT* h264, const BYTE* pSrcData, DWORD SrcFormat, if (!yuv420_context_encode(h264->yuv, pSrcData, nSrcStep, SrcFormat, h264->iStride, pYUVData, regionRect, 1)) - return -1; + goto fail; if (!detect_changes(h264->firstLumaFrameDone, h264->QP, regionRect, pYUVData, pOldYUVData, h264->iStride, meta)) - return -1; + goto fail; if (meta->numRegionRects == 0) - return 0; + { + rc = 0; + goto fail; + } for (x = 0; x < 3; x++) pcYUVData[x] = pYUVData[x]; @@ -278,6 +281,10 @@ INT32 avc420_compress(H264_CONTEXT* h264, const BYTE* pSrcData, DWORD SrcFormat, rc = h264->subsystem->Compress(h264, pcYUVData, h264->iStride, ppDstData, pDstSize); if (rc >= 0) h264->firstLumaFrameDone = TRUE; + +fail: + if (rc < 0) + free_h264_metablock(meta); return rc; } @@ -287,6 +294,7 @@ INT32 avc444_compress(H264_CONTEXT* h264, const BYTE* pSrcData, DWORD SrcFormat, UINT32* pAuxDstSize, RDPGFX_H264_METABLOCK* meta, RDPGFX_H264_METABLOCK* auxMeta) { + int rc = -1; BYTE* coded; UINT32 codedSize; BYTE** pYUV444Data; @@ -324,14 +332,14 @@ INT32 avc444_compress(H264_CONTEXT* h264, const BYTE* pSrcData, DWORD SrcFormat, if (!yuv444_context_encode(h264->yuv, version, pSrcData, nSrcStep, SrcFormat, h264->iStride, pYUV444Data, pYUVData, region, 1)) - return -1; + goto fail; if (!detect_changes(h264->firstLumaFrameDone, h264->QP, region, pYUV444Data, pOldYUV444Data, h264->iStride, meta)) - return -1; + goto fail; if (!detect_changes(h264->firstChromaFrameDone, h264->QP, region, pYUVData, pOldYUVData, h264->iStride, auxMeta)) - return -1; + goto fail; /* [MS-RDPEGFX] 2.2.4.5 RFX_AVC444_BITMAP_STREAM * LC: @@ -348,7 +356,8 @@ INT32 avc444_compress(H264_CONTEXT* h264, const BYTE* pSrcData, DWORD SrcFormat, else { WLog_INFO(TAG, "no changes detected for luma or chroma frame"); - return 0; + rc = 0; + goto fail; } if ((*op == 0) || (*op == 1)) @@ -356,7 +365,7 @@ INT32 avc444_compress(H264_CONTEXT* h264, const BYTE* pSrcData, DWORD SrcFormat, const BYTE* pcYUV444Data[3] = { pYUV444Data[0], pYUV444Data[1], pYUV444Data[2] }; if (h264->subsystem->Compress(h264, pcYUV444Data, h264->iStride, &coded, &codedSize) < 0) - return -1; + goto fail; h264->firstLumaFrameDone = TRUE; memcpy(h264->lumaData, coded, codedSize); *ppDstData = h264->lumaData; @@ -368,13 +377,20 @@ INT32 avc444_compress(H264_CONTEXT* h264, const BYTE* pSrcData, DWORD SrcFormat, const BYTE* pcYUVData[3] = { pYUVData[0], pYUVData[1], pYUVData[2] }; if (h264->subsystem->Compress(h264, pcYUVData, h264->iStride, &coded, &codedSize) < 0) - return -1; + goto fail; h264->firstChromaFrameDone = TRUE; *ppAuxDstData = coded; *pAuxDstSize = codedSize; } - return 1; + rc = 1; +fail: + if (rc < 0) + { + free_h264_metablock(meta); + free_h264_metablock(auxMeta); + } + return rc; } static BOOL avc444_ensure_buffer(H264_CONTEXT* h264, DWORD nDstHeight) diff --git a/libfreerdp/codec/progressive.c b/libfreerdp/codec/progressive.c index f124277e7..8dec836eb 100644 --- a/libfreerdp/codec/progressive.c +++ b/libfreerdp/codec/progressive.c @@ -390,10 +390,13 @@ static void progressive_surface_context_free(void* ptr) if (!surface) return; - for (index = 0; index < surface->gridSize; index++) + if (surface->tiles) { - RFX_PROGRESSIVE_TILE* tile = &(surface->tiles[index]); - progressive_tile_free(tile); + for (index = 0; index < surface->tilesSize; index++) + { + RFX_PROGRESSIVE_TILE* tile = &(surface->tiles[index]); + progressive_tile_free(tile); + } } free(surface->tiles); @@ -403,10 +406,12 @@ static void progressive_surface_context_free(void* ptr) static INLINE BOOL progressive_tile_allocate(RFX_PROGRESSIVE_TILE* tile) { + const RFX_PROGRESSIVE_TILE empty = { 0 }; BOOL rc = FALSE; if (!tile) return FALSE; + *tile = empty; tile->width = 64; tile->height = 64; tile->stride = 4 * tile->width; @@ -438,30 +443,38 @@ static INLINE BOOL progressive_tile_allocate(RFX_PROGRESSIVE_TILE* tile) static BOOL progressive_allocate_tile_cache(PROGRESSIVE_SURFACE_CONTEXT* surface) { - size_t oldIndex; + size_t x; + size_t oldIndex = 0; WINPR_ASSERT(surface); WINPR_ASSERT(surface->gridSize > 0); - oldIndex = surface->gridSize; if (surface->tiles) + { + oldIndex = surface->gridSize; surface->gridSize *= 2; + } { + const RFX_PROGRESSIVE_TILE empty = { 0 }; + void* tmp = realloc(surface->tiles, surface->gridSize * sizeof(RFX_PROGRESSIVE_TILE)); if (!tmp) return FALSE; + surface->tilesSize = surface->gridSize; surface->tiles = tmp; - memset(&surface->tiles[oldIndex], 0, - (surface->gridSize - oldIndex) * sizeof(RFX_PROGRESSIVE_TILE)); + + for (x = oldIndex; x < surface->tilesSize; x++) + surface->tiles[x] = empty; } { void* tmp = realloc(surface->updatedTileIndices, surface->gridSize * sizeof(UINT32)); if (!tmp) return FALSE; + surface->updatedTileIndices = tmp; - memset(&surface->updatedTileIndices[oldIndex], 0, - (surface->gridSize - oldIndex) * sizeof(UINT32)); + for (x = oldIndex; x < surface->gridSize; x++) + surface->updatedTileIndices[x] = 0; } return TRUE; } @@ -488,7 +501,7 @@ static PROGRESSIVE_SURFACE_CONTEXT* progressive_surface_context_new(UINT16 surfa progressive_surface_context_free(surface); return NULL; } - for (x = 0; x < surface->gridSize; x++) + for (x = 0; x < surface->tilesSize; x++) { if (!progressive_tile_allocate(&surface->tiles[x])) { @@ -512,7 +525,7 @@ static BOOL progressive_surface_tile_replace(PROGRESSIVE_SURFACE_CONTEXT* surfac zIdx = (tile->yIdx * surface->gridWidth) + tile->xIdx; - if (zIdx >= surface->gridSize) + if (zIdx >= surface->tilesSize) { WLog_ERR(TAG, "Invalid zIndex %" PRIuz, zIdx); return FALSE; @@ -575,7 +588,7 @@ static BOOL progressive_surface_tile_replace(PROGRESSIVE_SURFACE_CONTEXT* surfac region->numTiles, region->usedTiles); return FALSE; } - if (surface->numUpdatedTiles >= surface->gridSize) + if (surface->numUpdatedTiles >= surface->tilesSize) { if (!progressive_allocate_tile_cache(surface)) return FALSE; @@ -589,8 +602,7 @@ static BOOL progressive_surface_tile_replace(PROGRESSIVE_SURFACE_CONTEXT* surfac INT32 progressive_create_surface_context(PROGRESSIVE_CONTEXT* progressive, UINT16 surfaceId, UINT32 width, UINT32 height) { - PROGRESSIVE_SURFACE_CONTEXT* surface; - surface = progressive_get_surface_data(progressive, surfaceId); + PROGRESSIVE_SURFACE_CONTEXT* surface = progressive_get_surface_data(progressive, surfaceId); if (!surface) { @@ -1688,6 +1700,9 @@ static INLINE int progressive_process_tiles(PROGRESSIVE_CONTEXT* progressive, wS PROGRESSIVE_TILE_PROCESS_WORK_PARAM* params = NULL; UINT16 close_cnt = 0; + WINPR_ASSERT(progressive); + WINPR_ASSERT(region); + if (!Stream_CheckAndLogRequiredLength(TAG, s, region->tileDataSize)) return -1; @@ -1768,23 +1783,25 @@ static INLINE int progressive_process_tiles(PROGRESSIVE_CONTEXT* progressive, wS work_objects = (PTP_WORK*)calloc(region->numTiles, sizeof(PTP_WORK)); if (!work_objects) return -1; - - params = (PROGRESSIVE_TILE_PROCESS_WORK_PARAM*)calloc( - region->numTiles, sizeof(PROGRESSIVE_TILE_PROCESS_WORK_PARAM)); - if (!params) - { - free(work_objects); - return -1; - } } + params = (PROGRESSIVE_TILE_PROCESS_WORK_PARAM*)calloc( + region->numTiles, sizeof(PROGRESSIVE_TILE_PROCESS_WORK_PARAM)); + if (!params) + { + free(work_objects); + return -1; + } + + WINPR_ASSERT(region->tiles || (region->numTiles == 0)); for (index = 0; index < region->numTiles; index++) { RFX_PROGRESSIVE_TILE* tile = region->tiles[index]; - params[index].progressive = progressive; - params[index].region = region; - params[index].context = context; - params[index].tile = tile; + PROGRESSIVE_TILE_PROCESS_WORK_PARAM* param = ¶ms[index]; + param->progressive = progressive; + param->region = region; + param->context = context; + param->tile = tile; if (progressive->rfx_context->priv->UseThreads) { @@ -1809,7 +1826,7 @@ static INLINE int progressive_process_tiles(PROGRESSIVE_CONTEXT* progressive, wS { WLog_Print(progressive->log, WLOG_ERROR, "Failed to decompress %s at %" PRIu16, progressive_get_block_type_string(tile->blockType), index); - return -1; + goto fail; } } @@ -1826,6 +1843,7 @@ static INLINE int progressive_process_tiles(PROGRESSIVE_CONTEXT* progressive, wS } } +fail: free(work_objects); free(params); diff --git a/libfreerdp/codec/progressive.h b/libfreerdp/codec/progressive.h index ac74e9aea..eb287aca2 100644 --- a/libfreerdp/codec/progressive.h +++ b/libfreerdp/codec/progressive.h @@ -191,6 +191,7 @@ typedef struct UINT32 gridHeight; UINT32 gridSize; RFX_PROGRESSIVE_TILE* tiles; + size_t tilesSize; UINT32 frameId; UINT32 numUpdatedTiles; UINT32* updatedTileIndices; diff --git a/libfreerdp/common/settings.c b/libfreerdp/common/settings.c index 223f680c7..36023aaa5 100644 --- a/libfreerdp/common/settings.c +++ b/libfreerdp/common/settings.c @@ -240,6 +240,8 @@ RDPDR_DEVICE* freerdp_device_collection_find(rdpSettings* settings, const char* UINT32 index; RDPDR_DEVICE* device; + WINPR_ASSERT(settings); + WINPR_ASSERT(name); for (index = 0; index < settings->DeviceCount; index++) { device = (RDPDR_DEVICE*)settings->DeviceArray[index]; @@ -258,6 +260,7 @@ RDPDR_DEVICE* freerdp_device_collection_find_type(rdpSettings* settings, UINT32 { UINT32 index; RDPDR_DEVICE* device; + WINPR_ASSERT(settings); for (index = 0; index < settings->DeviceCount; index++) { @@ -583,11 +586,11 @@ BOOL freerdp_static_channel_collection_add(rdpSettings* settings, ADDIN_ARGV* ch count = freerdp_settings_get_uint32(settings, FreeRDP_StaticChannelCount) + 1; if (freerdp_settings_get_uint32(settings, FreeRDP_StaticChannelArraySize) < count) { - UINT32 new_size; - ADDIN_ARGV** new_array; - new_size = freerdp_settings_get_uint32(settings, FreeRDP_StaticChannelArraySize) * 2; + UINT32 new_size = + freerdp_settings_get_uint32(settings, FreeRDP_StaticChannelArraySize) * 2ul; + ADDIN_ARGV** new_array = NULL; if (new_size == 0) - new_size = count * 2; + new_size = count * 2ul; new_array = (ADDIN_ARGV**)realloc(settings->StaticChannelArray, new_size * sizeof(ADDIN_ARGV*)); @@ -680,7 +683,7 @@ BOOL freerdp_dynamic_channel_collection_add(rdpSettings* settings, ADDIN_ARGV* c if (freerdp_settings_get_uint32(settings, FreeRDP_DynamicChannelArraySize) < count) { ADDIN_ARGV** new_array; - size_t size = freerdp_settings_get_uint32(settings, FreeRDP_DynamicChannelArraySize) * 2; + UINT32 size = freerdp_settings_get_uint32(settings, FreeRDP_DynamicChannelArraySize) * 2; if (size == 0) size = count * 2; @@ -800,8 +803,11 @@ void freerdp_target_net_addresses_free(rdpSettings* settings) WINPR_ASSERT(settings); - for (index = 0; index < settings->TargetNetAddressCount; index++) - free(settings->TargetNetAddresses[index]); + if (settings->TargetNetAddresses) + { + for (index = 0; index < settings->TargetNetAddressCount; index++) + free(settings->TargetNetAddresses[index]); + } free(settings->TargetNetAddresses); free(settings->TargetNetPorts); @@ -1179,6 +1185,11 @@ BOOL freerdp_settings_set_pointer_len(rdpSettings* settings, size_t id, const vo return TRUE; case FreeRDP_RdpServerRsaKey: key_free(settings->RdpServerRsaKey); + if (len > 1) + { + WLog_ERR(TAG, "FreeRDP_RdpServerRsaKey::len must be 0 or 1"); + return FALSE; + } settings->RdpServerRsaKey = (rdpRsaKey*)cnv.v; return TRUE; case FreeRDP_RedirectionPassword: @@ -1201,34 +1212,35 @@ BOOL freerdp_settings_set_pointer_len(rdpSettings* settings, size_t id, const vo data, len, sizeof(char)); case FreeRDP_TargetNetAddresses: if (data == NULL) - { freerdp_target_net_addresses_free(settings); - if (!freerdp_settings_set_uint32(settings, FreeRDP_TargetNetAddressCount, len)) - return FALSE; - } return freerdp_settings_set_pointer_len_(settings, FreeRDP_TargetNetAddresses, FreeRDP_TargetNetAddressCount, data, len, sizeof(char)); - case FreeRDP_TargetNetPorts: if (data == NULL) - { freerdp_target_net_addresses_free(settings); - if (!freerdp_settings_set_uint32(settings, FreeRDP_TargetNetAddressCount, len)) - return FALSE; - } return freerdp_settings_set_pointer_len_(settings, FreeRDP_TargetNetPorts, FreeRDP_TargetNetAddressCount, data, len, sizeof(char)); + case FreeRDP_DeviceArray: + if (data == NULL) + freerdp_device_collection_free(settings); + return freerdp_settings_set_pointer_len_(settings, id, FreeRDP_DeviceArraySize, data, + len, sizeof(char)); case FreeRDP_ChannelDefArray: + if ((len > 0) && (len < CHANNEL_MAX_COUNT)) + WLog_WARN(TAG, + "FreeRDP_ChannelDefArray::len expected to be >= %" PRIu32 + ", but have %" PRIu32, + CHANNEL_MAX_COUNT, len); if (!freerdp_settings_set_pointer_len_(settings, FreeRDP_ChannelDefArray, FreeRDP_ChannelDefArraySize, data, len, sizeof(CHANNEL_DEF))) return FALSE; return freerdp_settings_set_uint32(settings, FreeRDP_ChannelCount, len); case FreeRDP_MonitorDefArray: - return freerdp_settings_set_pointer_len_(settings, id, -1, data, len, - sizeof(rdpMonitor)); + return freerdp_settings_set_pointer_len_(settings, id, FreeRDP_MonitorDefArraySize, + data, len, sizeof(rdpMonitor)); case FreeRDP_ClientAutoReconnectCookie: return freerdp_settings_set_pointer_len_(settings, id, -1, data, len, sizeof(ARC_CS_PRIVATE_PACKET)); @@ -1236,13 +1248,44 @@ BOOL freerdp_settings_set_pointer_len(rdpSettings* settings, size_t id, const vo return freerdp_settings_set_pointer_len_(settings, id, -1, data, len, sizeof(ARC_SC_PRIVATE_PACKET)); case FreeRDP_ClientTimeZone: + if (len > 1) + { + WLog_ERR(TAG, "FreeRDP_ClientTimeZone::len must be 0 or 1"); + return FALSE; + } return freerdp_settings_set_pointer_len_(settings, id, -1, data, len, sizeof(TIME_ZONE_INFORMATION)); + case FreeRDP_BitmapCacheV2CellInfo: + return freerdp_settings_set_pointer_len_(settings, id, FreeRDP_BitmapCacheV2NumCells, + data, len, sizeof(BITMAP_CACHE_V2_CELL_INFO)); + case FreeRDP_GlyphCache: + if ((len != 0) && (len != 10)) + { + WLog_ERR(TAG, "FreeRDP_GlyphCache::len must be 0 or 10"); + return FALSE; + } + return freerdp_settings_set_pointer_len_(settings, id, -1, data, len, + sizeof(GLYPH_CACHE_DEFINITION)); + case FreeRDP_FragCache: + if (len > 1) + { + WLog_ERR(TAG, "FreeRDP_FragCache::len must be 0 or 1"); + return FALSE; + } + return freerdp_settings_set_pointer_len_(settings, id, -1, data, len, + sizeof(GLYPH_CACHE_DEFINITION)); + case FreeRDP_StaticChannelArray: + if (data == NULL) + freerdp_static_channel_collection_free(settings); + return freerdp_settings_set_pointer_len_(settings, id, FreeRDP_StaticChannelArraySize, + data, len, sizeof(ADDIN_ARGV*)); + case FreeRDP_DynamicChannelArray: + if (data == NULL) + freerdp_dynamic_channel_collection_free(settings); + return freerdp_settings_set_pointer_len_(settings, id, FreeRDP_DynamicChannelArraySize, + data, len, sizeof(ADDIN_ARGV*)); case FreeRDP_ReceivedCapabilities: case FreeRDP_OrderSupport: - case FreeRDP_BitmapCacheV2CellInfo: - case FreeRDP_GlyphCache: - case FreeRDP_FragCache: return freerdp_settings_set_pointer_len_(settings, id, -1, data, len, sizeof(char)); case FreeRDP_MonitorIds: @@ -1324,60 +1367,95 @@ void* freerdp_settings_get_pointer_array_writable(const rdpSettings* settings, s BOOL freerdp_settings_set_pointer_array(rdpSettings* settings, size_t id, size_t offset, const void* data) { + size_t maxOffset = 0; if (!settings) return FALSE; switch (id) { + case FreeRDP_DeviceArray: + maxOffset = freerdp_settings_get_uint32(settings, FreeRDP_DeviceArraySize); + if ((offset >= maxOffset) || !data) + goto fail; + freerdp_device_free(settings->DeviceArray[offset]); + settings->DeviceArray[offset] = freerdp_device_clone(data); case FreeRDP_TargetNetAddresses: - if ((offset >= settings->TargetNetAddressCount) || !data) - return FALSE; + maxOffset = freerdp_settings_get_uint32(settings, FreeRDP_TargetNetAddressCount); + if ((offset >= maxOffset) || !data) + goto fail; free(settings->TargetNetAddresses[offset]); settings->TargetNetAddresses[offset] = _strdup((const char*)data); return settings->TargetNetAddresses[offset] != NULL; case FreeRDP_TargetNetPorts: - if ((offset >= settings->TargetNetAddressCount) || !data) - return FALSE; + maxOffset = freerdp_settings_get_uint32(settings, FreeRDP_TargetNetAddressCount); + if ((offset >= maxOffset) || !data) + goto fail; settings->TargetNetPorts[offset] = *((const UINT32*)data); return TRUE; + case FreeRDP_StaticChannelArray: + maxOffset = freerdp_settings_get_uint32(settings, FreeRDP_StaticChannelArraySize); + if ((offset >= maxOffset) || !data) + goto fail; + freerdp_addin_argv_free(settings->StaticChannelArray[offset]); + settings->StaticChannelArray[offset] = freerdp_addin_argv_clone(data); + return TRUE; + case FreeRDP_DynamicChannelArray: + maxOffset = freerdp_settings_get_uint32(settings, FreeRDP_DynamicChannelArraySize); + if ((offset >= maxOffset) || !data) + goto fail; + freerdp_addin_argv_free(settings->DynamicChannelArray[offset]); + settings->DynamicChannelArray[offset] = freerdp_addin_argv_clone(data); + return TRUE; case FreeRDP_BitmapCacheV2CellInfo: - if ((offset > 5) || !data) - return FALSE; + maxOffset = 5; + if ((offset >= maxOffset) || !data) + goto fail; settings->BitmapCacheV2CellInfo[offset] = *(const BITMAP_CACHE_V2_CELL_INFO*)data; return TRUE; case FreeRDP_OrderSupport: - if ((offset >= 32) || !data) - return FALSE; + maxOffset = 32; + if ((offset >= maxOffset) || !data) + goto fail; settings->OrderSupport[offset] = *(const BOOL*)data; return TRUE; case FreeRDP_GlyphCache: - if ((offset >= 10) || !data) - return FALSE; + maxOffset = 10; + if ((offset >= maxOffset) || !data) + goto fail; settings->GlyphCache[offset] = *(const GLYPH_CACHE_DEFINITION*)data; return TRUE; case FreeRDP_FragCache: - if ((offset >= 1) || !data) - return FALSE; + maxOffset = 1; + if ((offset >= maxOffset) || !data) + goto fail; settings->FragCache[offset] = *(const GLYPH_CACHE_DEFINITION*)data; return TRUE; case FreeRDP_MonitorIds: - if ((offset > freerdp_settings_get_uint32(settings, FreeRDP_NumMonitorIds)) || !data) - return FALSE; + maxOffset = freerdp_settings_get_uint32(settings, FreeRDP_NumMonitorIds); + if ((offset >= maxOffset) || !data) + goto fail; settings->MonitorIds[offset] = *(const UINT32*)data; return TRUE; case FreeRDP_ChannelDefArray: - if (offset > freerdp_settings_get_uint32(settings, FreeRDP_ChannelDefArraySize)) - return FALSE; + maxOffset = freerdp_settings_get_uint32(settings, FreeRDP_ChannelDefArraySize); + if ((offset >= maxOffset) || !data) + goto fail; settings->ChannelDefArray[offset] = *(const CHANNEL_DEF*)data; return TRUE; case FreeRDP_MonitorDefArray: - if (offset > freerdp_settings_get_uint32(settings, FreeRDP_MonitorDefArraySize)) - return FALSE; + maxOffset = freerdp_settings_get_uint32(settings, FreeRDP_MonitorDefArraySize); + if ((offset >= maxOffset) || !data) + goto fail; settings->MonitorDefArray[offset] = *(const rdpMonitor*)data; return TRUE; default: WLog_WARN(TAG, "Invalid id %" PRIuz " for %s", id, __FUNCTION__); return FALSE; } + +fail: + WLog_WARN(TAG, "[%s] Invalid offset=%" PRIuz " [%" PRIuz "] or NULL data=%p", + freerdp_settings_get_name_for_key(id), offset, maxOffset, data); + return FALSE; } const void* freerdp_settings_get_pointer_array(const rdpSettings* settings, size_t id, diff --git a/libfreerdp/core/certificate.c b/libfreerdp/core/certificate.c index 83cef6dd7..f02078d81 100644 --- a/libfreerdp/core/certificate.c +++ b/libfreerdp/core/certificate.c @@ -352,10 +352,13 @@ static void certificate_free_x509_certificate_chain(rdpX509CertChain* x509_cert_ if (!x509_cert_chain) return; - for (i = 0; i < x509_cert_chain->count; i++) + if (x509_cert_chain->array) { - struct rdp_CertBlob* element = &x509_cert_chain->array[i]; - free(element->data); + for (i = 0; i < x509_cert_chain->count; i++) + { + struct rdp_CertBlob* element = &x509_cert_chain->array[i]; + free(element->data); + } } free(x509_cert_chain->array); diff --git a/libfreerdp/core/display.c b/libfreerdp/core/display.c index 5eb5fdbf6..d9dfaefde 100644 --- a/libfreerdp/core/display.c +++ b/libfreerdp/core/display.c @@ -48,14 +48,21 @@ BOOL display_convert_rdp_monitor_to_monitor_def(UINT32 monitorCount, MONITOR_DEF** result) { UINT32 index; - const rdpMonitor* monitor; + MONITOR_DEF* mdef = NULL; - if (!monitorDefArray || !(*result)) + if (!monitorDefArray || !result || (*result)) return FALSE; - for (index = 0, monitor = monitorDefArray; index < monitorCount; index++, monitor++) + mdef = (MONITOR_DEF*)calloc(monitorCount, sizeof(MONITOR_DEF)); + + if (!mdef) + return FALSE; + + for (index = 0; index < monitorCount; index++) { - MONITOR_DEF* current = (*result + index); + const rdpMonitor* monitor = &monitorDefArray[index]; + MONITOR_DEF* current = &mdef[index]; + current->left = monitor->x; /* left (4 bytes) */ current->top = monitor->y; /* top (4 bytes) */ current->right = monitor->x + monitor->width - 1; /* right (4 bytes) */ @@ -63,6 +70,7 @@ BOOL display_convert_rdp_monitor_to_monitor_def(UINT32 monitorCount, current->flags = monitor->is_primary ? MONITOR_PRIMARY : 0x0; /* flags (4 bytes) */ } + *result = mdef; return TRUE; } diff --git a/libfreerdp/core/gateway/rts.c b/libfreerdp/core/gateway/rts.c index 9a47df74c..39d58e18b 100644 --- a/libfreerdp/core/gateway/rts.c +++ b/libfreerdp/core/gateway/rts.c @@ -1588,9 +1588,9 @@ fail: BOOL rts_recv_CONN_C2_pdu(rdpRpc* rpc, wStream* buffer) { - BOOL rc; - UINT32 ReceiveWindowSize; - UINT32 ConnectionTimeout; + BOOL rc = FALSE; + UINT32 ReceiveWindowSize = 0; + UINT32 ConnectionTimeout = 0; WINPR_ASSERT(rpc); WINPR_ASSERT(buffer); diff --git a/libfreerdp/core/nla.c b/libfreerdp/core/nla.c index 5a1df091a..ab8fd72f6 100644 --- a/libfreerdp/core/nla.c +++ b/libfreerdp/core/nla.c @@ -892,8 +892,8 @@ static BOOL parseKerberosDeltat(const char* value, INT32* dest, const char* mess *dest = 0; do { - INT32 factor; - INT32 maxValue; + INT32 factor = 0; + INT32 maxValue = 0; switch (*value) { @@ -913,6 +913,9 @@ static BOOL parseKerberosDeltat(const char* value, INT32* dest, const char* mess factor = 1; maxValue = 60; break; + default: + WLog_ERR(TAG, "invalid value for unit %c when parsing %s", *value, message); + return FALSE; } if ((maxValue > 0) && (v > maxValue)) diff --git a/libfreerdp/core/orders.c b/libfreerdp/core/orders.c index 413254099..5da8561ae 100644 --- a/libfreerdp/core/orders.c +++ b/libfreerdp/core/orders.c @@ -2512,7 +2512,10 @@ static CACHE_GLYPH_ORDER* update_read_cache_glyph_order(rdpUpdate* update, wStre UINT32 i; CACHE_GLYPH_ORDER* cache_glyph_order = calloc(1, sizeof(CACHE_GLYPH_ORDER)); - if (!cache_glyph_order || !update || !s) + WINPR_ASSERT(update); + WINPR_ASSERT(s); + + if (!cache_glyph_order) goto fail; if (!Stream_CheckAndLogRequiredLength(TAG, s, 2)) diff --git a/libfreerdp/core/server.c b/libfreerdp/core/server.c index fd23885d8..092271259 100644 --- a/libfreerdp/core/server.c +++ b/libfreerdp/core/server.c @@ -1328,27 +1328,29 @@ HANDLE WINAPI FreeRDP_WTSVirtualChannelOpenEx(DWORD SessionId, LPSTR pVirtualNam channel->channelId = InterlockedIncrement(&vcm->dvc_channel_id_seq); if (!ArrayList_Append(vcm->dynamicVirtualChannels, channel)) + { + channel_free(channel); + channel = NULL; goto fail; - + } s = Stream_New(NULL, 64); if (!s) - goto fail2; + goto fail; if (!wts_write_drdynvc_create_request(s, channel->channelId, pVirtualName)) - goto fail2; + goto fail; if (!WTSVirtualChannelWrite(vcm->drdynvc_channel, (PCHAR)Stream_Buffer(s), Stream_GetPosition(s), &written)) - goto fail2; + goto fail; Stream_Free(s, TRUE); return channel; fail: - channel_free(channel); -fail2: Stream_Free(s, TRUE); - ArrayList_Remove(vcm->dynamicVirtualChannels, channel); + if (channel) + ArrayList_Remove(vcm->dynamicVirtualChannels, channel); SetLastError(ERROR_NOT_ENOUGH_MEMORY); return NULL; diff --git a/libfreerdp/core/settings.c b/libfreerdp/core/settings.c index 149723beb..100fd1074 100644 --- a/libfreerdp/core/settings.c +++ b/libfreerdp/core/settings.c @@ -931,105 +931,66 @@ static BOOL freerdp_settings_int_buffer_copy(rdpSettings* _settings, const rdpSe } } - _settings->DeviceCount = settings->DeviceCount; - freerdp_settings_set_uint32(_settings, FreeRDP_DeviceArraySize, - freerdp_settings_get_uint32(settings, FreeRDP_DeviceArraySize)); - _settings->DeviceArray = (RDPDR_DEVICE**)calloc( - freerdp_settings_get_uint32(_settings, FreeRDP_DeviceArraySize), sizeof(RDPDR_DEVICE*)); - - if (!_settings->DeviceArray && freerdp_settings_get_uint32(_settings, FreeRDP_DeviceArraySize)) { - _settings->DeviceCount = 0; - freerdp_settings_set_uint32(_settings, FreeRDP_DeviceArraySize, 0); - goto out_fail; - } + const UINT32 len = freerdp_settings_get_uint32(_settings, FreeRDP_DeviceArraySize); + const UINT32 count = freerdp_settings_get_uint32(settings, FreeRDP_DeviceCount); - if (freerdp_settings_get_uint32(_settings, FreeRDP_DeviceArraySize) < _settings->DeviceCount) - { - _settings->DeviceCount = 0; - freerdp_settings_set_uint32(_settings, FreeRDP_DeviceArraySize, 0); - goto out_fail; - } - - for (index = 0; index < _settings->DeviceCount; index++) - { - _settings->DeviceArray[index] = freerdp_device_clone(settings->DeviceArray[index]); - - if (!_settings->DeviceArray[index]) + if (len < count) goto out_fail; - } - - freerdp_settings_set_uint32(_settings, FreeRDP_StaticChannelCount, - freerdp_settings_get_uint32(settings, FreeRDP_StaticChannelCount)); - freerdp_settings_set_uint32( - _settings, FreeRDP_StaticChannelArraySize, - freerdp_settings_get_uint32(settings, FreeRDP_StaticChannelArraySize)); - _settings->StaticChannelArray = - (ADDIN_ARGV**)calloc(freerdp_settings_get_uint32(_settings, FreeRDP_StaticChannelArraySize), - sizeof(ADDIN_ARGV*)); - - if (!_settings->StaticChannelArray && - freerdp_settings_get_uint32(_settings, FreeRDP_StaticChannelArraySize)) - { - freerdp_settings_set_uint32(_settings, FreeRDP_StaticChannelArraySize, 0); - freerdp_settings_set_uint32(_settings, FreeRDP_ChannelCount, 0); - goto out_fail; - } - - if (freerdp_settings_get_uint32(_settings, FreeRDP_StaticChannelArraySize) < - freerdp_settings_get_uint32(_settings, FreeRDP_StaticChannelCount)) - { - freerdp_settings_set_uint32(_settings, FreeRDP_StaticChannelArraySize, 0); - freerdp_settings_set_uint32(_settings, FreeRDP_ChannelCount, 0); - goto out_fail; - } - - for (index = 0; index < freerdp_settings_get_uint32(_settings, FreeRDP_StaticChannelCount); - index++) - { - _settings->StaticChannelArray[index] = - freerdp_addin_argv_clone(settings->StaticChannelArray[index]); - - if (!_settings->StaticChannelArray[index]) + if (!freerdp_settings_set_uint32(_settings, FreeRDP_DeviceCount, count)) goto out_fail; - } - - if (!freerdp_settings_set_uint32( - _settings, FreeRDP_DynamicChannelCount, - freerdp_settings_get_uint32(settings, FreeRDP_DynamicChannelCount)) || - !freerdp_settings_set_uint32( - _settings, FreeRDP_DynamicChannelArraySize, - freerdp_settings_get_uint32(settings, FreeRDP_DynamicChannelArraySize))) - goto out_fail; - - _settings->DynamicChannelArray = (ADDIN_ARGV**)calloc( - freerdp_settings_get_uint32(_settings, FreeRDP_DynamicChannelArraySize), - sizeof(ADDIN_ARGV*)); - - if (!_settings->DynamicChannelArray && - freerdp_settings_get_uint32(_settings, FreeRDP_DynamicChannelArraySize)) - { - freerdp_settings_set_uint32(_settings, FreeRDP_DynamicChannelCount, 0); - freerdp_settings_set_uint32(_settings, FreeRDP_DynamicChannelArraySize, 0); - goto out_fail; - } - - if (freerdp_settings_get_uint32(_settings, FreeRDP_DynamicChannelArraySize) < - freerdp_settings_get_uint32(_settings, FreeRDP_DynamicChannelCount)) - { - freerdp_settings_set_uint32(_settings, FreeRDP_DynamicChannelCount, 0); - freerdp_settings_set_uint32(_settings, FreeRDP_DynamicChannelArraySize, 0); - goto out_fail; - } - - for (index = 0; index < freerdp_settings_get_uint32(_settings, FreeRDP_DynamicChannelCount); - index++) - { - _settings->DynamicChannelArray[index] = - freerdp_addin_argv_clone(settings->DynamicChannelArray[index]); - - if (!_settings->DynamicChannelArray[index]) + if (!freerdp_settings_set_pointer_len(_settings, FreeRDP_DeviceArray, NULL, len)) goto out_fail; + + for (index = 0; index < count; index++) + { + const RDPDR_DEVICE* argv = + freerdp_settings_get_pointer_array(settings, FreeRDP_DeviceArray, index); + if (!freerdp_settings_set_pointer_array(_settings, FreeRDP_DeviceArray, index, argv)) + goto out_fail; + } + } + { + const UINT32 len = freerdp_settings_get_uint32(_settings, FreeRDP_StaticChannelArraySize); + const UINT32 count = freerdp_settings_get_uint32(settings, FreeRDP_StaticChannelCount); + + if (len < count) + goto out_fail; + if (!freerdp_settings_set_uint32(_settings, FreeRDP_StaticChannelCount, count)) + goto out_fail; + if (!freerdp_settings_set_pointer_len(_settings, FreeRDP_StaticChannelArray, NULL, len)) + goto out_fail; + + for (index = 0; index < count; index++) + { + const ADDIN_ARGV* argv = + freerdp_settings_get_pointer_array(settings, FreeRDP_StaticChannelArray, index); + if (!freerdp_settings_set_pointer_array(_settings, FreeRDP_StaticChannelArray, index, + argv)) + goto out_fail; + } + } + { + const UINT32 len = freerdp_settings_get_uint32(_settings, FreeRDP_DynamicChannelArraySize); + const UINT32 count = freerdp_settings_get_uint32(settings, FreeRDP_DynamicChannelCount); + + if (len < count) + goto out_fail; + if (!freerdp_settings_set_uint32(_settings, FreeRDP_DynamicChannelCount, count)) + goto out_fail; + if (!freerdp_settings_set_uint32(_settings, FreeRDP_DynamicChannelCount, count)) + goto out_fail; + if (!freerdp_settings_set_pointer_len(_settings, FreeRDP_DynamicChannelArray, NULL, len)) + goto out_fail; + + for (index = 0; index < count; index++) + { + const ADDIN_ARGV* argv = + freerdp_settings_get_pointer_array(settings, FreeRDP_DynamicChannelArray, index); + if (!freerdp_settings_set_pointer_array(_settings, FreeRDP_DynamicChannelArray, index, + argv)) + goto out_fail; + } } rc = freerdp_settings_set_string(_settings, FreeRDP_ActionScript, diff --git a/libfreerdp/utils/smartcard_call.c b/libfreerdp/utils/smartcard_call.c index 55b33b947..d04b9b440 100644 --- a/libfreerdp/utils/smartcard_call.c +++ b/libfreerdp/utils/smartcard_call.c @@ -910,7 +910,9 @@ static LONG smartcard_GetStatusChangeA_Call(scard_call_context* smartcard, wStre for (x = 0; x < MAX(1, dwTimeOut); x += dwTimeStep) { - memcpy(rgReaderStates, call->rgReaderStates, call->cReaders * sizeof(SCARD_READERSTATEA)); + if (call->cReaders > 0) + memcpy(rgReaderStates, call->rgReaderStates, + call->cReaders * sizeof(SCARD_READERSTATEA)); ret.ReturnCode = wrap(smartcard, SCardGetStatusChangeA, operation->hContext, MIN(dwTimeOut, dwTimeStep), rgReaderStates, call->cReaders); if (ret.ReturnCode != SCARD_E_TIMEOUT) @@ -971,7 +973,9 @@ static LONG smartcard_GetStatusChangeW_Call(scard_call_context* smartcard, wStre for (x = 0; x < MAX(1, dwTimeOut); x += dwTimeStep) { - memcpy(rgReaderStates, call->rgReaderStates, call->cReaders * sizeof(SCARD_READERSTATEW)); + if (call->cReaders > 0) + memcpy(rgReaderStates, call->rgReaderStates, + call->cReaders * sizeof(SCARD_READERSTATEW)); { ret.ReturnCode = wrap(smartcard, SCardGetStatusChangeW, operation->hContext, MIN(dwTimeOut, dwTimeStep), rgReaderStates, call->cReaders); diff --git a/libfreerdp/utils/smartcard_pack.c b/libfreerdp/utils/smartcard_pack.c index 103b4ee57..1ff82b358 100644 --- a/libfreerdp/utils/smartcard_pack.c +++ b/libfreerdp/utils/smartcard_pack.c @@ -344,7 +344,7 @@ static char* smartcard_convert_string_list(const void* in, size_t bytes, BOOL un length = (bytes / sizeof(WCHAR)) - 1; WINPR_ASSERT(length < INT_MAX); - mszA = (char*)calloc(length + 1, sizeof(WCHAR)); + mszA = (char*)calloc(length + 1, sizeof(char)); if (!mszA) return NULL; if (ConvertFromUnicode(CP_UTF8, 0, string.wz, (int)length, &mszA, (int)length + 1, NULL, @@ -432,8 +432,6 @@ static char* smartcard_array_dump(const void* pd, size_t len, char* buffer, size rc = _snprintf(buffer, bufferLen, " }"); if ((rc < 0) || ((size_t)rc > bufferLen)) goto fail; - buffer += rc; - bufferLen -= (size_t)rc; fail: return start; diff --git a/server/shadow/shadow_server.c b/server/shadow/shadow_server.c index 610d68375..7340d5c7f 100644 --- a/server/shadow/shadow_server.c +++ b/server/shadow/shadow_server.c @@ -598,6 +598,7 @@ int shadow_server_start(rdpShadowServer* server) } } + WINPR_ASSERT(list || (count == 0)); for (x = 1; x < count; x++) { BOOL success = open_port(server, list[x]); diff --git a/winpr/libwinpr/sspi/Kerberos/kerberos.c b/winpr/libwinpr/sspi/Kerberos/kerberos.c index f54c27bd5..a87b1c0bc 100644 --- a/winpr/libwinpr/sspi/Kerberos/kerberos.c +++ b/winpr/libwinpr/sspi/Kerberos/kerberos.c @@ -626,7 +626,9 @@ static SECURITY_STATUS SEC_ENTRY kerberos_InitializeSecurityContextA( sspi_gss_buffer_desc output_tok = { 0 }; #if defined(WITH_GSSAPI) sspi_gss_OID actual_mech; +#if defined(WITH_GSSAPI) sspi_gss_OID desired_mech = SSPI_GSS_C_SPNEGO_KRB5; +#endif UINT32 actual_services; #endif diff --git a/winpr/libwinpr/thread/apc.c b/winpr/libwinpr/thread/apc.c index 6b8ac5e91..f06d4c28a 100644 --- a/winpr/libwinpr/thread/apc.c +++ b/winpr/libwinpr/thread/apc.c @@ -22,6 +22,7 @@ #include "thread.h" #include "../log.h" #include "../synch/pollset.h" +#include #define TAG WINPR_TAG("apc") @@ -30,6 +31,8 @@ BOOL apc_init(APC_QUEUE* apc) pthread_mutexattr_t attr; BOOL ret = FALSE; + WINPR_ASSERT(apc); + pthread_mutexattr_init(&attr); if (pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE) != 0) { @@ -53,13 +56,20 @@ out: BOOL apc_uninit(APC_QUEUE* apc) { + WINPR_ASSERT(apc); return pthread_mutex_destroy(&apc->mutex) == 0; } void apc_register(WINPR_THREAD* thread, WINPR_APC_ITEM* addItem) { WINPR_APC_ITEM** nextp; - APC_QUEUE* apc = &thread->apc; + APC_QUEUE* apc; + + WINPR_ASSERT(thread); + WINPR_ASSERT(addItem); + + apc = &thread->apc; + WINPR_ASSERT(apc); pthread_mutex_lock(&apc->mutex); if (apc->tail) @@ -84,6 +94,9 @@ void apc_register(WINPR_THREAD* thread, WINPR_APC_ITEM* addItem) static INLINE void apc_item_remove(APC_QUEUE* apc, WINPR_APC_ITEM* item) { + WINPR_ASSERT(apc); + WINPR_ASSERT(item); + if (!item->last) apc->head = item->next; else @@ -103,6 +116,8 @@ APC_REMOVE_RESULT apc_remove(WINPR_APC_ITEM* item) APC_QUEUE* apc; APC_REMOVE_RESULT ret = APC_REMOVE_OK; + WINPR_ASSERT(item); + if (!item->linked) return APC_REMOVE_OK; @@ -119,6 +134,8 @@ APC_REMOVE_RESULT apc_remove(WINPR_APC_ITEM* item) } apc = &thread->apc; + WINPR_ASSERT(apc); + pthread_mutex_lock(&apc->mutex); if (apc->treatingCompletions) { @@ -140,7 +157,12 @@ BOOL apc_collectFds(WINPR_THREAD* thread, WINPR_POLL_SET* set, BOOL* haveAutoSig { WINPR_APC_ITEM* item; BOOL ret = FALSE; - APC_QUEUE* apc = &thread->apc; + APC_QUEUE* apc; + + WINPR_ASSERT(thread); + + apc = &thread->apc; + WINPR_ASSERT(apc); *haveAutoSignaled = FALSE; pthread_mutex_lock(&apc->mutex); @@ -148,7 +170,10 @@ BOOL apc_collectFds(WINPR_THREAD* thread, WINPR_POLL_SET* set, BOOL* haveAutoSig for (; item; item = item->next) { if (item->alwaysSignaled) + { + WINPR_ASSERT(haveAutoSignaled); *haveAutoSignaled = TRUE; + } else if (!pollset_add(set, item->pollFd, item->pollMode)) goto out; } @@ -161,10 +186,15 @@ out: int apc_executeCompletions(WINPR_THREAD* thread, WINPR_POLL_SET* set, size_t idx) { - APC_QUEUE* apc = &thread->apc; + APC_QUEUE* apc; WINPR_APC_ITEM *item, *nextItem; int ret = 0; + WINPR_ASSERT(thread); + + apc = &thread->apc; + WINPR_ASSERT(apc); + pthread_mutex_lock(&apc->mutex); apc->treatingCompletions = TRUE; @@ -187,14 +217,6 @@ int apc_executeCompletions(WINPR_THREAD* thread, WINPR_POLL_SET* set, size_t idx } nextItem = item->next; - - if (item->markedForRemove) - { - apc_item_remove(apc, item); - - if (item->markedForFree) - free(item); - } } /* third pass: to do final cleanup */ @@ -220,7 +242,12 @@ void apc_cleanupThread(WINPR_THREAD* thread) { WINPR_APC_ITEM* item; WINPR_APC_ITEM* nextItem; - APC_QUEUE* apc = &thread->apc; + APC_QUEUE* apc; + + WINPR_ASSERT(thread); + + apc = &thread->apc; + WINPR_ASSERT(apc); pthread_mutex_lock(&apc->mutex); item = apc->head; diff --git a/winpr/libwinpr/thread/argv.c b/winpr/libwinpr/thread/argv.c index 8d1cdf467..c57debb14 100644 --- a/winpr/libwinpr/thread/argv.c +++ b/winpr/libwinpr/thread/argv.c @@ -192,7 +192,7 @@ LPSTR* CommandLineToArgvA(LPCSTR lpCmdLine, int* pNumArgs) } maxBufferSize = (maxNumArgs * (sizeof(char*))) + (cmdLineLength + 1); - buffer = calloc(maxBufferSize, sizeof(WCHAR)); + buffer = calloc(maxBufferSize, sizeof(char)); if (!buffer) { diff --git a/winpr/libwinpr/utils/collections/BufferPool.c b/winpr/libwinpr/utils/collections/BufferPool.c index cda69e2a8..7f1e63707 100644 --- a/winpr/libwinpr/utils/collections/BufferPool.c +++ b/winpr/libwinpr/utils/collections/BufferPool.c @@ -483,7 +483,7 @@ wBufferPool* BufferPool_New(BOOL synchronized, SSIZE_T fixedSize, DWORD alignmen { wBufferPool* pool = NULL; - pool = (wBufferPool*)malloc(sizeof(wBufferPool)); + pool = (wBufferPool*)calloc(1, sizeof(wBufferPool)); if (pool) { @@ -522,10 +522,7 @@ wBufferPool* BufferPool_New(BOOL synchronized, SSIZE_T fixedSize, DWORD alignmen pool->uCapacity = 32; pool->uArray = (wBufferPoolItem*)calloc(pool->uCapacity, sizeof(wBufferPoolItem)); if (!pool->uArray) - { - free(pool->aArray); goto out_error; - } } }