From 81d2c1f05743bec3b49abfea299a7179f282f1fe Mon Sep 17 00:00:00 2001 From: akallabeth Date: Tue, 23 Jan 2024 21:03:28 +0100 Subject: [PATCH] [clang-tidy] clang-analyzer-core.NullDereference --- channels/audin/server/audin.c | 2 +- channels/rdpdr/client/rdpdr_main.c | 7 +- channels/rdpgfx/server/rdpgfx_main.c | 2 +- libfreerdp/cache/bitmap.c | 69 ++++++++++--------- libfreerdp/codec/progressive.c | 7 +- libfreerdp/core/rdp.c | 3 +- libfreerdp/core/test/TestSettings.c | 2 + libfreerdp/gdi/gdi.c | 3 +- libfreerdp/utils/smartcard_call.c | 1 + server/proxy/channels/pf_channel_rdpdr.c | 4 +- server/shadow/shadow_server.c | 3 +- winpr/libwinpr/file/generic.c | 8 ++- winpr/libwinpr/sspi/Kerberos/kerberos.c | 6 +- .../libwinpr/sspi/Kerberos/krb5glue_heimdal.c | 2 +- winpr/libwinpr/sspi/NTLM/ntlm.c | 2 +- winpr/libwinpr/sspi/Negotiate/negotiate.c | 22 +++--- winpr/libwinpr/utils/cmdline.c | 3 +- winpr/libwinpr/utils/image.c | 22 ++++-- 18 files changed, 105 insertions(+), 63 deletions(-) diff --git a/channels/audin/server/audin.c b/channels/audin/server/audin.c index ef60919b7..d67937ab7 100644 --- a/channels/audin/server/audin.c +++ b/channels/audin/server/audin.c @@ -826,7 +826,7 @@ audin_server_context* audin_server_context_new(HANDLE vcm) if (!audin) { - WLog_Print(audin->log, WLOG_ERROR, "calloc failed!"); + WLog_ERR(AUDIN_TAG, "calloc failed!"); return NULL; } audin->log = WLog_Get(AUDIN_TAG); diff --git a/channels/rdpdr/client/rdpdr_main.c b/channels/rdpdr/client/rdpdr_main.c index 857e236a6..bed7a2328 100644 --- a/channels/rdpdr/client/rdpdr_main.c +++ b/channels/rdpdr/client/rdpdr_main.c @@ -68,6 +68,8 @@ #include "rdpdr_main.h" +#define TAG CHANNELS_TAG("rdpdr.client") + /* IMPORTANT: Keep in sync with DRIVE_DEVICE */ typedef struct { @@ -2231,7 +2233,7 @@ static VOID VCAPITYPE rdpdr_virtual_channel_init_event_ex(LPVOID lpUserParam, LP if (!rdpdr || (rdpdr->InitHandle != pInitHandle)) { - WLog_Print(rdpdr->log, WLOG_ERROR, "error no match"); + WLog_ERR(TAG, "error no match"); return; } @@ -2277,7 +2279,6 @@ static VOID VCAPITYPE rdpdr_virtual_channel_init_event_ex(LPVOID lpUserParam, LP } /* rdpdr is always built-in */ -#define TAG CHANNELS_TAG("rdpdr.client") #define VirtualChannelEntryEx rdpdr_VirtualChannelEntryEx FREERDP_ENTRY_POINT(BOOL VCAPITYPE VirtualChannelEntryEx(PCHANNEL_ENTRY_POINTS pEntryPoints, @@ -2294,7 +2295,7 @@ FREERDP_ENTRY_POINT(BOOL VCAPITYPE VirtualChannelEntryEx(PCHANNEL_ENTRY_POINTS p if (!rdpdr) { - WLog_Print(rdpdr->log, WLOG_ERROR, "calloc failed!"); + WLog_ERR(TAG, "calloc failed!"); return FALSE; } rdpdr->log = WLog_Get(TAG); diff --git a/channels/rdpgfx/server/rdpgfx_main.c b/channels/rdpgfx/server/rdpgfx_main.c index 94e647a35..b8e93a7be 100644 --- a/channels/rdpgfx/server/rdpgfx_main.c +++ b/channels/rdpgfx/server/rdpgfx_main.c @@ -1689,7 +1689,7 @@ RdpgfxServerContext* rdpgfx_server_context_new(HANDLE vcm) if (!context) { - WLog_Print(context->priv->log, WLOG_ERROR, "calloc failed!"); + WLog_ERR(TAG, "calloc failed!"); return NULL; } diff --git a/libfreerdp/cache/bitmap.c b/libfreerdp/cache/bitmap.c index ee18501bc..25070b8fc 100644 --- a/libfreerdp/cache/bitmap.c +++ b/libfreerdp/cache/bitmap.c @@ -310,27 +310,31 @@ static int bitmap_cache_save_persistent(rdpBitmapCache* bitmapCache) if (status < 1) goto end; - for (UINT32 i = 0; i < bitmapCache->maxCells; i++) + if (bitmapCache->cells) { - for (UINT32 j = 0; j < bitmapCache->cells[i].number + 1; j++) + for (UINT32 i = 0; i < bitmapCache->maxCells; i++) { - PERSISTENT_CACHE_ENTRY cacheEntry; - rdpBitmap* bitmap = bitmapCache->cells[i].entries[j]; - - if (!bitmap || !bitmap->key64) - continue; - - cacheEntry.key64 = bitmap->key64; - cacheEntry.width = bitmap->width; - cacheEntry.height = bitmap->height; - cacheEntry.size = (UINT32)(bitmap->width * bitmap->height * 4); - cacheEntry.flags = 0; - cacheEntry.data = bitmap->data; - - if (persistent_cache_write_entry(persistent, &cacheEntry) < 1) + BITMAP_V2_CELL* cell = &bitmapCache->cells[i]; + for (UINT32 j = 0; j < cell->number + 1 && cell->entries; j++) { - status = -1; - goto end; + PERSISTENT_CACHE_ENTRY cacheEntry; + rdpBitmap* bitmap = cell->entries[j]; + + if (!bitmap || !bitmap->key64) + continue; + + cacheEntry.key64 = bitmap->key64; + cacheEntry.width = bitmap->width; + cacheEntry.height = bitmap->height; + cacheEntry.size = (UINT32)(bitmap->width * bitmap->height * 4); + cacheEntry.flags = 0; + cacheEntry.data = bitmap->data; + + if (persistent_cache_write_entry(persistent, &cacheEntry) < 1) + { + status = -1; + goto end; + } } } } @@ -397,25 +401,28 @@ void bitmap_cache_free(rdpBitmapCache* bitmapCache) bitmap_cache_save_persistent(bitmapCache); - UINT32 i = 0; - for (i = 0; i < bitmapCache->maxCells; i++) + if (bitmapCache->cells) { - UINT32 j = 0; - BITMAP_V2_CELL* cell = &bitmapCache->cells[i]; - - if (!cell->entries) - continue; - - for (j = 0; j < cell->number + 1; j++) + for (UINT32 i = 0; i < bitmapCache->maxCells; i++) { - rdpBitmap* bitmap = cell->entries[j]; - Bitmap_Free(bitmapCache->context, bitmap); + UINT32 j = 0; + BITMAP_V2_CELL* cell = &bitmapCache->cells[i]; + + if (!cell->entries) + continue; + + for (j = 0; j < cell->number + 1; j++) + { + rdpBitmap* bitmap = cell->entries[j]; + Bitmap_Free(bitmapCache->context, bitmap); + } + + free(cell->entries); } - free(bitmapCache->cells[i].entries); + free(bitmapCache->cells); } - free(bitmapCache->cells); persistent_cache_free(bitmapCache->persistent); free(bitmapCache); diff --git a/libfreerdp/codec/progressive.c b/libfreerdp/codec/progressive.c index 22e475ff9..1d755df50 100644 --- a/libfreerdp/codec/progressive.c +++ b/libfreerdp/codec/progressive.c @@ -1767,9 +1767,12 @@ static INLINE SSIZE_T progressive_process_tiles(PROGRESSIVE_CONTEXT* progressive return -1044; } - if (progressive->rfx_context->priv->UseThreads) { - work_objects = (PTP_WORK*)winpr_aligned_calloc(region->numTiles, sizeof(PTP_WORK), 32); + size_t tcount = 1; + if (progressive->rfx_context->priv->UseThreads) + tcount = region->numTiles; + + work_objects = (PTP_WORK*)winpr_aligned_calloc(tcount, sizeof(PTP_WORK), 32); if (!work_objects) return -1; } diff --git a/libfreerdp/core/rdp.c b/libfreerdp/core/rdp.c index f4a50453e..16b48a280 100644 --- a/libfreerdp/core/rdp.c +++ b/libfreerdp/core/rdp.c @@ -260,7 +260,8 @@ BOOL rdp_read_share_control_header(rdpRdp* rdp, wStream* s, UINT16* tpktLength, char buffer[128] = { 0 }; WLog_Print(rdp->log, WLOG_DEBUG, "[Flow control PDU] type=%s, tpktLength=%" PRIuz ", remainingLength=%" PRIuz, - pdu_type_to_str(*type, buffer, sizeof(buffer)), *tpktLength, *remainingLength); + pdu_type_to_str(*type, buffer, sizeof(buffer)), tpktLength ? *tpktLength : 0, + *remainingLength); return TRUE; } diff --git a/libfreerdp/core/test/TestSettings.c b/libfreerdp/core/test/TestSettings.c index 1d68f4711..5896ac810 100644 --- a/libfreerdp/core/test/TestSettings.c +++ b/libfreerdp/core/test/TestSettings.c @@ -17,6 +17,8 @@ static BOOL compare(const ADDIN_ARGV* got, const ADDIN_ARGV* expect) { int x = 0; BOOL rc = TRUE; + if (!got && !expect) + return FALSE; if (!got && expect) return FALSE; if (got && !expect) diff --git a/libfreerdp/gdi/gdi.c b/libfreerdp/gdi/gdi.c index a6478a28f..5290c44d6 100644 --- a/libfreerdp/gdi/gdi.c +++ b/libfreerdp/gdi/gdi.c @@ -476,7 +476,8 @@ BOOL gdi_bitmap_update(rdpContext* context, const BITMAP_UPDATE* bitmapUpdate) WLog_ERR(TAG, "Invalid arguments: context=%p, bitmapUpdate=%p, context->gdi=%p, " "context->codecs=%p", - context, bitmapUpdate, context->gdi, context->codecs); + context, bitmapUpdate, context ? context->gdi : NULL, + context ? context->codecs : NULL); return FALSE; } diff --git a/libfreerdp/utils/smartcard_call.c b/libfreerdp/utils/smartcard_call.c index ded5885bc..b796d1681 100644 --- a/libfreerdp/utils/smartcard_call.c +++ b/libfreerdp/utils/smartcard_call.c @@ -1923,6 +1923,7 @@ void smartcard_call_context_free(scard_call_context* ctx) LinkedList_Free(ctx->names); if (ctx->StartedEvent) { + WINPR_ASSERT(ctx->useEmulatedCard || ctx->pWinSCardApi); wrap(ctx, SCardReleaseStartedEvent); } diff --git a/server/proxy/channels/pf_channel_rdpdr.c b/server/proxy/channels/pf_channel_rdpdr.c index 62c2acd00..424b4c974 100644 --- a/server/proxy/channels/pf_channel_rdpdr.c +++ b/server/proxy/channels/pf_channel_rdpdr.c @@ -1333,7 +1333,7 @@ BOOL pf_channel_rdpdr_client_handle(pClientContext* pc, UINT16 channelId, const rdpdr = HashTable_GetItemValue(pc->interceptContextMap, channel_name); if (!rdpdr) { - CLIENT_RX_LOG(rdpdr->log, WLOG_ERROR, + CLIENT_RX_LOG(WLog_Get(RTAG), WLOG_ERROR, "Channel %s [0x%04" PRIx16 "] missing context in interceptContextMap", channel_name, channelId); return FALSE; @@ -1837,7 +1837,7 @@ static pf_channel_server_context* get_channel(pServerContext* ps, BOOL send) rdpdr = HashTable_GetItemValue(ps->interceptContextMap, RDPDR_SVC_CHANNEL_NAME); if (!rdpdr) { - SERVER_RXTX_LOG(send, rdpdr->log, WLOG_ERROR, + SERVER_RXTX_LOG(send, WLog_Get(RTAG), WLOG_ERROR, "Channel %s missing context in interceptContextMap", RDPDR_SVC_CHANNEL_NAME); return NULL; diff --git a/server/shadow/shadow_server.c b/server/shadow/shadow_server.c index 653c77995..474554046 100644 --- a/server/shadow/shadow_server.c +++ b/server/shadow/shadow_server.c @@ -707,7 +707,8 @@ int shadow_server_stop(rdpShadowServer* server) WaitForSingleObject(server->thread, INFINITE); CloseHandle(server->thread); server->thread = NULL; - server->listener->Close(server->listener); + if (server->listener && server->listener->Close) + server->listener->Close(server->listener); } if (server->screen) diff --git a/winpr/libwinpr/file/generic.c b/winpr/libwinpr/file/generic.c index d1ee0a3c0..2d0eb38bb 100644 --- a/winpr/libwinpr/file/generic.c +++ b/winpr/libwinpr/file/generic.c @@ -1197,10 +1197,12 @@ BOOL FindNextFileW(HANDLE hFindFile, LPWIN32_FIND_DATAW lpFindFileData) BOOL FindClose(HANDLE hFindFile) { WIN32_FILE_SEARCH* pFileSearch = (WIN32_FILE_SEARCH*)hFindFile; + if (!pFileSearch) + return FALSE; - /* Since INVALID_HANDLE_VALUE != NULL the analyzer guesses that there - * is a initialized HANDLE that is not freed properly. - * Disable this return to stop confusing the analyzer. */ + /* Since INVALID_HANDLE_VALUE != NULL the analyzer guesses that there + * is a initialized HANDLE that is not freed properly. + * Disable this return to stop confusing the analyzer. */ #ifndef __clang_analyzer__ if (!is_valid_file_search_handle(hFindFile)) return FALSE; diff --git a/winpr/libwinpr/sspi/Kerberos/kerberos.c b/winpr/libwinpr/sspi/Kerberos/kerberos.c index d099d7c9b..04497929d 100644 --- a/winpr/libwinpr/sspi/Kerberos/kerberos.c +++ b/winpr/libwinpr/sspi/Kerberos/kerberos.c @@ -975,7 +975,8 @@ static SECURITY_STATUS SEC_ENTRY kerberos_InitializeSecurityContextA( context->state = KERBEROS_STATE_FINAL; - output_buffer->cbBuffer = 0; + if (output_buffer) + output_buffer->cbBuffer = 0; status = SEC_E_OK; break; @@ -1224,7 +1225,8 @@ static SECURITY_STATUS SEC_ENTRY kerberos_AcceptSecurityContext( } else { - output_buffer->cbBuffer = 0; + if (output_buffer) + output_buffer->cbBuffer = 0; } *pfContextAttr = context->flags & 0x1F; diff --git a/winpr/libwinpr/sspi/Kerberos/krb5glue_heimdal.c b/winpr/libwinpr/sspi/Kerberos/krb5glue_heimdal.c index 249a733a5..8e01b5550 100644 --- a/winpr/libwinpr/sspi/Kerberos/krb5glue_heimdal.c +++ b/winpr/libwinpr/sspi/Kerberos/krb5glue_heimdal.c @@ -189,7 +189,7 @@ krb5_error_code krb5glue_get_init_creds(krb5_context ctx, krb5_principal princ, break; if ((rv = krb5_init_creds_set_password(ctx, creds_ctx, password)) != 0) break; - if (krb_settings->armorCache) + if (krb_settings && krb_settings->armorCache) { krb5_ccache armor_cc = NULL; if ((rv = krb5_cc_resolve(ctx, krb_settings->armorCache, &armor_cc)) != 0) diff --git a/winpr/libwinpr/sspi/NTLM/ntlm.c b/winpr/libwinpr/sspi/NTLM/ntlm.c index ed86f5c9f..36671d534 100644 --- a/winpr/libwinpr/sspi/NTLM/ntlm.c +++ b/winpr/libwinpr/sspi/NTLM/ntlm.c @@ -56,7 +56,7 @@ static BOOL check_context_(NTLM_CONTEXT* context, const char* file, const char* WLog_PrintMessage(log, WLOG_MESSAGE_TEXT, log_level, line, file, fkt, "invalid context"); - rc = FALSE; + return FALSE; } if (!context->RecvRc4Seal) diff --git a/winpr/libwinpr/sspi/Negotiate/negotiate.c b/winpr/libwinpr/sspi/Negotiate/negotiate.c index cfbdb47e7..7249399b5 100644 --- a/winpr/libwinpr/sspi/Negotiate/negotiate.c +++ b/winpr/libwinpr/sspi/Negotiate/negotiate.c @@ -583,10 +583,12 @@ static SECURITY_STATUS negotiate_mic_exchange(NEGOTIATE_CONTEXT* context, NegTok { /* Store the mic token after the mech token in the output buffer */ output_token->mic.BufferType = SECBUFFER_TOKEN; - output_token->mic.cbBuffer = output_buffer->cbBuffer - output_token->mechToken.cbBuffer; - output_token->mic.pvBuffer = - (BYTE*)output_buffer->pvBuffer + output_token->mechToken.cbBuffer; - + if (output_buffer) + { + output_token->mic.cbBuffer = output_buffer->cbBuffer - output_token->mechToken.cbBuffer; + output_token->mic.pvBuffer = + (BYTE*)output_buffer->pvBuffer + output_token->mechToken.cbBuffer; + } mic_buffers[1] = output_token->mic; status = table->MakeSignature(&context->sub_context, 0, &mic_buffer_desc, 0); @@ -849,7 +851,8 @@ static SECURITY_STATUS SEC_ENTRY negotiate_InitializeSecurityContextW( if (context->mic || input_token.negState != ACCEPT_COMPLETED) return SEC_E_INVALID_TOKEN; - output_buffer->cbBuffer = 0; + if (output_buffer) + output_buffer->cbBuffer = 0; return SEC_E_OK; } @@ -864,7 +867,8 @@ static SECURITY_STATUS SEC_ENTRY negotiate_InitializeSecurityContextW( if (input_token.negState == ACCEPT_COMPLETED) { - output_buffer->cbBuffer = 0; + if (output_buffer) + output_buffer->cbBuffer = 0; return SEC_E_OK; } @@ -1016,7 +1020,8 @@ static SECURITY_STATUS SEC_ENTRY negotiate_AcceptSecurityContext( if (init_context.mech) { - output_token.mechToken = *output_buffer; + if (output_buffer) + output_token.mechToken = *output_buffer; WLog_DBG(TAG, "Requested mechanism: %s", negotiate_mech_name(init_context.mech->oid)); } @@ -1156,7 +1161,8 @@ static SECURITY_STATUS SEC_ENTRY negotiate_AcceptSecurityContext( if (input_token.negState == ACCEPT_COMPLETED) { - output_buffer->cbBuffer = 0; + if (output_buffer) + output_buffer->cbBuffer = 0; return SEC_E_OK; } diff --git a/winpr/libwinpr/utils/cmdline.c b/winpr/libwinpr/utils/cmdline.c index cceea5f14..52d748107 100644 --- a/winpr/libwinpr/utils/cmdline.c +++ b/winpr/libwinpr/utils/cmdline.c @@ -796,7 +796,8 @@ fail: free(copy); if (!success) { - *count = 0; + if (count) + *count = 0; free(p); return NULL; } diff --git a/winpr/libwinpr/utils/image.c b/winpr/libwinpr/utils/image.c index c728c911a..0769960b6 100644 --- a/winpr/libwinpr/utils/image.c +++ b/winpr/libwinpr/utils/image.c @@ -173,6 +173,8 @@ fail: static void* winpr_bitmap_write_buffer(const BYTE* data, size_t size, UINT32 width, UINT32 height, UINT32 stride, UINT32 bpp, UINT32* pSize) { + WINPR_ASSERT(data || (size == 0)); + void* result = NULL; const size_t bpp_stride = 1ull * width * (bpp / 8); wStream* s = Stream_New(NULL, 1024); @@ -265,11 +267,14 @@ fail: int winpr_image_write(wImage* image, const char* filename) { + WINPR_ASSERT(image); return winpr_image_write_ex(image, image->type, filename); } int winpr_image_write_ex(wImage* image, UINT32 format, const char* filename) { + WINPR_ASSERT(image); + size_t size = 0; void* data = winpr_image_write_buffer(image, format, &size); if (!data) @@ -688,11 +693,21 @@ static SSIZE_T save_png_to_buffer(UINT32 bpp, UINT32 width, UINT32 height, const int rc = -1; png_structp png_ptr = NULL; png_infop info_ptr = NULL; - png_uint_32 bytes_per_row = 0; png_byte** row_pointers = NULL; struct png_mem_encode state = { 0 }; *pDstData = NULL; + + if (!data || (size == 0)) + return 0; + + WINPR_ASSERT(pDstData); + + const size_t bytes_per_pixel = (bpp + 7) / 8; + const size_t bytes_per_row = width * bytes_per_pixel; + if (size < bytes_per_row * height) + goto fail; + /* Initialize the write struct. */ png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL); if (png_ptr == NULL) @@ -720,8 +735,6 @@ static SSIZE_T save_png_to_buffer(UINT32 bpp, UINT32 width, UINT32 height, const PNG_COMPRESSION_TYPE_DEFAULT, PNG_FILTER_TYPE_DEFAULT); /* Initialize rows of PNG. */ - const size_t bytes_per_pixel = (bpp + 7) / 8; - bytes_per_row = width * bytes_per_pixel; row_pointers = png_malloc(png_ptr, height * sizeof(png_byte*)); for (size_t y = 0; y < height; ++y) { @@ -853,7 +866,7 @@ void* winpr_convert_to_png(const void* data, size_t size, UINT32 width, UINT32 h *pSize = 0; #if defined(WINPR_UTILS_IMAGE_PNG) - char* dst = NULL; + void* dst = NULL; SSIZE_T rc = save_png_to_buffer(bpp, width, height, data, size, &dst); if (rc <= 0) return NULL; @@ -1088,6 +1101,7 @@ const char* winpr_image_format_extension(UINT32 format) void* winpr_image_write_buffer(wImage* image, UINT32 format, size_t* psize) { + WINPR_ASSERT(image); switch (format) { case WINPR_IMAGE_BITMAP: