diff --git a/.clang-tidy b/.clang-tidy index d639d1b19..75f35c30a 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -3,7 +3,6 @@ Checks: > -*, abseil-*, altera-*, - boost-*, bugprone-*, cert-*, clang-analyzer*, diff --git a/channels/tsmf/client/ffmpeg/tsmf_ffmpeg.c b/channels/tsmf/client/ffmpeg/tsmf_ffmpeg.c index c817580b8..4136ab7ba 100644 --- a/channels/tsmf/client/ffmpeg/tsmf_ffmpeg.c +++ b/channels/tsmf/client/ffmpeg/tsmf_ffmpeg.c @@ -442,7 +442,10 @@ static BOOL tsmf_ffmpeg_decode_video(ITSMFDecoder* decoder, const BYTE* data, UI mdecoder->codec_context->pix_fmt, mdecoder->codec_context->width, mdecoder->codec_context->height, 1); - const uint8_t* const* ptr = (const uint8_t* const*)mdecoder->frame->data; + const uint8_t* ptr[AV_NUM_DATA_POINTERS] = { 0 }; + for (size_t x = 0; x < AV_NUM_DATA_POINTERS; x++) + ptr[x] = mdecoder->frame->data[x]; + av_image_copy(frame->data, frame->linesize, ptr, mdecoder->frame->linesize, mdecoder->codec_context->pix_fmt, mdecoder->codec_context->width, mdecoder->codec_context->height); diff --git a/channels/urbdrc/client/libusb/libusb_udevice.c b/channels/urbdrc/client/libusb/libusb_udevice.c index f02aac2a7..e843d7142 100644 --- a/channels/urbdrc/client/libusb/libusb_udevice.c +++ b/channels/urbdrc/client/libusb/libusb_udevice.c @@ -799,7 +799,8 @@ static int libusb_udev_control_pipe_request(IUDEVICE* idev, UINT32 RequestId, uint8_t request_type, uint8_t bRequest, */ error = libusb_control_transfer(pdev->libusb_handle, - LIBUSB_ENDPOINT_OUT | LIBUSB_RECIPIENT_ENDPOINT, + (uint8_t)LIBUSB_ENDPOINT_OUT | + (uint8_t)LIBUSB_RECIPIENT_ENDPOINT, LIBUSB_REQUEST_SET_FEATURE, ENDPOINT_HALT, (uint16_t)EndpointAddress, NULL, 0, 1000); break; @@ -950,7 +951,8 @@ static int libusb_udev_os_feature_descriptor_request(IUDEVICE* idev, UINT32 Requ const BYTE bMS_Vendorcode = ms_string_desc[16]; /** get os descriptor */ error = libusb_control_transfer( - pdev->libusb_handle, LIBUSB_ENDPOINT_IN | LIBUSB_REQUEST_TYPE_VENDOR | Recipient, + pdev->libusb_handle, + (uint8_t)LIBUSB_ENDPOINT_IN | (uint8_t)LIBUSB_REQUEST_TYPE_VENDOR | Recipient, bMS_Vendorcode, (UINT16)((InterfaceNumber << 8) | Ms_PageIndex), Ms_featureDescIndex, Buffer, (UINT16)*BufferSize, Timeout); log_libusb_result(pdev->urbdrc->log, WLOG_DEBUG, "libusb_control_transfer", error); @@ -1190,7 +1192,8 @@ static int libusb_udev_query_device_port_status(IUDEVICE* idev, UINT32* UsbdStat { ret = idev->control_transfer( idev, 0xffff, 0, 0, - LIBUSB_ENDPOINT_IN | LIBUSB_REQUEST_TYPE_CLASS | LIBUSB_RECIPIENT_OTHER, + (uint8_t)LIBUSB_ENDPOINT_IN | (uint8_t)LIBUSB_REQUEST_TYPE_CLASS | + (uint8_t)LIBUSB_RECIPIENT_OTHER, LIBUSB_REQUEST_GET_STATUS, 0, pdev->port_number, UsbdStatus, BufferSize, Buffer, 1000); if (log_libusb_result(urbdrc->log, WLOG_DEBUG, "libusb_control_transfer", ret)) @@ -1466,7 +1469,7 @@ BASIC_STATE_FUNC_DEFINED(channelID, UINT32) BASIC_STATE_FUNC_DEFINED(ReqCompletion, UINT32) BASIC_STATE_FUNC_DEFINED(bus_number, BYTE) BASIC_STATE_FUNC_DEFINED(dev_number, BYTE) -BASIC_STATE_FUNC_DEFINED(port_number, int) +BASIC_STATE_FUNC_DEFINED(port_number, UINT8) BASIC_STATE_FUNC_DEFINED(MsConfig, MSUSB_CONFIG_DESCRIPTOR*) BASIC_POINT_FUNC_DEFINED(udev, void*) @@ -1613,7 +1616,7 @@ static int udev_get_device_handle(URBDRC_PLUGIN* urbdrc, libusb_context* ctx, UD pdev->port_number = port_numbers[(error - 1)]; error = 0; - WLog_Print(urbdrc->log, WLOG_DEBUG, " Port: %d", pdev->port_number); + WLog_Print(urbdrc->log, WLOG_DEBUG, " Port: %" PRIu8, pdev->port_number); /* gen device path */ (void)_snprintf(pdev->path, sizeof(pdev->path), "%" PRIu16 "-%d", bus_number, pdev->port_number); diff --git a/channels/urbdrc/client/libusb/libusb_udevice.h b/channels/urbdrc/client/libusb/libusb_udevice.h index 79203c829..ac12abf43 100644 --- a/channels/urbdrc/client/libusb/libusb_udevice.h +++ b/channels/urbdrc/client/libusb/libusb_udevice.h @@ -51,7 +51,7 @@ typedef struct BYTE bus_number; BYTE dev_number; char path[17]; - int port_number; + UINT8 port_number; int isCompositeDevice; LIBUSB_DEVICE_HANDLE* libusb_handle; diff --git a/channels/urbdrc/client/urbdrc_main.h b/channels/urbdrc/client/urbdrc_main.h index caf40edad..4eead9f2b 100644 --- a/channels/urbdrc/client/urbdrc_main.h +++ b/channels/urbdrc/client/urbdrc_main.h @@ -25,6 +25,8 @@ #include #include +#include + #define DEVICE_HARDWARE_ID_SIZE 32 #define DEVICE_COMPATIBILITY_ID_SIZE 36 #define DEVICE_INSTANCE_STR_SIZE 37 @@ -162,7 +164,7 @@ struct S_IUDEVICE BASIC_DEV_STATE_DEFINED(ReqCompletion, UINT32); BASIC_DEV_STATE_DEFINED(bus_number, BYTE); BASIC_DEV_STATE_DEFINED(dev_number, BYTE); - BASIC_DEV_STATE_DEFINED(port_number, int); + BASIC_DEV_STATE_DEFINED(port_number, UINT8); BASIC_DEV_STATE_DEFINED(MsConfig, MSUSB_CONFIG_DESCRIPTOR*); BASIC_DEV_STATE_DEFINED(p_udev, void*); diff --git a/ci/cmake-preloads/config-oss-fuzz.cmake b/ci/cmake-preloads/config-oss-fuzz.cmake index 36c182a36..05d04e452 100644 --- a/ci/cmake-preloads/config-oss-fuzz.cmake +++ b/ci/cmake-preloads/config-oss-fuzz.cmake @@ -1,6 +1,6 @@ message("PRELOADING cache") set (CMAKE_VERBOSE_MAKEFILE ON CACHE BOOL "preload") -set (WITH_VERBOSE_WINPR_ASSERT OFF CACHE BOOL "oss fuzz") +set (WITH_VERBOSE_WINPR_ASSERT ON CACHE BOOL "oss fuzz") set (WITH_SERVER ON CACHE BOOL "oss fuzz") set (WITH_SAMPLE OFF CACHE BOOL "oss fuzz") diff --git a/client/SDL/SDL2/dialogs/sdl_widget.cpp b/client/SDL/SDL2/dialogs/sdl_widget.cpp index b80eedc69..c8a8626f5 100644 --- a/client/SDL/SDL2/dialogs/sdl_widget.cpp +++ b/client/SDL/SDL2/dialogs/sdl_widget.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -163,8 +164,7 @@ SDL_Texture* SdlWidget::render_text_wrapped(SDL_Renderer* renderer, const std::s dst.x += hpadding; dst.w -= 2 * hpadding; auto dh = scale(src.w, src.h); - if (dh < dst.h) - dst.h = dh; + dst.h = std::min(dh, dst.h); return texture; } diff --git a/client/SDL/SDL3/dialogs/sdl_widget.cpp b/client/SDL/SDL3/dialogs/sdl_widget.cpp index 14e9c8932..af5980171 100644 --- a/client/SDL/SDL3/dialogs/sdl_widget.cpp +++ b/client/SDL/SDL3/dialogs/sdl_widget.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -183,8 +184,7 @@ SDL_Texture* SdlWidget::render_text_wrapped(SDL_Renderer* renderer, const std::s dst.x += hpadding; dst.w -= 2 * hpadding; auto dh = scale(src.w, src.h); - if (dh < dst.h) - dst.h = dh; + dst.h = std::min(dh, dst.h); return texture; } diff --git a/client/SDL/SDL3/sdl_clip.cpp b/client/SDL/SDL3/sdl_clip.cpp index a972c5274..5784382d0 100644 --- a/client/SDL/SDL3/sdl_clip.cpp +++ b/client/SDL/SDL3/sdl_clip.cpp @@ -34,9 +34,16 @@ #define mime_text_plain "text/plain" #define mime_text_utf8 mime_text_plain ";charset=utf-8" -static const std::vector s_mime_text = { mime_text_plain, mime_text_utf8, - "UTF8_STRING", "COMPOUND_TEXT", - "TEXT", "STRING" }; +static const std::vector& s_mime_text() +{ + static std::vector values; + if (values.empty()) + { + values = std::vector( + { mime_text_plain, mime_text_utf8, "UTF8_STRING", "COMPOUND_TEXT", "TEXT", "STRING" }); + } + return values; +} static const char s_mime_png[] = "image/png"; static const char s_mime_webp[] = "image/webp"; @@ -46,9 +53,27 @@ static const char s_mime_uri_list[] = "text/uri-list"; static const char s_mime_html[] = "text/html"; #define BMP_MIME_LIST "image/bmp", "image/x-bmp", "image/x-MS-bmp", "image/x-win-bitmap" -static const std::vector s_mime_bitmap = { BMP_MIME_LIST }; -static const std::vector s_mime_image = { s_mime_png, s_mime_webp, s_mime_jpg, - s_mime_tiff, BMP_MIME_LIST }; + +static const std::vector& s_mime_bitmap() +{ + static std::vector values; + if (values.empty()) + { + values = std::vector({ BMP_MIME_LIST }); + } + return values; +} + +static const std::vector& s_mime_image() +{ + static std::vector values; + if (values.empty()) + { + values = std::vector( + { s_mime_png, s_mime_webp, s_mime_jpg, s_mime_tiff, BMP_MIME_LIST }); + } + return values; +} static const char s_mime_gnome_copied_files[] = "x-special/gnome-copied-files"; static const char s_mime_mate_copied_files[] = "x-special/mate-copied-files"; @@ -164,7 +189,8 @@ bool sdlClip::handle_update(const SDL_ClipboardEvent& ev) std::string local_mime = clipboard_mime_formats[i]; WLog_Print(_log, WLOG_TRACE, " - %s", local_mime.c_str()); - if (std::find(s_mime_text.begin(), s_mime_text.end(), local_mime) != s_mime_text.end()) + if (std::find(s_mime_text().begin(), s_mime_text().end(), local_mime) != + s_mime_text().end()) { /* text formats */ if (!textPushed) @@ -456,12 +482,12 @@ UINT sdlClip::ReceiveServerFormatList(CliprdrClientContext* context, std::vector mimetypes; if (text) { - mimetypes.insert(mimetypes.end(), s_mime_text.begin(), s_mime_text.end()); + mimetypes.insert(mimetypes.end(), s_mime_text().begin(), s_mime_text().end()); } if (image) { - mimetypes.insert(mimetypes.end(), s_mime_bitmap.begin(), s_mime_bitmap.end()); - mimetypes.insert(mimetypes.end(), s_mime_image.begin(), s_mime_image.end()); + mimetypes.insert(mimetypes.end(), s_mime_bitmap().begin(), s_mime_bitmap().end()); + mimetypes.insert(mimetypes.end(), s_mime_image().begin(), s_mime_image().end()); } if (html) { @@ -524,7 +550,7 @@ std::shared_ptr sdlClip::ReceiveFormatDataRequestHandle( case CF_DIB: case CF_DIBV5: - mime = s_mime_bitmap[0]; + mime = s_mime_bitmap()[0]; break; case CF_TIFF: @@ -801,7 +827,7 @@ bool sdlClip::mime_is_file(const std::string& mime) bool sdlClip::mime_is_text(const std::string& mime) { - for (const auto& tmime : s_mime_text) + for (const auto& tmime : s_mime_text()) { assert(tmime != nullptr); if (mime == tmime) @@ -813,7 +839,7 @@ bool sdlClip::mime_is_text(const std::string& mime) bool sdlClip::mime_is_image(const std::string& mime) { - for (const auto& imime : s_mime_image) + for (const auto& imime : s_mime_image()) { assert(imime != nullptr); if (mime == imime) diff --git a/client/common/cmdline.c b/client/common/cmdline.c index f15c6d383..206d3730f 100644 --- a/client/common/cmdline.c +++ b/client/common/cmdline.c @@ -5810,7 +5810,7 @@ BOOL freerdp_client_load_addins(rdpChannels* channels, rdpSettings* settings) if ((dynChannels[i].settingId == FreeRDP_BOOL_UNUSED) || freerdp_settings_get_bool(settings, dynChannels[i].settingId)) { - const char* p[] = { dynChannels[i].channelName }; + const char* const p[] = { dynChannels[i].channelName }; if (!freerdp_client_add_dynamic_channel(settings, ARRAYSIZE(p), p)) return FALSE; @@ -5922,7 +5922,7 @@ BOOL freerdp_client_load_addins(rdpChannels* channels, rdpSettings* settings) { if (!freerdp_device_collection_find(settings, "drive")) { - const char* params[] = { "drive", "media", "*" }; + const char* const params[] = { "drive", "media", "*" }; if (!freerdp_client_add_device_channel(settings, ARRAYSIZE(params), params)) return FALSE; @@ -5959,7 +5959,7 @@ BOOL freerdp_client_load_addins(rdpChannels* channels, rdpSettings* settings) if (!freerdp_static_channel_collection_find(settings, RDPSND_CHANNEL_NAME) && !freerdp_dynamic_channel_collection_find(settings, RDPSND_CHANNEL_NAME)) { - const char* params[] = { RDPSND_CHANNEL_NAME, "sys:fake" }; + const char* const params[] = { RDPSND_CHANNEL_NAME, "sys:fake" }; if (!freerdp_client_add_static_channel(settings, ARRAYSIZE(params), params)) return FALSE; @@ -6037,7 +6037,7 @@ BOOL freerdp_client_load_addins(rdpChannels* channels, rdpSettings* settings) } else { - const char* p[] = { staticChannels[i].channelName }; + const char* const p[] = { staticChannels[i].channelName }; if (!freerdp_client_add_static_channel(settings, ARRAYSIZE(p), p)) return FALSE; } diff --git a/client/common/file.c b/client/common/file.c index 6d83126b6..35e8643f2 100644 --- a/client/common/file.c +++ b/client/common/file.c @@ -2365,7 +2365,7 @@ BOOL freerdp_client_populate_settings_from_rdp_file(const rdpFile* file, rdpSett union { char** c; - const char** cc; + const char* const* cc; } cnv; ADDIN_ARGV* args = rdp_file_to_args(RDPECAM_DVC_CHANNEL_NAME, file->RedirectCameras); if (!args) @@ -2409,7 +2409,7 @@ BOOL freerdp_client_populate_settings_from_rdp_file(const rdpFile* file, rdpSett union { char** c; - const char** cc; + const char* const* cc; } cnv; ADDIN_ARGV* args = rdp_file_to_args(URBDRC_CHANNEL_NAME, file->UsbDevicesToRedirect); if (!args) diff --git a/libfreerdp/codec/bulk.c b/libfreerdp/codec/bulk.c index 7a2005b12..c804f00bc 100644 --- a/libfreerdp/codec/bulk.c +++ b/libfreerdp/codec/bulk.c @@ -37,7 +37,7 @@ struct rdp_bulk { ALIGN64 rdpContext* context; ALIGN64 UINT32 CompressionLevel; - ALIGN64 UINT32 CompressionMaxSize; + ALIGN64 UINT16 CompressionMaxSize; ALIGN64 MPPC_CONTEXT* mppcSend; ALIGN64 MPPC_CONTEXT* mppcRecv; ALIGN64 NCRUSH_CONTEXT* ncrushRecv; @@ -83,14 +83,15 @@ static UINT32 bulk_compression_level(rdpBulk* WINPR_RESTRICT bulk) bulk->CompressionLevel = (settings->CompressionLevel >= PACKET_COMPR_TYPE_RDP61) ? PACKET_COMPR_TYPE_RDP61 : settings->CompressionLevel; + WINPR_ASSERT(bulk->CompressionLevel <= UINT16_MAX); return bulk->CompressionLevel; } -UINT32 bulk_compression_max_size(rdpBulk* WINPR_RESTRICT bulk) +UINT16 bulk_compression_max_size(rdpBulk* WINPR_RESTRICT bulk) { WINPR_ASSERT(bulk); - bulk_compression_level(bulk); - bulk->CompressionMaxSize = (bulk->CompressionLevel < PACKET_COMPR_TYPE_64K) ? 8192 : 65536; + (void)bulk_compression_level(bulk); + bulk->CompressionMaxSize = (bulk->CompressionLevel < PACKET_COMPR_TYPE_64K) ? 8192 : UINT16_MAX; return bulk->CompressionMaxSize; } @@ -161,7 +162,7 @@ int bulk_decompress(rdpBulk* WINPR_RESTRICT bulk, const BYTE* WINPR_RESTRICT pSr rdpMetrics* metrics = bulk->context->metrics; WINPR_ASSERT(metrics); - bulk_compression_max_size(bulk); + (void)bulk_compression_max_size(bulk); const UINT32 type = flags & BULK_COMPRESSION_TYPE_MASK; if (flags & BULK_COMPRESSION_FLAGS_MASK) @@ -259,8 +260,8 @@ int bulk_compress(rdpBulk* WINPR_RESTRICT bulk, const BYTE* WINPR_RESTRICT pSrcD } *pDstSize = sizeof(bulk->OutputBuffer); - bulk_compression_level(bulk); - bulk_compression_max_size(bulk); + (void)bulk_compression_level(bulk); + (void)bulk_compression_max_size(bulk); switch (bulk->CompressionLevel) { diff --git a/libfreerdp/codec/bulk.h b/libfreerdp/codec/bulk.h index c1dfeb302..fece0d669 100644 --- a/libfreerdp/codec/bulk.h +++ b/libfreerdp/codec/bulk.h @@ -28,7 +28,7 @@ typedef struct rdp_bulk rdpBulk; #define BULK_COMPRESSION_FLAGS_MASK 0xE0 #define BULK_COMPRESSION_TYPE_MASK 0x0F -FREERDP_LOCAL UINT32 bulk_compression_max_size(rdpBulk* WINPR_RESTRICT bulk); +FREERDP_LOCAL UINT16 bulk_compression_max_size(rdpBulk* WINPR_RESTRICT bulk); FREERDP_LOCAL int bulk_decompress(rdpBulk* WINPR_RESTRICT bulk, const BYTE* WINPR_RESTRICT pSrcData, UINT32 SrcSize, const BYTE** WINPR_RESTRICT ppDstData, diff --git a/libfreerdp/core/fastpath.c b/libfreerdp/core/fastpath.c index e7fdac178..dbf7d0fc6 100644 --- a/libfreerdp/core/fastpath.c +++ b/libfreerdp/core/fastpath.c @@ -679,9 +679,6 @@ static BOOL fastpath_read_input_event_header(wStream* s, BYTE* eventFlags, BYTE* static BOOL fastpath_recv_input_event_scancode(rdpFastPath* fastpath, wStream* s, BYTE eventFlags) { - rdpInput* input = NULL; - UINT16 flags = 0; - UINT16 code = 0; WINPR_ASSERT(fastpath); WINPR_ASSERT(fastpath->rdp); WINPR_ASSERT(fastpath->rdp->input); @@ -690,11 +687,11 @@ static BOOL fastpath_recv_input_event_scancode(rdpFastPath* fastpath, wStream* s if (!Stream_CheckAndLogRequiredLength(TAG, s, 1)) return FALSE; - input = fastpath->rdp->input; + rdpInput* input = fastpath->rdp->input; - Stream_Read_UINT8(s, code); /* keyCode (1 byte) */ - flags = 0; + const UINT8 code = Stream_Get_UINT8(s); /* keyCode (1 byte) */ + UINT16 flags = 0; if ((eventFlags & FASTPATH_INPUT_KBDFLAGS_RELEASE)) flags |= KBD_FLAGS_RELEASE; @@ -1004,7 +1001,9 @@ wStream* fastpath_input_pdu_init(rdpFastPath* fastpath, BYTE eventFlags, BYTE ev if (!s) return NULL; - Stream_Write_UINT8(s, eventFlags | (eventCode << 5)); /* eventHeader (1 byte) */ + WINPR_ASSERT(eventCode < 8); + WINPR_ASSERT(eventFlags < 0x20); + Stream_Write_UINT8(s, (UINT8)(eventFlags | (eventCode << 5))); /* eventHeader (1 byte) */ return s; } @@ -1071,7 +1070,7 @@ BOOL fastpath_send_multiple_input_pdu(rdpFastPath* fastpath, wStream* s, size_t goto unlock; BYTE* fpInputEvents = Stream_PointerAs(s, BYTE) + sec_bytes; - const UINT16 fpInputEvents_length = (UINT16)length - 3 - sec_bytes; + const UINT16 fpInputEvents_length = (UINT16)(length - 3 - sec_bytes); WINPR_ASSERT(rdp->settings); if (rdp->settings->EncryptionMethods == ENCRYPTION_METHOD_FIPS) @@ -1190,9 +1189,8 @@ BOOL fastpath_send_update_pdu(rdpFastPath* fastpath, BYTE updateCode, wStream* s if (settings->CompressionEnabled && !skipCompression) { - const UINT32 CompressionMaxSize = bulk_compression_max_size(rdp->bulk); - maxLength = - (maxLength < CompressionMaxSize) ? maxLength : MIN(CompressionMaxSize, UINT16_MAX); + const UINT16 CompressionMaxSize = bulk_compression_max_size(rdp->bulk); + maxLength = (maxLength < CompressionMaxSize) ? maxLength : CompressionMaxSize; maxLength -= 20; } @@ -1236,7 +1234,7 @@ BOOL fastpath_send_update_pdu(rdpFastPath* fastpath, BYTE updateCode, wStream* s fpUpdateHeader.compression = 0; fpUpdateHeader.compressionFlags = 0; fpUpdateHeader.updateCode = updateCode; - fpUpdateHeader.size = (UINT16)(totalLength > maxLength) ? maxLength : totalLength; + fpUpdateHeader.size = (UINT16)(totalLength > maxLength) ? maxLength : (UINT16)totalLength; const BYTE* pSrcData = Stream_Pointer(s); UINT32 SrcSize = DstSize = fpUpdateHeader.size; @@ -1253,7 +1251,8 @@ BOOL fastpath_send_update_pdu(rdpFastPath* fastpath, BYTE updateCode, wStream* s { if (compressionFlags) { - fpUpdateHeader.compressionFlags = compressionFlags; + WINPR_ASSERT(compressionFlags <= UINT8_MAX); + fpUpdateHeader.compressionFlags = (UINT8)compressionFlags; fpUpdateHeader.compression = FASTPATH_OUTPUT_COMPRESSION_USED; } } @@ -1265,7 +1264,9 @@ BOOL fastpath_send_update_pdu(rdpFastPath* fastpath, BYTE updateCode, wStream* s DstSize = fpUpdateHeader.size; } - fpUpdateHeader.size = DstSize; + if (DstSize > UINT16_MAX) + return FALSE; + fpUpdateHeader.size = (UINT16)DstSize; totalLength -= SrcSize; if (totalLength == 0) @@ -1297,7 +1298,11 @@ BOOL fastpath_send_update_pdu(rdpFastPath* fastpath, BYTE updateCode, wStream* s } } - fpUpdatePduHeader.length = fpUpdateHeader.size + fpHeaderSize + pad; + const size_t len = fpUpdateHeader.size + fpHeaderSize + pad; + if (len > UINT16_MAX) + return FALSE; + + fpUpdatePduHeader.length = (UINT16)len; Stream_SetPosition(fs, 0); if (!fastpath_write_update_pdu_header(fs, &fpUpdatePduHeader, rdp)) return FALSE; diff --git a/libfreerdp/core/orders.c b/libfreerdp/core/orders.c index ae037507e..6c5f98ab6 100644 --- a/libfreerdp/core/orders.c +++ b/libfreerdp/core/orders.c @@ -43,15 +43,48 @@ #define TAG FREERDP_TAG("core.orders") -static inline const char* gdi_rob3_code_string_checked(UINT32 rob) +/* Exposed type definitions in public headers have the wrong type. + * assert to the correct types internally to trigger the ci checkers on wrong data passed */ +#define get_checked_uint16(value) get_checked_uint16_int((value), __FILE__, __func__, __LINE__) +static inline UINT16 get_checked_uint16_int(UINT32 value, const char* file, const char* fkt, + size_t line) { - WINPR_ASSERT((rob) <= UINT8_MAX); + WINPR_ASSERT_AT(value <= UINT16_MAX, file, fkt, line); + return (UINT16)value; +} + +#define get_checked_uint8(value) get_checked_uint8_int((value), __FILE__, __func__, __LINE__) +static inline UINT8 get_checked_uint8_int(UINT32 value, const char* file, const char* fkt, + size_t line) +{ + WINPR_ASSERT_AT(value <= UINT8_MAX, file, fkt, line); + return (UINT8)value; +} + +#define get_checked_int16(value) get_checked_int16_int((value), __FILE__, __func__, __LINE__) +static inline INT16 get_checked_int16_int(INT32 value, const char* file, const char* fkt, + size_t line) +{ + WINPR_ASSERT_AT(value <= INT16_MAX, file, fkt, line); + WINPR_ASSERT_AT(value >= INT16_MIN, file, fkt, line); + return (INT16)value; +} + +#define gdi_rob3_code_string_checked(value) \ + gdi_rob3_code_string_checked_int((value), __FILE__, __func__, __LINE__) +static inline const char* gdi_rob3_code_string_checked_int(UINT32 rob, const char* file, + const char* fkt, size_t line) +{ + WINPR_ASSERT_AT((rob) <= UINT8_MAX, file, fkt, line); return gdi_rop3_code_string((BYTE)rob); } -static inline DWORD gdi_rop3_code_checked(UINT32 code) +#define gdi_rop3_code_checked(value) \ + gdi_rop3_code_checked_int((value), __FILE__, __func__, __LINE__) +static inline DWORD gdi_rop3_code_checked_int(UINT32 code, const char* file, const char* fkt, + size_t line) { - WINPR_ASSERT(code <= UINT8_MAX); + WINPR_ASSERT_AT(code <= UINT8_MAX, file, fkt, line); return gdi_rop3_code((UINT8)code); } @@ -935,7 +968,7 @@ static INLINE BOOL update_read_brush(wStream* s, rdpBrush* brush, BYTE fieldFlag Stream_Read_UINT8(s, brush->data[3]); Stream_Read_UINT8(s, brush->data[2]); Stream_Read_UINT8(s, brush->data[1]); - brush->data[0] = brush->hatch; + brush->data[0] = get_checked_uint8(brush->hatch); } return TRUE; @@ -944,17 +977,23 @@ static INLINE BOOL update_write_brush(wStream* s, rdpBrush* brush, BYTE fieldFla { if (fieldFlags & ORDER_FIELD_01) { - Stream_Write_UINT8(s, brush->x); + if (!Stream_EnsureRemainingCapacity(s, 1)) + return FALSE; + Stream_Write_UINT8(s, get_checked_uint8(brush->x)); } if (fieldFlags & ORDER_FIELD_02) { - Stream_Write_UINT8(s, brush->y); + if (!Stream_EnsureRemainingCapacity(s, 1)) + return FALSE; + Stream_Write_UINT8(s, get_checked_uint8(brush->y)); } if (fieldFlags & ORDER_FIELD_03) { - Stream_Write_UINT8(s, brush->style); + if (!Stream_EnsureRemainingCapacity(s, 1)) + return FALSE; + Stream_Write_UINT8(s, get_checked_uint8(brush->style)); } if (brush->style & CACHED_BRUSH) @@ -970,12 +1009,16 @@ static INLINE BOOL update_write_brush(wStream* s, rdpBrush* brush, BYTE fieldFla if (fieldFlags & ORDER_FIELD_04) { - Stream_Write_UINT8(s, brush->hatch); + if (!Stream_EnsureRemainingCapacity(s, 1)) + return FALSE; + Stream_Write_UINT8(s, get_checked_uint8(brush->hatch)); } if (fieldFlags & ORDER_FIELD_05) { brush->data = (BYTE*)brush->p8x8; + if (!Stream_EnsureRemainingCapacity(s, 7)) + return FALSE; Stream_Write_UINT8(s, brush->data[7]); Stream_Write_UINT8(s, brush->data[6]); Stream_Write_UINT8(s, brush->data[5]); @@ -983,7 +1026,7 @@ static INLINE BOOL update_write_brush(wStream* s, rdpBrush* brush, BYTE fieldFla Stream_Write_UINT8(s, brush->data[3]); Stream_Write_UINT8(s, brush->data[2]); Stream_Write_UINT8(s, brush->data[1]); - brush->data[0] = brush->hatch; + brush->data[0] = get_checked_uint8(brush->hatch); } return TRUE; @@ -1215,9 +1258,10 @@ static INLINE BOOL read_order_field_coord(const char* orderName, const ORDER_INF WINPR_ASSERT(orderInfo); WINPR_ASSERT(TARGET); - if (!order_field_flag_is_set(orderInfo, NO)) + if (!order_field_flag_is_set(orderInfo, get_checked_uint8(NO))) { - WLog_DBG(TAG, "order %s field %" PRIu8 " not found [optional:%d]", orderName, NO, optional); + WLog_DBG(TAG, "order %s field %" PRIu8 " not found [optional:%d]", orderName, + get_checked_uint8(NO), optional); return TRUE; } @@ -1231,9 +1275,10 @@ static INLINE BOOL read_order_field_color(const char* orderName, const ORDER_INF WINPR_ASSERT(orderInfo); WINPR_ASSERT(TARGET); - if (!order_field_flag_is_set(orderInfo, NO)) + if (!order_field_flag_is_set(orderInfo, get_checked_uint8(NO))) { - WLog_DBG(TAG, "order %s field %" PRIu8 " not found [optional:%d]", orderName, NO, optional); + WLog_DBG(TAG, "order %s field %" PRIu8 " not found [optional:%d]", orderName, + get_checked_uint8(NO), optional); return TRUE; } @@ -1296,7 +1341,7 @@ BOOL update_write_dstblt_order(wStream* s, ORDER_INFO* orderInfo, const DSTBLT_O if (!update_write_coord(s, dstblt->nHeight)) return FALSE; orderInfo->fieldFlags |= ORDER_FIELD_05; - Stream_Write_UINT8(s, dstblt->bRop); + Stream_Write_UINT8(s, get_checked_uint8(dstblt->bRop)); return TRUE; } @@ -1310,7 +1355,8 @@ static BOOL update_read_patblt_order(const char* orderName, wStream* s, const OR read_order_field_byte(orderName, orderInfo, s, 5, &patblt->bRop, TRUE) && read_order_field_color(orderName, orderInfo, s, 6, &patblt->backColor, TRUE) && read_order_field_color(orderName, orderInfo, s, 7, &patblt->foreColor, TRUE) && - update_read_brush(s, &patblt->brush, orderInfo->fieldFlags >> 7)) + update_read_brush(s, &patblt->brush, + get_checked_uint8((orderInfo->fieldFlags >> 7) & 0x1F))) return TRUE; return FALSE; } @@ -1341,7 +1387,7 @@ BOOL update_write_patblt_order(wStream* s, ORDER_INFO* orderInfo, PATBLT_ORDER* if (!update_write_coord(s, patblt->nHeight)) return FALSE; orderInfo->fieldFlags |= ORDER_FIELD_05; - Stream_Write_UINT8(s, patblt->bRop); + Stream_Write_UINT8(s, get_checked_uint8(patblt->bRop)); orderInfo->fieldFlags |= ORDER_FIELD_06; update_write_color(s, patblt->backColor); orderInfo->fieldFlags |= ORDER_FIELD_07; @@ -1351,7 +1397,7 @@ BOOL update_write_patblt_order(wStream* s, ORDER_INFO* orderInfo, PATBLT_ORDER* orderInfo->fieldFlags |= ORDER_FIELD_10; orderInfo->fieldFlags |= ORDER_FIELD_11; orderInfo->fieldFlags |= ORDER_FIELD_12; - update_write_brush(s, &patblt->brush, orderInfo->fieldFlags >> 7); + update_write_brush(s, &patblt->brush, get_checked_uint8((orderInfo->fieldFlags >> 7) & 0x1F)); return TRUE; } @@ -1553,7 +1599,8 @@ static BOOL update_read_multi_patblt_order(const char* orderName, wStream* s, !read_order_field_color(orderName, orderInfo, s, 7, &multi_patblt->foreColor, TRUE)) return FALSE; - if (!update_read_brush(s, &multi_patblt->brush, orderInfo->fieldFlags >> 7)) + if (!update_read_brush(s, &multi_patblt->brush, + get_checked_uint8((orderInfo->fieldFlags >> 7) & 0x1F))) return FALSE; UINT32 numRectangles = multi_patblt->numRectangles; @@ -1754,7 +1801,7 @@ BOOL update_write_line_to_order(wStream* s, ORDER_INFO* orderInfo, const LINE_TO orderInfo->fieldFlags = 0; orderInfo->fieldFlags |= ORDER_FIELD_01; - Stream_Write_UINT16(s, line_to->backMode); + Stream_Write_UINT16(s, get_checked_uint16(line_to->backMode)); orderInfo->fieldFlags |= ORDER_FIELD_02; if (!update_write_coord(s, line_to->nXStart)) return FALSE; @@ -1770,11 +1817,11 @@ BOOL update_write_line_to_order(wStream* s, ORDER_INFO* orderInfo, const LINE_TO orderInfo->fieldFlags |= ORDER_FIELD_06; update_write_color(s, line_to->backColor); orderInfo->fieldFlags |= ORDER_FIELD_07; - Stream_Write_UINT8(s, line_to->bRop2); + Stream_Write_UINT8(s, get_checked_uint8(line_to->bRop2)); orderInfo->fieldFlags |= ORDER_FIELD_08; - Stream_Write_UINT8(s, line_to->penStyle); + Stream_Write_UINT8(s, get_checked_uint8(line_to->penStyle)); orderInfo->fieldFlags |= ORDER_FIELD_09; - Stream_Write_UINT8(s, line_to->penWidth); + Stream_Write_UINT8(s, get_checked_uint8(line_to->penWidth)); orderInfo->fieldFlags |= ORDER_FIELD_10; update_write_color(s, line_to->penColor); return TRUE; @@ -1805,7 +1852,8 @@ static BOOL update_read_polyline_order(const char* orderName, wStream* s, polyline->numDeltaEntries = new_num; return update_read_delta_points(s, &polyline->points, polyline->numDeltaEntries, - polyline->xStart, polyline->yStart); + get_checked_int16(polyline->xStart), + get_checked_int16(polyline->yStart)); } if (new_num > polyline->numDeltaEntries) { @@ -1849,12 +1897,10 @@ size_t update_approximate_memblt_order(ORDER_INFO* orderInfo, const MEMBLT_ORDER BOOL update_write_memblt_order(wStream* s, ORDER_INFO* orderInfo, const MEMBLT_ORDER* memblt) { - UINT16 cacheId = 0; - if (!Stream_EnsureRemainingCapacity(s, update_approximate_memblt_order(orderInfo, memblt))) return FALSE; - cacheId = (memblt->cacheId & 0xFF) | ((memblt->colorIndex & 0xFF) << 8); + const UINT16 cacheId = (UINT16)((memblt->cacheId & 0xFF) | ((memblt->colorIndex & 0xFF) << 8)); orderInfo->fieldFlags |= ORDER_FIELD_01; Stream_Write_UINT16(s, cacheId); orderInfo->fieldFlags |= ORDER_FIELD_02; @@ -1870,7 +1916,7 @@ BOOL update_write_memblt_order(wStream* s, ORDER_INFO* orderInfo, const MEMBLT_O if (!update_write_coord(s, memblt->nHeight)) return FALSE; orderInfo->fieldFlags |= ORDER_FIELD_06; - Stream_Write_UINT8(s, memblt->bRop); + Stream_Write_UINT8(s, get_checked_uint8(memblt->bRop)); orderInfo->fieldFlags |= ORDER_FIELD_07; if (!update_write_coord(s, memblt->nXSrc)) return FALSE; @@ -1878,7 +1924,7 @@ BOOL update_write_memblt_order(wStream* s, ORDER_INFO* orderInfo, const MEMBLT_O if (!update_write_coord(s, memblt->nYSrc)) return FALSE; orderInfo->fieldFlags |= ORDER_FIELD_09; - Stream_Write_UINT16(s, memblt->cacheIndex); + Stream_Write_UINT16(s, get_checked_uint16(memblt->cacheIndex)); return TRUE; } static BOOL update_read_mem3blt_order(const char* orderName, wStream* s, @@ -1896,7 +1942,8 @@ static BOOL update_read_mem3blt_order(const char* orderName, wStream* s, !read_order_field_color(orderName, orderInfo, s, 10, &mem3blt->foreColor, TRUE)) return FALSE; - if (!update_read_brush(s, &mem3blt->brush, orderInfo->fieldFlags >> 10) || + if (!update_read_brush(s, &mem3blt->brush, + get_checked_uint8((orderInfo->fieldFlags >> 10) & 0x1F)) || !read_order_field_uint16(orderName, orderInfo, s, 16, &mem3blt->cacheIndex, TRUE)) return FALSE; mem3blt->colorIndex = (mem3blt->cacheId >> 8); @@ -1936,7 +1983,8 @@ static BOOL update_read_glyph_index_order(const char* orderName, wStream* s, !read_order_field_int16(orderName, orderInfo, s, 12, &glyph_index->opTop, TRUE) || !read_order_field_int16(orderName, orderInfo, s, 13, &glyph_index->opRight, TRUE) || !read_order_field_int16(orderName, orderInfo, s, 14, &glyph_index->opBottom, TRUE) || - !update_read_brush(s, &glyph_index->brush, orderInfo->fieldFlags >> 14) || + !update_read_brush(s, &glyph_index->brush, + get_checked_uint8((orderInfo->fieldFlags >> 14) & 0x1F)) || !read_order_field_int16(orderName, orderInfo, s, 20, &glyph_index->x, TRUE) || !read_order_field_int16(orderName, orderInfo, s, 21, &glyph_index->y, TRUE)) return FALSE; @@ -1974,47 +2022,59 @@ BOOL update_write_glyph_index_order(wStream* s, ORDER_INFO* orderInfo, if (!Stream_EnsureRemainingCapacity(s, inf)) return FALSE; + if (!Stream_EnsureRemainingCapacity(s, 4)) + return FALSE; orderInfo->fieldFlags = 0; orderInfo->fieldFlags |= ORDER_FIELD_01; - Stream_Write_UINT8(s, glyph_index->cacheId); + Stream_Write_UINT8(s, get_checked_uint8(glyph_index->cacheId)); orderInfo->fieldFlags |= ORDER_FIELD_02; - Stream_Write_UINT8(s, glyph_index->flAccel); + Stream_Write_UINT8(s, get_checked_uint8(glyph_index->flAccel)); orderInfo->fieldFlags |= ORDER_FIELD_03; - Stream_Write_UINT8(s, glyph_index->ulCharInc); + Stream_Write_UINT8(s, get_checked_uint8(glyph_index->ulCharInc)); orderInfo->fieldFlags |= ORDER_FIELD_04; - Stream_Write_UINT8(s, glyph_index->fOpRedundant); + Stream_Write_UINT8(s, get_checked_uint8(glyph_index->fOpRedundant)); orderInfo->fieldFlags |= ORDER_FIELD_05; - update_write_color(s, glyph_index->backColor); + if (!update_write_color(s, get_checked_uint8(glyph_index->backColor))) + return FALSE; orderInfo->fieldFlags |= ORDER_FIELD_06; - update_write_color(s, glyph_index->foreColor); + if (!update_write_color(s, glyph_index->foreColor)) + return FALSE; + + if (!Stream_EnsureRemainingCapacity(s, 14)) + return FALSE; orderInfo->fieldFlags |= ORDER_FIELD_07; - Stream_Write_UINT16(s, glyph_index->bkLeft); + Stream_Write_INT16(s, get_checked_int16(glyph_index->bkLeft)); orderInfo->fieldFlags |= ORDER_FIELD_08; - Stream_Write_UINT16(s, glyph_index->bkTop); + Stream_Write_INT16(s, get_checked_int16(glyph_index->bkTop)); orderInfo->fieldFlags |= ORDER_FIELD_09; - Stream_Write_UINT16(s, glyph_index->bkRight); + Stream_Write_INT16(s, get_checked_int16(glyph_index->bkRight)); orderInfo->fieldFlags |= ORDER_FIELD_10; - Stream_Write_UINT16(s, glyph_index->bkBottom); + Stream_Write_INT16(s, get_checked_int16(glyph_index->bkBottom)); orderInfo->fieldFlags |= ORDER_FIELD_11; - Stream_Write_UINT16(s, glyph_index->opLeft); + Stream_Write_INT16(s, get_checked_int16(glyph_index->opLeft)); orderInfo->fieldFlags |= ORDER_FIELD_12; - Stream_Write_UINT16(s, glyph_index->opTop); + Stream_Write_INT16(s, get_checked_int16(glyph_index->opTop)); orderInfo->fieldFlags |= ORDER_FIELD_13; - Stream_Write_UINT16(s, glyph_index->opRight); + Stream_Write_INT16(s, get_checked_int16(glyph_index->opRight)); orderInfo->fieldFlags |= ORDER_FIELD_14; - Stream_Write_UINT16(s, glyph_index->opBottom); + Stream_Write_INT16(s, get_checked_int16(glyph_index->opBottom)); orderInfo->fieldFlags |= ORDER_FIELD_15; orderInfo->fieldFlags |= ORDER_FIELD_16; orderInfo->fieldFlags |= ORDER_FIELD_17; orderInfo->fieldFlags |= ORDER_FIELD_18; orderInfo->fieldFlags |= ORDER_FIELD_19; - update_write_brush(s, &glyph_index->brush, orderInfo->fieldFlags >> 14); + if (!update_write_brush(s, &glyph_index->brush, + get_checked_uint8((orderInfo->fieldFlags >> 14) & 0x1F))) + return FALSE; + + if (!Stream_EnsureRemainingCapacity(s, 5ULL + glyph_index->cbData)) + return FALSE; orderInfo->fieldFlags |= ORDER_FIELD_20; - Stream_Write_UINT16(s, glyph_index->x); + Stream_Write_INT16(s, get_checked_int16(glyph_index->x)); orderInfo->fieldFlags |= ORDER_FIELD_21; - Stream_Write_UINT16(s, glyph_index->y); + Stream_Write_INT16(s, get_checked_int16(glyph_index->y)); orderInfo->fieldFlags |= ORDER_FIELD_22; - Stream_Write_UINT8(s, glyph_index->cbData); + Stream_Write_UINT8(s, get_checked_uint8(glyph_index->cbData)); Stream_Write(s, glyph_index->data, glyph_index->cbData); return TRUE; } @@ -2160,7 +2220,8 @@ static BOOL update_read_polygon_sc_order(const char* orderName, wStream* s, polygon_sc->numPoints = num; return update_read_delta_points(s, &polygon_sc->points, polygon_sc->numPoints, - polygon_sc->xStart, polygon_sc->yStart); + get_checked_int16(polygon_sc->xStart), + get_checked_int16(polygon_sc->yStart)); } if (num > polygon_sc->numPoints) { @@ -2183,7 +2244,8 @@ static BOOL update_read_polygon_cb_order(const char* orderName, wStream* s, !read_order_field_color(orderName, orderInfo, s, 6, &polygon_cb->foreColor, TRUE)) return FALSE; - if (!update_read_brush(s, &polygon_cb->brush, orderInfo->fieldFlags >> 6)) + if (!update_read_brush(s, &polygon_cb->brush, + get_checked_uint8((orderInfo->fieldFlags >> 6) & 0x1F))) return FALSE; if (!read_order_field_byte(orderName, orderInfo, s, 12, &num, TRUE)) @@ -2201,7 +2263,8 @@ static BOOL update_read_polygon_cb_order(const char* orderName, wStream* s, polygon_cb->numPoints = num; if (!update_read_delta_points(s, &polygon_cb->points, polygon_cb->numPoints, - polygon_cb->xStart, polygon_cb->yStart)) + get_checked_int16(polygon_cb->xStart), + get_checked_int16(polygon_cb->yStart))) return FALSE; } @@ -2240,7 +2303,8 @@ static BOOL update_read_ellipse_cb_order(const char* orderName, wStream* s, read_order_field_byte(orderName, orderInfo, s, 6, &ellipse_cb->fillMode, TRUE) && read_order_field_color(orderName, orderInfo, s, 7, &ellipse_cb->backColor, TRUE) && read_order_field_color(orderName, orderInfo, s, 8, &ellipse_cb->foreColor, TRUE) && - update_read_brush(s, &ellipse_cb->brush, orderInfo->fieldFlags >> 8)) + update_read_brush(s, &ellipse_cb->brush, + get_checked_uint8((orderInfo->fieldFlags >> 8) & 0x1F))) return TRUE; return FALSE; } @@ -2338,13 +2402,14 @@ BOOL update_write_cache_bitmap_order(wStream* s, const CACHE_BITMAP_ORDER* cache if ((*flags & NO_BITMAP_COMPRESSION_HDR) == 0) bitmapLength += 8; - Stream_Write_UINT8(s, cache_bitmap->cacheId); /* cacheId (1 byte) */ + Stream_Write_UINT8(s, get_checked_uint8(cache_bitmap->cacheId)); /* cacheId (1 byte) */ Stream_Write_UINT8(s, 0); /* pad1Octet (1 byte) */ - Stream_Write_UINT8(s, cache_bitmap->bitmapWidth); /* bitmapWidth (1 byte) */ - Stream_Write_UINT8(s, cache_bitmap->bitmapHeight); /* bitmapHeight (1 byte) */ - Stream_Write_UINT8(s, cache_bitmap->bitmapBpp); /* bitmapBpp (1 byte) */ - Stream_Write_UINT16(s, bitmapLength); /* bitmapLength (2 bytes) */ - Stream_Write_UINT16(s, cache_bitmap->cacheIndex); /* cacheIndex (2 bytes) */ + Stream_Write_UINT8(s, get_checked_uint8(cache_bitmap->bitmapWidth)); /* bitmapWidth (1 byte) */ + Stream_Write_UINT8(s, + get_checked_uint8(cache_bitmap->bitmapHeight)); /* bitmapHeight (1 byte) */ + Stream_Write_UINT8(s, get_checked_uint8(cache_bitmap->bitmapBpp)); /* bitmapBpp (1 byte) */ + Stream_Write_UINT16(s, get_checked_uint16(bitmapLength)); /* bitmapLength (2 bytes) */ + Stream_Write_UINT16(s, get_checked_uint16(cache_bitmap->cacheIndex)); /* cacheIndex (2 bytes) */ if (compressed) { @@ -2484,8 +2549,11 @@ BOOL update_write_cache_bitmap_v2_order(wStream* s, CACHE_BITMAP_V2_ORDER* cache bitsPerPixelId = get_bpp_bmf(cache_bitmap_v2->bitmapBpp, &rc); if (!rc) return FALSE; - *flags = (cache_bitmap_v2->cacheId & 0x0003) | (bitsPerPixelId << 3) | - ((cache_bitmap_v2->flags << 7) & 0xFF80); + WINPR_ASSERT(cache_bitmap_v2->cacheId <= 3); + WINPR_ASSERT(bitsPerPixelId <= 0x0f); + WINPR_ASSERT(cache_bitmap_v2->flags <= 0x1FF); + *flags = (UINT16)((cache_bitmap_v2->cacheId & 0x0003) | (bitsPerPixelId << 3) | + ((cache_bitmap_v2->flags << 7) & 0xFF80)); if (cache_bitmap_v2->flags & CBR2_PERSISTENT_KEY_PRESENT) { @@ -2517,12 +2585,16 @@ BOOL update_write_cache_bitmap_v2_order(wStream* s, CACHE_BITMAP_V2_ORDER* cache if (!(cache_bitmap_v2->flags & CBR2_NO_BITMAP_COMPRESSION_HDR)) { Stream_Write_UINT16( - s, cache_bitmap_v2->cbCompFirstRowSize); /* cbCompFirstRowSize (2 bytes) */ + s, get_checked_uint16( + cache_bitmap_v2->cbCompFirstRowSize)); /* cbCompFirstRowSize (2 bytes) */ Stream_Write_UINT16( - s, cache_bitmap_v2->cbCompMainBodySize); /* cbCompMainBodySize (2 bytes) */ - Stream_Write_UINT16(s, cache_bitmap_v2->cbScanWidth); /* cbScanWidth (2 bytes) */ + s, get_checked_uint16( + cache_bitmap_v2->cbCompMainBodySize)); /* cbCompMainBodySize (2 bytes) */ Stream_Write_UINT16( - s, cache_bitmap_v2->cbUncompressedSize); /* cbUncompressedSize (2 bytes) */ + s, get_checked_uint16(cache_bitmap_v2->cbScanWidth)); /* cbScanWidth (2 bytes) */ + Stream_Write_UINT16( + s, get_checked_uint16( + cache_bitmap_v2->cbUncompressedSize)); /* cbUncompressedSize (2 bytes) */ cache_bitmap_v2->bitmapLength = cache_bitmap_v2->cbCompMainBodySize; } @@ -2636,15 +2708,16 @@ BOOL update_write_cache_bitmap_v3_order(wStream* s, CACHE_BITMAP_V3_ORDER* cache return FALSE; *flags = (cache_bitmap_v3->cacheId & 0x00000003) | ((cache_bitmap_v3->flags << 7) & 0x0000FF80) | ((bitsPerPixelId << 3) & 0x00000078); - Stream_Write_UINT16(s, cache_bitmap_v3->cacheIndex); /* cacheIndex (2 bytes) */ + Stream_Write_UINT16(s, + get_checked_uint16(cache_bitmap_v3->cacheIndex)); /* cacheIndex (2 bytes) */ Stream_Write_UINT32(s, cache_bitmap_v3->key1); /* key1 (4 bytes) */ Stream_Write_UINT32(s, cache_bitmap_v3->key2); /* key2 (4 bytes) */ - Stream_Write_UINT8(s, bitmapData->bpp); + Stream_Write_UINT8(s, get_checked_uint8(bitmapData->bpp)); Stream_Write_UINT8(s, 0); /* reserved1 (1 byte) */ Stream_Write_UINT8(s, 0); /* reserved2 (1 byte) */ - Stream_Write_UINT8(s, bitmapData->codecID); /* codecID (1 byte) */ - Stream_Write_UINT16(s, bitmapData->width); /* width (2 bytes) */ - Stream_Write_UINT16(s, bitmapData->height); /* height (2 bytes) */ + Stream_Write_UINT8(s, get_checked_uint8(bitmapData->codecID)); /* codecID (1 byte) */ + Stream_Write_UINT16(s, get_checked_uint16(bitmapData->width)); /* width (2 bytes) */ + Stream_Write_UINT16(s, get_checked_uint16(bitmapData->height)); /* height (2 bytes) */ Stream_Write_UINT32(s, bitmapData->length); /* length (4 bytes) */ Stream_Write(s, bitmapData->data, bitmapData->length); return TRUE; @@ -2713,8 +2786,10 @@ BOOL update_write_cache_color_table_order(wStream* s, if (!Stream_EnsureRemainingCapacity(s, inf)) return FALSE; - Stream_Write_UINT8(s, cache_color_table->cacheIndex); /* cacheIndex (1 byte) */ - Stream_Write_UINT16(s, cache_color_table->numberColors); /* numberColors (2 bytes) */ + Stream_Write_UINT8(s, + get_checked_uint8(cache_color_table->cacheIndex)); /* cacheIndex (1 byte) */ + Stream_Write_UINT16( + s, get_checked_uint16(cache_color_table->numberColors)); /* numberColors (2 bytes) */ colorTable = (const UINT32*)&cache_color_table->colorTable; for (size_t i = 0; i < cache_color_table->numberColors; i++) @@ -2797,27 +2872,24 @@ size_t update_approximate_cache_glyph_order(const CACHE_GLYPH_ORDER* cache_glyph BOOL update_write_cache_glyph_order(wStream* s, const CACHE_GLYPH_ORDER* cache_glyph, UINT16* flags) { - INT16 lsi16 = 0; const GLYPH_DATA* glyph = NULL; size_t inf = update_approximate_cache_glyph_order(cache_glyph, flags); if (!Stream_EnsureRemainingCapacity(s, inf)) return FALSE; - Stream_Write_UINT8(s, cache_glyph->cacheId); /* cacheId (1 byte) */ - Stream_Write_UINT8(s, cache_glyph->cGlyphs); /* cGlyphs (1 byte) */ + Stream_Write_UINT8(s, get_checked_uint8(cache_glyph->cacheId)); /* cacheId (1 byte) */ + Stream_Write_UINT8(s, get_checked_uint8(cache_glyph->cGlyphs)); /* cGlyphs (1 byte) */ for (UINT32 i = 0; i < cache_glyph->cGlyphs; i++) { UINT32 cb = 0; glyph = &cache_glyph->glyphData[i]; - Stream_Write_UINT16(s, glyph->cacheIndex); /* cacheIndex (2 bytes) */ - lsi16 = glyph->x; - Stream_Write_UINT16(s, lsi16); /* x (2 bytes) */ - lsi16 = glyph->y; - Stream_Write_UINT16(s, lsi16); /* y (2 bytes) */ - Stream_Write_UINT16(s, glyph->cx); /* cx (2 bytes) */ - Stream_Write_UINT16(s, glyph->cy); /* cy (2 bytes) */ + Stream_Write_UINT16(s, get_checked_uint16(glyph->cacheIndex)); /* cacheIndex (2 bytes) */ + Stream_Write_INT16(s, glyph->x); /* x (2 bytes) */ + Stream_Write_INT16(s, glyph->y); /* y (2 bytes) */ + Stream_Write_UINT16(s, get_checked_uint16(glyph->cx)); /* cx (2 bytes) */ + Stream_Write_UINT16(s, get_checked_uint16(glyph->cy)); /* cy (2 bytes) */ cb = ((glyph->cx + 7) / 8) * glyph->cy; cb += ((cb % 4) > 0) ? 4 - (cb % 4) : 0; Stream_Write(s, glyph->aj, cb); @@ -2908,14 +2980,17 @@ BOOL update_write_cache_glyph_v2_order(wStream* s, const CACHE_GLYPH_V2_ORDER* c if (!Stream_EnsureRemainingCapacity(s, inf)) return FALSE; - *flags = (cache_glyph_v2->cacheId & 0x000F) | ((cache_glyph_v2->flags & 0x000F) << 4) | - ((cache_glyph_v2->cGlyphs & 0x00FF) << 8); + WINPR_ASSERT(cache_glyph_v2->cacheId <= 0x0F); + WINPR_ASSERT(cache_glyph_v2->flags <= 0x0F); + WINPR_ASSERT(cache_glyph_v2->cGlyphs <= 0xFF); + *flags = (UINT16)((cache_glyph_v2->cacheId & 0x000F) | ((cache_glyph_v2->flags & 0x000F) << 4) | + ((cache_glyph_v2->cGlyphs & 0x00FF) << 8)); for (UINT32 i = 0; i < cache_glyph_v2->cGlyphs; i++) { UINT32 cb = 0; const GLYPH_DATA_V2* glyph = &cache_glyph_v2->glyphData[i]; - Stream_Write_UINT8(s, glyph->cacheIndex); + Stream_Write_UINT8(s, get_checked_uint8(glyph->cacheIndex)); if (!update_write_2byte_signed(s, glyph->x) || !update_write_2byte_signed(s, glyph->y) || !update_write_2byte_unsigned(s, glyph->cx) || @@ -3033,7 +3108,7 @@ static CACHE_BRUSH_ORDER* update_read_cache_brush_order(rdpUpdate* update, wStre { /* compressed brush */ if (!update_decompress_brush(s, cache_brush->data, sizeof(cache_brush->data), - cache_brush->bpp)) + get_checked_uint8(cache_brush->bpp))) goto fail; } else @@ -3080,12 +3155,12 @@ BOOL update_write_cache_brush_order(wStream* s, const CACHE_BRUSH_ORDER* cache_b iBitmapFormat = get_bpp_bmf(cache_brush->bpp, &rc); if (!rc) return FALSE; - Stream_Write_UINT8(s, cache_brush->index); /* cacheEntry (1 byte) */ + Stream_Write_UINT8(s, get_checked_uint8(cache_brush->index)); /* cacheEntry (1 byte) */ Stream_Write_UINT8(s, iBitmapFormat); /* iBitmapFormat (1 byte) */ - Stream_Write_UINT8(s, cache_brush->cx); /* cx (1 byte) */ - Stream_Write_UINT8(s, cache_brush->cy); /* cy (1 byte) */ - Stream_Write_UINT8(s, cache_brush->style); /* style (1 byte) */ - Stream_Write_UINT8(s, cache_brush->length); /* iBytes (1 byte) */ + Stream_Write_UINT8(s, get_checked_uint8(cache_brush->cx)); /* cx (1 byte) */ + Stream_Write_UINT8(s, get_checked_uint8(cache_brush->cy)); /* cy (1 byte) */ + Stream_Write_UINT8(s, get_checked_uint8(cache_brush->style)); /* style (1 byte) */ + Stream_Write_UINT8(s, get_checked_uint8(cache_brush->length)); /* iBytes (1 byte) */ if ((cache_brush->cx == 8) && (cache_brush->cy == 8)) { @@ -3114,7 +3189,8 @@ BOOL update_write_cache_brush_order(wStream* s, const CACHE_BRUSH_ORDER* cache_b if (compressed != FALSE) { /* compressed brush */ - if (!update_compress_brush(s, cache_brush->data, cache_brush->bpp)) + if (!update_compress_brush(s, cache_brush->data, + get_checked_uint8(cache_brush->bpp))) return FALSE; } else @@ -3225,12 +3301,12 @@ BOOL update_write_create_offscreen_bitmap_order( flags |= 0x8000; Stream_Write_UINT16(s, flags); /* flags (2 bytes) */ - Stream_Write_UINT16(s, create_offscreen_bitmap->cx); /* cx (2 bytes) */ - Stream_Write_UINT16(s, create_offscreen_bitmap->cy); /* cy (2 bytes) */ + Stream_Write_UINT16(s, get_checked_uint16(create_offscreen_bitmap->cx)); /* cx (2 bytes) */ + Stream_Write_UINT16(s, get_checked_uint16(create_offscreen_bitmap->cy)); /* cy (2 bytes) */ if (deleteListPresent) { - Stream_Write_UINT16(s, deleteList->cIndices); + Stream_Write_UINT16(s, get_checked_uint16(deleteList->cIndices)); for (size_t i = 0; i < deleteList->cIndices; i++) { @@ -3545,7 +3621,7 @@ BOOL update_write_bounds(wStream* s, ORDER_INFO* orderInfo) if (orderInfo->controlFlags & ORDER_ZERO_BOUNDS_DELTAS) return TRUE; - Stream_Write_UINT8(s, orderInfo->boundsFlags); /* field flags */ + Stream_Write_UINT8(s, get_checked_uint8(orderInfo->boundsFlags)); /* field flags */ if (orderInfo->boundsFlags & BOUND_LEFT) { diff --git a/libfreerdp/gdi/gdi.c b/libfreerdp/gdi/gdi.c index 7e712bc73..f80ada8ae 100644 --- a/libfreerdp/gdi/gdi.c +++ b/libfreerdp/gdi/gdi.c @@ -323,9 +323,11 @@ static const BYTE GDI_BS_HATCHED_PATTERNS[] = { 0x7E, 0xBD, 0xDB, 0xE7, 0xE7, 0xDB, 0xBD, 0x7E /* HS_DIACROSS */ }; -static inline DWORD gdi_rop3_code_checked(UINT32 code) +#define gdi_rop3_code_checked(code) gdi_rop3_code_checked_int((code), __FILE__, __func__, __LINE__) +static inline DWORD gdi_rop3_code_checked_int(UINT32 code, const char* file, const char* fkt, + size_t line) { - WINPR_ASSERT(code <= UINT8_MAX); + WINPR_ASSERT_AT(code <= UINT8_MAX, file, fkt, line); return gdi_rop3_code((UINT8)code); } diff --git a/winpr/include/winpr/platform.h b/winpr/include/winpr/platform.h index a3c95366e..41225d47b 100644 --- a/winpr/include/winpr/platform.h +++ b/winpr/include/winpr/platform.h @@ -79,8 +79,8 @@ #define WINPR_PRAGMA_DIAG_PUSH WINPR_DO_PRAGMA(clang diagnostic push) #define WINPR_PRAGMA_DIAG_IGNORED_OVERLENGTH_STRINGS \ WINPR_DO_PRAGMA(clang diagnostic ignored "-Woverlength-strings") /** @since version 3.9.0 */ -#define WINPR_PRAGMA_DIAG_IGNORED_QUALIFIERS \ - WINPR_DO_PRAGMA(clang diagnostic ignored "-Wdiscarded-qualifiers") /** @since version 3.9.0 */ +#define WINPR_PRAGMA_DIAG_IGNORED_QUALIFIERS +/* unsupported by clang WINPR_DO_PRAGMA(clang diagnostic ignored "-Wdiscarded-qualifiers") */ /** @since version 3.9.0 */ #define WINPR_PRAGMA_DIAG_IGNORED_PEDANTIC WINPR_DO_PRAGMA(clang diagnostic ignored "-Wpedantic") #define WINPR_PRAGMA_DIAG_IGNORED_MISSING_PROTOTYPES \ WINPR_DO_PRAGMA(clang diagnostic ignored "-Wmissing-prototypes") diff --git a/winpr/include/winpr/synch.h b/winpr/include/winpr/synch.h index 6d24a0cb9..809651c29 100644 --- a/winpr/include/winpr/synch.h +++ b/winpr/include/winpr/synch.h @@ -321,8 +321,8 @@ extern "C" WINPR_API BOOL DeleteTimerQueue(HANDLE TimerQueue); WINPR_API BOOL DeleteTimerQueueEx(HANDLE TimerQueue, HANDLE CompletionEvent); - WINPR_API BOOL CreateTimerQueueTimer(PHANDLE phNewTimer, HANDLE TimerQueue, - WAITORTIMERCALLBACK Callback, PVOID Parameter, + WINPR_API BOOL CreateTimerQueueTimer(HANDLE* phNewTimer, HANDLE TimerQueue, + WAITORTIMERCALLBACK Callback, void* Parameter, DWORD DueTime, DWORD Period, ULONG Flags); WINPR_API BOOL ChangeTimerQueueTimer(HANDLE TimerQueue, HANDLE Timer, ULONG DueTime, ULONG Period); diff --git a/winpr/libwinpr/synch/event.c b/winpr/libwinpr/synch/event.c index b18ea5677..2541083e5 100644 --- a/winpr/libwinpr/synch/event.c +++ b/winpr/libwinpr/synch/event.c @@ -171,13 +171,13 @@ BOOL winpr_event_reset(WINPR_EVENT_IMPL* event) void winpr_event_uninit(WINPR_EVENT_IMPL* event) { - if (event->fds[0] != -1) + if (event->fds[0] >= 0) { close(event->fds[0]); event->fds[0] = -1; } - if (event->fds[1] != -1) + if (event->fds[1] >= 0) { close(event->fds[1]); event->fds[1] = -1; diff --git a/winpr/libwinpr/synch/timer.c b/winpr/libwinpr/synch/timer.c index 6b40de177..61a682af5 100644 --- a/winpr/libwinpr/synch/timer.c +++ b/winpr/libwinpr/synch/timer.c @@ -990,25 +990,23 @@ BOOL DeleteTimerQueue(HANDLE TimerQueue) return DeleteTimerQueueEx(TimerQueue, NULL); } -BOOL CreateTimerQueueTimer(PHANDLE phNewTimer, HANDLE TimerQueue, WAITORTIMERCALLBACK Callback, - PVOID Parameter, DWORD DueTime, DWORD Period, ULONG Flags) +BOOL CreateTimerQueueTimer(HANDLE* phNewTimer, HANDLE TimerQueue, WAITORTIMERCALLBACK Callback, + void* Parameter, DWORD DueTime, DWORD Period, ULONG Flags) { - struct timespec CurrentTime; - WINPR_TIMER_QUEUE* timerQueue = NULL; - WINPR_TIMER_QUEUE_TIMER* timer = NULL; + struct timespec CurrentTime = { 0 }; if (!TimerQueue) return FALSE; timespec_gettimeofday(&CurrentTime); - timerQueue = (WINPR_TIMER_QUEUE*)TimerQueue; - timer = (WINPR_TIMER_QUEUE_TIMER*)malloc(sizeof(WINPR_TIMER_QUEUE_TIMER)); + WINPR_TIMER_QUEUE* timerQueue = (WINPR_TIMER_QUEUE*)TimerQueue; + WINPR_TIMER_QUEUE_TIMER* timer = calloc(1, sizeof(WINPR_TIMER_QUEUE_TIMER)); if (!timer) return FALSE; WINPR_HANDLE_SET_TYPE_AND_MODE(timer, HANDLE_TYPE_TIMER_QUEUE_TIMER, WINPR_FD_READ); - *((UINT_PTR*)phNewTimer) = (UINT_PTR)(HANDLE)timer; + *phNewTimer = (HANDLE)timer; timespec_copy(&(timer->StartTime), &CurrentTime); timespec_add_ms(&(timer->StartTime), DueTime); timespec_copy(&(timer->ExpirationTime), &(timer->StartTime)); diff --git a/winpr/libwinpr/thread/argv.c b/winpr/libwinpr/thread/argv.c index 090d60dd7..f219503e1 100644 --- a/winpr/libwinpr/thread/argv.c +++ b/winpr/libwinpr/thread/argv.c @@ -244,8 +244,8 @@ LPSTR* CommandLineToArgvA(LPCSTR lpCmdLine, int* pNumArgs) if (*p != '"') WLog_ERR(TAG, "parsing error: uneven number of unescaped double quotes!"); - if (*p && *(++p)) - p += strcspn(p, " \t\0"); + if (p[0] && p[1]) + p += 1 + strcspn(&p[1], " \t\0"); pArgs[numArgs++] = pOutput; diff --git a/winpr/libwinpr/thread/test/TestThreadCommandLineToArgv.c b/winpr/libwinpr/thread/test/TestThreadCommandLineToArgv.c index 33d0c94f4..51906b65b 100644 --- a/winpr/libwinpr/thread/test/TestThreadCommandLineToArgv.c +++ b/winpr/libwinpr/thread/test/TestThreadCommandLineToArgv.c @@ -5,55 +5,76 @@ static const char* test_args_line_1 = "app.exe abc d e"; -static const char* test_args_list_1[] = { "app.exe", "abc", "d", "e", NULL }; +static const char* test_args_list_1[] = { "app.exe", "abc", "d", "e" }; static const char* test_args_line_2 = "app.exe abc \t def"; -static const char* test_args_list_2[] = { "app.exe", "abc", "def", NULL }; +static const char* test_args_list_2[] = { "app.exe", "abc", "def" }; static const char* test_args_line_3 = "app.exe \"abc\" d e"; -static const char* test_args_list_3[] = { "app.exe", "abc", "d", "e", NULL }; +static const char* test_args_list_3[] = { "app.exe", "abc", "d", "e" }; static const char* test_args_line_4 = "app.exe a\\\\b d\"e f\"g h"; -static const char* test_args_list_4[] = { "app.exe", "a\\\\b", "de fg", "h", NULL }; +static const char* test_args_list_4[] = { "app.exe", "a\\\\b", "de fg", "h" }; static const char* test_args_line_5 = "app.exe a\\\\\\\"b c d"; -static const char* test_args_list_5[] = { "app.exe", "a\\\"b", "c", "d", NULL }; +static const char* test_args_list_5[] = { "app.exe", "a\\\"b", "c", "d" }; static const char* test_args_line_6 = "app.exe a\\\\\\\\\"b c\" d e"; -static const char* test_args_list_6[] = { "app.exe", "a\\\\b c", "d", "e", NULL }; +static const char* test_args_list_6[] = { "app.exe", "a\\\\b c", "d", "e" }; static const char* test_args_line_7 = "app.exe a\\\\\\\\\"b c\" d e f\\\\\\\\\"g h\" i j"; -static const char* test_args_list_7[] = { "app.exe", "a\\\\b c", "d", "e", - "f\\\\g h", "i", "j", NULL }; +static const char* test_args_list_7[] = { "app.exe", "a\\\\b c", "d", "e", "f\\\\g h", "i", "j" }; -static int test_command_line_parsing_case(const char* line, const char** list) +static BOOL test_command_line_parsing_case(const char* line, const char** list, size_t expect) { - LPSTR* pArgs = NULL; + BOOL rc = FALSE; int numArgs = 0; - pArgs = NULL; - numArgs = 0; - printf("Parsing: %s\n", line); - pArgs = CommandLineToArgvA(line, &numArgs); + LPSTR* pArgs = CommandLineToArgvA(line, &numArgs); + if (numArgs < 0) + { + (void)fprintf(stderr, "expected %" PRIuz " arguments, got %d return\n", expect, numArgs); + goto fail; + } + if (numArgs != expect) + { + (void)fprintf(stderr, "expected %" PRIuz " arguments, got %d from '%s'\n", expect, numArgs, + line); + goto fail; + } + + if ((numArgs > 0) && !pArgs) + { + (void)fprintf(stderr, "expected %d arguments, got NULL return\n", numArgs); + goto fail; + } printf("pNumArgs: %d\n", numArgs); for (int i = 0; i < numArgs; i++) { printf("argv[%d] = %s\n", i, pArgs[i]); + if (strcmp(pArgs[i], list[i]) != 0) + { + (void)fprintf(stderr, "failed at argument %d: got '%s' but expected '%s'\n", i, + pArgs[i], list[i]); + goto fail; + } } + rc = TRUE; +fail: free(pArgs); - return 0; + return rc; } int TestThreadCommandLineToArgv(int argc, char* argv[]) @@ -62,13 +83,27 @@ int TestThreadCommandLineToArgv(int argc, char* argv[]) WINPR_UNUSED(argc); WINPR_UNUSED(argv); - test_command_line_parsing_case(test_args_line_1, test_args_list_1); - test_command_line_parsing_case(test_args_line_2, test_args_list_2); - test_command_line_parsing_case(test_args_line_3, test_args_list_3); - test_command_line_parsing_case(test_args_line_4, test_args_list_4); - test_command_line_parsing_case(test_args_line_5, test_args_list_5); - test_command_line_parsing_case(test_args_line_6, test_args_list_6); - test_command_line_parsing_case(test_args_line_7, test_args_list_7); + if (!test_command_line_parsing_case(test_args_line_1, test_args_list_1, + ARRAYSIZE(test_args_list_1))) + return -1; + if (!test_command_line_parsing_case(test_args_line_2, test_args_list_2, + ARRAYSIZE(test_args_list_2))) + return -1; + if (!test_command_line_parsing_case(test_args_line_3, test_args_list_3, + ARRAYSIZE(test_args_list_3))) + return -1; + if (!test_command_line_parsing_case(test_args_line_4, test_args_list_4, + ARRAYSIZE(test_args_list_4))) + return -1; + if (!test_command_line_parsing_case(test_args_line_5, test_args_list_5, + ARRAYSIZE(test_args_list_5))) + return -1; + if (!test_command_line_parsing_case(test_args_line_6, test_args_list_6, + ARRAYSIZE(test_args_list_6))) + return -1; + if (!test_command_line_parsing_case(test_args_line_7, test_args_list_7, + ARRAYSIZE(test_args_list_7))) + return -1; return 0; }