From f084389cd7338b95622e43f30fa9c60b61761f49 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 31 Oct 2024 10:07:02 +0100 Subject: [PATCH 01/18] [channels,urbdrc] fix enum-enum conversion * fix integer type of port, use uint8_t * cast enum values to uint8_t (enum values are used as constants here and multiple different ones are mixed together which provokes this warning) --- channels/urbdrc/client/libusb/libusb_udevice.c | 13 ++++++++----- channels/urbdrc/client/libusb/libusb_udevice.h | 2 +- channels/urbdrc/client/urbdrc_main.h | 4 +++- 3 files changed, 12 insertions(+), 7 deletions(-) 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*); From 7aa233590ba8539df7d7474a887da3f14b5998e2 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 31 Oct 2024 10:01:29 +0100 Subject: [PATCH 02/18] [channels,tsmf] fix incompatible pointer types --- channels/tsmf/client/ffmpeg/tsmf_ffmpeg.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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); From 163e74844fc23a8128cef61a9fbf2574f18eeb88 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 31 Oct 2024 10:09:21 +0100 Subject: [PATCH 03/18] [pragma] clang does not support -Wdiscarded-qualifiers --- winpr/include/winpr/platform.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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") From ecbccecef6f5862630a844378b0cc2c7298dc913 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 31 Oct 2024 10:16:08 +0100 Subject: [PATCH 04/18] [client,sdl] fix global-constructors --- client/SDL/SDL3/sdl_clip.cpp | 52 +++++++++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 13 deletions(-) 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) From 79bbb958847d63ff10fc62818a6e167f93851504 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 31 Oct 2024 11:19:24 +0100 Subject: [PATCH 05/18] [client,sdl] use std::min --- client/SDL/SDL2/dialogs/sdl_widget.cpp | 4 ++-- client/SDL/SDL3/dialogs/sdl_widget.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) 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; } From 402cc34e8c0cc64950dc4a68304944d9eafb4ef2 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 31 Oct 2024 11:21:02 +0100 Subject: [PATCH 06/18] [winpr,synch] fix timer HANDLE casts --- winpr/include/winpr/synch.h | 4 ++-- winpr/libwinpr/synch/timer.c | 14 ++++++-------- 2 files changed, 8 insertions(+), 10 deletions(-) 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/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)); From 3029af1a5a24319918544a504e3eae95c1a818f2 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 31 Oct 2024 11:31:05 +0100 Subject: [PATCH 07/18] [winpr,synch] check for valid file descriptor --- winpr/libwinpr/synch/event.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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; From 38494f5ce73d60c46fd2ad3141409b436ee6f1e8 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 31 Oct 2024 11:37:50 +0100 Subject: [PATCH 08/18] [clang,tidy] deactivate suggestions for boost --- .clang-tidy | 1 - 1 file changed, 1 deletion(-) 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*, From cbffbd16dbf88a91b343089bca6a8acf6a4a11f7 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 31 Oct 2024 11:40:19 +0100 Subject: [PATCH 09/18] [winpr,thread] fix bugprone dec in condition --- winpr/libwinpr/thread/argv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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; From 8e7637ce1bc8e459a8778ac7d782af59d353e3ea Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 31 Oct 2024 11:49:58 +0100 Subject: [PATCH 10/18] implicit-int-conversion --- libfreerdp/core/fastpath.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/libfreerdp/core/fastpath.c b/libfreerdp/core/fastpath.c index e7fdac178..772fc43e2 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) From 841151c456cded8895b64188dabfd0ee34e85720 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 31 Oct 2024 12:17:28 +0100 Subject: [PATCH 11/18] implicit-int-conversion --- libfreerdp/core/orders.c | 219 +++++++++++++++++++++++---------------- 1 file changed, 128 insertions(+), 91 deletions(-) diff --git a/libfreerdp/core/orders.c b/libfreerdp/core/orders.c index ae037507e..60db54384 100644 --- a/libfreerdp/core/orders.c +++ b/libfreerdp/core/orders.c @@ -43,6 +43,27 @@ #define TAG FREERDP_TAG("core.orders") +/* 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 */ +static inline UINT16 get_checked_uint16(UINT32 value) +{ + WINPR_ASSERT(value <= UINT16_MAX); + return (UINT16)value; +} + +static inline UINT8 get_checked_uint8(UINT32 value) +{ + WINPR_ASSERT(value <= UINT8_MAX); + return (UINT8)value; +} + +static inline INT16 get_checked_int16(INT32 value) +{ + WINPR_ASSERT(value <= INT16_MAX); + WINPR_ASSERT(value >= INT16_MIN); + return (INT16)value; +} + static inline const char* gdi_rob3_code_string_checked(UINT32 rob) { WINPR_ASSERT((rob) <= UINT8_MAX); @@ -935,7 +956,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 +965,17 @@ static INLINE BOOL update_write_brush(wStream* s, rdpBrush* brush, BYTE fieldFla { if (fieldFlags & ORDER_FIELD_01) { - Stream_Write_UINT8(s, brush->x); + Stream_Write_UINT8(s, get_checked_uint8(brush->x)); } if (fieldFlags & ORDER_FIELD_02) { - Stream_Write_UINT8(s, brush->y); + Stream_Write_UINT8(s, get_checked_uint8(brush->y)); } if (fieldFlags & ORDER_FIELD_03) { - Stream_Write_UINT8(s, brush->style); + Stream_Write_UINT8(s, get_checked_uint8(brush->style)); } if (brush->style & CACHED_BRUSH) @@ -970,7 +991,7 @@ static INLINE BOOL update_write_brush(wStream* s, rdpBrush* brush, BYTE fieldFla if (fieldFlags & ORDER_FIELD_04) { - Stream_Write_UINT8(s, brush->hatch); + Stream_Write_UINT8(s, get_checked_uint8(brush->hatch)); } if (fieldFlags & ORDER_FIELD_05) @@ -983,7 +1004,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 +1236,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 +1253,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 +1319,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 +1333,7 @@ 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))) return TRUE; return FALSE; } @@ -1341,7 +1364,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 +1374,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)); return TRUE; } @@ -1553,7 +1576,7 @@ 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))) return FALSE; UINT32 numRectangles = multi_patblt->numRectangles; @@ -1754,7 +1777,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 +1793,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 +1828,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 +1873,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 +1892,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 +1900,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 +1918,7 @@ 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)) || !read_order_field_uint16(orderName, orderInfo, s, 16, &mem3blt->cacheIndex, TRUE)) return FALSE; mem3blt->colorIndex = (mem3blt->cacheId >> 8); @@ -1936,7 +1958,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)) || !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; @@ -1976,45 +1999,45 @@ BOOL update_write_glyph_index_order(wStream* s, ORDER_INFO* orderInfo, 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); + update_write_color(s, get_checked_uint8(glyph_index->backColor)); orderInfo->fieldFlags |= ORDER_FIELD_06; update_write_color(s, glyph_index->foreColor); 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); + update_write_brush(s, &glyph_index->brush, get_checked_uint8(orderInfo->fieldFlags >> 14)); 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 +2183,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 +2207,7 @@ 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))) return FALSE; if (!read_order_field_byte(orderName, orderInfo, s, 12, &num, TRUE)) @@ -2201,7 +2225,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 +2265,7 @@ 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))) return TRUE; return FALSE; } @@ -2338,13 +2363,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 +2510,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 +2546,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 +2669,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 +2747,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 +2833,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 +2941,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 +3069,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 +3116,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 +3150,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 +3262,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 +3582,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) { From 0dc709a64d0ca7daaceef99816da2c6058ecb48d Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 31 Oct 2024 12:50:24 +0100 Subject: [PATCH 12/18] implicit-int-conversion --- libfreerdp/core/fastpath.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/libfreerdp/core/fastpath.c b/libfreerdp/core/fastpath.c index 772fc43e2..8a7e61386 100644 --- a/libfreerdp/core/fastpath.c +++ b/libfreerdp/core/fastpath.c @@ -1190,8 +1190,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); + WINPR_ASSERT(CompressionMaxSize <= UINT16_MAX); + maxLength = (maxLength < CompressionMaxSize) ? maxLength : (UINT16)(CompressionMaxSize); maxLength -= 20; } @@ -1235,7 +1235,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; @@ -1252,7 +1252,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; } } @@ -1264,7 +1265,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) @@ -1296,7 +1299,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; From 64f4acdd2f01c082ed77f57a67a167baca7594d1 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Mon, 4 Nov 2024 08:33:06 +0100 Subject: [PATCH 13/18] [core,gdi] improve range asserts use WINPR_ASSERT_AT to pinpoint location of call of checker function --- libfreerdp/core/orders.c | 34 +++++++++++++++++++++++----------- libfreerdp/gdi/gdi.c | 6 ++++-- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/libfreerdp/core/orders.c b/libfreerdp/core/orders.c index 60db54384..3044ad7d9 100644 --- a/libfreerdp/core/orders.c +++ b/libfreerdp/core/orders.c @@ -45,34 +45,46 @@ /* 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 */ -static inline UINT16 get_checked_uint16(UINT32 value) +#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(value <= UINT16_MAX); + WINPR_ASSERT_AT(value <= UINT16_MAX, file, fkt, line); return (UINT16)value; } -static inline UINT8 get_checked_uint8(UINT32 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(value <= UINT8_MAX); + WINPR_ASSERT_AT(value <= UINT8_MAX, file, fkt, line); return (UINT8)value; } -static inline INT16 get_checked_int16(INT32 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(value <= INT16_MAX); - WINPR_ASSERT(value >= INT16_MIN); + WINPR_ASSERT_AT(value <= INT16_MAX, file, fkt, line); + WINPR_ASSERT_AT(value >= INT16_MIN, file, fkt, line); return (INT16)value; } -static inline const char* gdi_rob3_code_string_checked(UINT32 rob) +#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((rob) <= UINT8_MAX); + 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); } 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); } From ec3e8b564e1d9f1c11cee8cde652b825c3a5148c Mon Sep 17 00:00:00 2001 From: akallabeth Date: Mon, 4 Nov 2024 08:59:51 +0100 Subject: [PATCH 14/18] fuzz-verbose-assert --- ci/cmake-preloads/config-oss-fuzz.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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") From 1977338a3267cece245ab3c2cfa59c2d28ee2b54 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Mon, 4 Nov 2024 09:28:34 +0100 Subject: [PATCH 15/18] [core,orders] fix update_write_brush * Fix calls, the fieldFlags need to be masked * Fix implementation, ensure the size is correct and abort on failures --- libfreerdp/core/orders.c | 47 +++++++++++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 10 deletions(-) diff --git a/libfreerdp/core/orders.c b/libfreerdp/core/orders.c index 3044ad7d9..6c5f98ab6 100644 --- a/libfreerdp/core/orders.c +++ b/libfreerdp/core/orders.c @@ -977,16 +977,22 @@ static INLINE BOOL update_write_brush(wStream* s, rdpBrush* brush, BYTE fieldFla { if (fieldFlags & ORDER_FIELD_01) { + if (!Stream_EnsureRemainingCapacity(s, 1)) + return FALSE; Stream_Write_UINT8(s, get_checked_uint8(brush->x)); } if (fieldFlags & ORDER_FIELD_02) { + if (!Stream_EnsureRemainingCapacity(s, 1)) + return FALSE; Stream_Write_UINT8(s, get_checked_uint8(brush->y)); } if (fieldFlags & ORDER_FIELD_03) { + if (!Stream_EnsureRemainingCapacity(s, 1)) + return FALSE; Stream_Write_UINT8(s, get_checked_uint8(brush->style)); } @@ -1003,12 +1009,16 @@ static INLINE BOOL update_write_brush(wStream* s, rdpBrush* brush, BYTE fieldFla if (fieldFlags & ORDER_FIELD_04) { + 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]); @@ -1345,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, get_checked_uint8(orderInfo->fieldFlags >> 7))) + update_read_brush(s, &patblt->brush, + get_checked_uint8((orderInfo->fieldFlags >> 7) & 0x1F))) return TRUE; return FALSE; } @@ -1386,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, get_checked_uint8(orderInfo->fieldFlags >> 7)); + update_write_brush(s, &patblt->brush, get_checked_uint8((orderInfo->fieldFlags >> 7) & 0x1F)); return TRUE; } @@ -1588,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, get_checked_uint8(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; @@ -1930,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, get_checked_uint8(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); @@ -1971,7 +1984,7 @@ static BOOL update_read_glyph_index_order(const char* orderName, wStream* s, !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, - get_checked_uint8(orderInfo->fieldFlags >> 14)) || + 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; @@ -2009,6 +2022,8 @@ 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, get_checked_uint8(glyph_index->cacheId)); @@ -2019,9 +2034,14 @@ BOOL update_write_glyph_index_order(wStream* s, ORDER_INFO* orderInfo, orderInfo->fieldFlags |= ORDER_FIELD_04; Stream_Write_UINT8(s, get_checked_uint8(glyph_index->fOpRedundant)); orderInfo->fieldFlags |= ORDER_FIELD_05; - update_write_color(s, get_checked_uint8(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_INT16(s, get_checked_int16(glyph_index->bkLeft)); orderInfo->fieldFlags |= ORDER_FIELD_08; @@ -2043,7 +2063,12 @@ BOOL update_write_glyph_index_order(wStream* s, ORDER_INFO* orderInfo, orderInfo->fieldFlags |= ORDER_FIELD_17; orderInfo->fieldFlags |= ORDER_FIELD_18; orderInfo->fieldFlags |= ORDER_FIELD_19; - update_write_brush(s, &glyph_index->brush, get_checked_uint8(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_INT16(s, get_checked_int16(glyph_index->x)); orderInfo->fieldFlags |= ORDER_FIELD_21; @@ -2219,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, get_checked_uint8(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)) @@ -2277,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, get_checked_uint8(orderInfo->fieldFlags >> 8))) + update_read_brush(s, &ellipse_cb->brush, + get_checked_uint8((orderInfo->fieldFlags >> 8) & 0x1F))) return TRUE; return FALSE; } From ae3160174f5e957c3a9081bd44260bac990da708 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Mon, 4 Nov 2024 10:10:40 +0100 Subject: [PATCH 16/18] [core,fastpath] fix bulk max size the type is UINT16 so the maximum size can only be UINT16_MAX --- libfreerdp/codec/bulk.c | 15 ++++++++------- libfreerdp/codec/bulk.h | 2 +- libfreerdp/core/fastpath.c | 5 ++--- 3 files changed, 11 insertions(+), 11 deletions(-) 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 8a7e61386..dbf7d0fc6 100644 --- a/libfreerdp/core/fastpath.c +++ b/libfreerdp/core/fastpath.c @@ -1189,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); - WINPR_ASSERT(CompressionMaxSize <= UINT16_MAX); - maxLength = (maxLength < CompressionMaxSize) ? maxLength : (UINT16)(CompressionMaxSize); + const UINT16 CompressionMaxSize = bulk_compression_max_size(rdp->bulk); + maxLength = (maxLength < CompressionMaxSize) ? maxLength : CompressionMaxSize; maxLength -= 20; } From cd1330be80febeac809481323414f031988f27c0 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 21 Nov 2024 09:43:25 +0100 Subject: [PATCH 17/18] [client,common] const correct variable declaration --- client/common/cmdline.c | 8 ++++---- client/common/file.c | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) 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) From fdba9bfc465060d9815e2981bb2ec37362a74be8 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Fri, 22 Nov 2024 10:51:47 +0100 Subject: [PATCH 18/18] [winpr,thread] improve unittest --- .../thread/test/TestThreadCommandLineToArgv.c | 79 +++++++++++++------ 1 file changed, 57 insertions(+), 22 deletions(-) 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; }