Merge pull request #12411 from akallabeth/multifrag-add-checks

Address various error handling inconsistencies
This commit is contained in:
akallabeth
2026-03-03 10:31:09 +01:00
committed by GitHub
24 changed files with 312 additions and 295 deletions

View File

@@ -149,6 +149,9 @@ UINT rail_read_handshake_order(wStream* s, RAIL_HANDSHAKE_ORDER* handshake)
void rail_write_handshake_order(wStream* s, const RAIL_HANDSHAKE_ORDER* handshake)
{
WINPR_ASSERT(s);
WINPR_ASSERT(handshake);
WINPR_ASSERT(Stream_EnsureRemainingCapacity(s, 4));
Stream_Write_UINT32(s, handshake->buildNumber); /* buildNumber (4 bytes) */
}
@@ -169,6 +172,10 @@ UINT rail_read_handshake_ex_order(wStream* s, RAIL_HANDSHAKE_EX_ORDER* handshake
void rail_write_handshake_ex_order(wStream* s, const RAIL_HANDSHAKE_EX_ORDER* handshakeEx)
{
WINPR_ASSERT(s);
WINPR_ASSERT(handshakeEx);
WINPR_ASSERT(Stream_EnsureRemainingCapacity(s, 8));
Stream_Write_UINT32(s, handshakeEx->buildNumber); /* buildNumber (4 bytes) */
Stream_Write_UINT32(s, handshakeEx->railHandshakeFlags); /* railHandshakeFlags (4 bytes) */
}
@@ -471,11 +478,6 @@ UINT rail_read_sysparam_order(wStream* s, RAIL_SYSPARAM_ORDER* sysparam, BOOL ex
return error;
}
/**
* Function description
*
* @return 0 on success, otherwise a Win32 err2or code
*/
UINT rail_write_sysparam_order(wStream* s, const RAIL_SYSPARAM_ORDER* sysparam,
BOOL extendedSpiSupported)
{

File diff suppressed because it is too large Load Diff

View File

@@ -70,7 +70,8 @@ static BOOL delete_surface(const void* key, void* value, void* arg)
static void free_surfaces(RdpgfxClientContext* context, wHashTable* SurfaceTable)
{
HashTable_Foreach(SurfaceTable, delete_surface, context);
if (!HashTable_Foreach(SurfaceTable, delete_surface, context))
WLog_WARN(TAG, "delete_surface failed");
}
static UINT evict_cache_slots(RdpgfxClientContext* context, UINT16 MaxCacheSlots, void** CacheSlots)

View File

@@ -267,7 +267,7 @@ static void LIBUSB_CALL func_iso_callback(struct libusb_transfer* transfer)
BYTE* dataStart = Stream_Pointer(user_data->data);
if (!Stream_SetPosition(user_data->data,
40)) /* TS_URB_ISOCH_TRANSFER_RESULT IsoPacket offset */
return;
break;
for (uint32_t i = 0; i < WINPR_ASSERTING_INT_CAST(uint32_t, transfer->num_iso_packets);
i++)

View File

@@ -723,7 +723,8 @@ static UINT urbdrc_udevman_parse_addin_args(UDEVMAN* udevman, const ADDIN_ARGV*
const char* arg = args->argv[x];
if (strcmp(arg, "dbg") == 0)
{
WLog_SetLogLevel(WLog_Get(TAG), WLOG_TRACE);
if (!WLog_SetLogLevel(WLog_Get(TAG), WLOG_TRACE))
return ERROR_INTERNAL_ERROR;
}
else if (_strnicmp(arg, "device:", 7) == 0)
{

View File

@@ -890,7 +890,8 @@ static UINT urbdrc_process_addin_args(URBDRC_PLUGIN* urbdrc, const ADDIN_ARGV* a
CommandLineSwitchStart(arg) CommandLineSwitchCase(arg, "dbg")
{
WLog_SetLogLevel(urbdrc->log, WLOG_TRACE);
if (!WLog_SetLogLevel(urbdrc->log, WLOG_TRACE))
return ERROR_INTERNAL_ERROR;
}
CommandLineSwitchCase(arg, "sys")
{

View File

@@ -108,7 +108,8 @@ int main(int argc, char* argv[])
thread = freerdp_client_get_thread(context);
(void)WaitForSingleObject(thread, INFINITE);
GetExitCodeThread(thread, &dwExitCode);
if (!GetExitCodeThread(thread, &dwExitCode))
goto out;
rc = xf_exit_code_from_disconnect_reason(dwExitCode);
freerdp_client_stop(context);

View File

@@ -754,7 +754,11 @@ xfWindow* xf_CreateDesktopWindow(xfContext* xfc, char* name, int width, int heig
LogDynAndXClearWindow(xfc->log, xfc->display, window->handle);
xf_SetWindowTitleText(xfc, window->handle, name);
LogDynAndXMapWindow(xfc->log, xfc->display, window->handle);
xf_input_init(xfc, window->handle);
if (!xf_input_init(xfc, window->handle))
{
xf_DestroyDesktopWindow(xfc, window);
return nullptr;
}
/*
* NOTE: This must be done here to handle reparenting the window,

View File

@@ -549,15 +549,20 @@ static BOOL clear_clip_data_entries(WINPR_ATTR_UNUSED const void* key, void* val
return TRUE;
}
static void clear_cdi_entries(CliprdrFileContext* file_context)
WINPR_ATTR_NODISCARD
static UINT clear_cdi_entries(CliprdrFileContext* file_context)
{
UINT res = CHANNEL_RC_OK;
WINPR_ASSERT(file_context);
HashTable_Lock(file_context->inode_table);
HashTable_Foreach(file_context->clip_data_table, clear_clip_data_entries, nullptr);
if (!HashTable_Foreach(file_context->clip_data_table, clear_clip_data_entries, nullptr))
res = ERROR_INTERNAL_ERROR;
HashTable_Clear(file_context->clip_data_table);
HashTable_Unlock(file_context->inode_table);
return res;
}
static UINT prepare_clip_data_entry_with_id(CliprdrFileContext* file_context)
@@ -616,7 +621,7 @@ UINT cliprdr_file_context_notify_new_server_format_list(CliprdrFileContext* file
#if defined(WITH_FUSE)
clear_no_cdi_entry(file_context);
/* TODO: assign timeouts to old locks instead */
clear_cdi_entries(file_context);
rc = clear_cdi_entries(file_context);
if (does_server_support_clipdata_locking(file_context))
rc = prepare_clip_data_entry_with_id(file_context);
@@ -634,7 +639,7 @@ UINT cliprdr_file_context_notify_new_client_format_list(CliprdrFileContext* file
#if defined(WITH_FUSE)
clear_no_cdi_entry(file_context);
/* TODO: assign timeouts to old locks instead */
clear_cdi_entries(file_context);
return clear_cdi_entries(file_context);
#endif
return CHANNEL_RC_OK;

View File

@@ -5606,7 +5606,6 @@ static int freerdp_client_settings_parse_command_line_arguments_int(
if (!isArgsFrom)
warn_credential_args(largs);
CommandLineFindArgumentA(largs, "v");
arg = largs;
errno = 0;

View File

@@ -145,7 +145,6 @@ extern "C"
FREERDP_API void rail_server_context_free(RailServerContext* context);
WINPR_ATTR_MALLOC(rail_server_context_free, 1)
WINPR_ATTR_NODISCARD
FREERDP_API RailServerContext* rail_server_context_new(HANDLE vcm);
WINPR_ATTR_NODISCARD

View File

@@ -1952,6 +1952,9 @@ static inline RFX_MESSAGE* rfx_split_message(RFX_CONTEXT* WINPR_RESTRICT context
WINPR_ASSERT(message);
WINPR_ASSERT(numMessages);
if (maxDataSize <= 1024)
return nullptr;
maxDataSize -= 1024; /* reserve enough space for headers */
*numMessages = ((message->tilesDataSize + maxDataSize) / maxDataSize) * 4ull;

View File

@@ -2706,12 +2706,11 @@ static BOOL rdp_print_desktop_composition_capability_set(wLog* log, wStream* s)
static BOOL rdp_apply_multifragment_update_capability_set(rdpSettings* settings,
const rdpSettings* src)
{
UINT32 multifragMaxRequestSize = 0;
WINPR_ASSERT(settings);
WINPR_ASSERT(src);
multifragMaxRequestSize = src->MultifragMaxRequestSize;
UINT32 multifragMaxRequestSize =
freerdp_settings_get_uint32(src, FreeRDP_MultifragMaxRequestSize);
if (settings->ServerMode)
{
@@ -2733,14 +2732,17 @@ static BOOL rdp_apply_multifragment_update_capability_set(rdpSettings* settings,
* than or equal to the value we've previously sent in the server to
* client multi-fragment update capability set (MS-RDPRFX 1.5)
*/
if (multifragMaxRequestSize < settings->MultifragMaxRequestSize)
if (multifragMaxRequestSize <
freerdp_settings_get_uint32(settings, FreeRDP_MultifragMaxRequestSize))
{
/*
* If it happens to be smaller we honor the client's value but
* have to disable RemoteFX
*/
settings->RemoteFxCodec = FALSE;
settings->MultifragMaxRequestSize = multifragMaxRequestSize;
if (!freerdp_settings_set_uint32(settings, FreeRDP_MultifragMaxRequestSize,
multifragMaxRequestSize))
return FALSE;
}
else
{
@@ -2749,7 +2751,9 @@ static BOOL rdp_apply_multifragment_update_capability_set(rdpSettings* settings,
}
else
{
settings->MultifragMaxRequestSize = multifragMaxRequestSize;
if (!freerdp_settings_set_uint32(settings, FreeRDP_MultifragMaxRequestSize,
multifragMaxRequestSize))
return FALSE;
}
}
else
@@ -2759,8 +2763,13 @@ static BOOL rdp_apply_multifragment_update_capability_set(rdpSettings* settings,
* In RemoteFX mode we MUST do this but it might also be useful to
* receive larger related bitmap updates.
*/
if (multifragMaxRequestSize > settings->MultifragMaxRequestSize)
settings->MultifragMaxRequestSize = multifragMaxRequestSize;
if (multifragMaxRequestSize >
freerdp_settings_get_uint32(settings, FreeRDP_MultifragMaxRequestSize))
{
if (!freerdp_settings_set_uint32(settings, FreeRDP_MultifragMaxRequestSize,
multifragMaxRequestSize))
return FALSE;
}
}
return TRUE;
}
@@ -2773,16 +2782,13 @@ static BOOL rdp_apply_multifragment_update_capability_set(rdpSettings* settings,
static BOOL rdp_read_multifragment_update_capability_set(wLog* log, wStream* s,
rdpSettings* settings)
{
UINT32 multifragMaxRequestSize = 0;
WINPR_ASSERT(settings);
if (!Stream_CheckAndLogRequiredLengthWLog(log, s, 4))
return FALSE;
Stream_Read_UINT32(s, multifragMaxRequestSize); /* MaxRequestSize (4 bytes) */
settings->MultifragMaxRequestSize = multifragMaxRequestSize;
return TRUE;
const UINT32 multifragMaxRequestSize = Stream_Get_UINT32(s); /* MaxRequestSize (4 bytes) */
return freerdp_settings_set_uint32(settings, FreeRDP_MultifragMaxRequestSize,
multifragMaxRequestSize);
}
/*
@@ -2794,7 +2800,8 @@ static BOOL rdp_write_multifragment_update_capability_set(wLog* log, wStream* s,
rdpSettings* settings)
{
WINPR_ASSERT(settings);
if (settings->ServerMode && settings->MultifragMaxRequestSize == 0)
if (settings->ServerMode &&
(freerdp_settings_get_uint32(settings, FreeRDP_MultifragMaxRequestSize) == 0))
{
/*
* In server mode we prefer to use the highest useful request size that
@@ -2807,11 +2814,19 @@ static BOOL rdp_write_multifragment_update_capability_set(wLog* log, wStream* s,
* greater than or equal to the value we're sending here.
* See [MS-RDPRFX 1.5 capability #2]
*/
UINT32 tileNumX = (settings->DesktopWidth + 63) / 64;
UINT32 tileNumY = (settings->DesktopHeight + 63) / 64;
settings->MultifragMaxRequestSize = tileNumX * tileNumY * 16384;
const UINT32 tileNumX = (settings->DesktopWidth + 63) / 64;
const UINT32 tileNumY = (settings->DesktopHeight + 63) / 64;
WINPR_ASSERT(tileNumX < UINT32_MAX / tileNumY);
WINPR_ASSERT(tileNumY < UINT32_MAX / tileNumX);
WINPR_ASSERT(tileNumX * tileNumY < UINT32_MAX / 16384u);
/* and add room for headers, regions, frame markers, etc. */
settings->MultifragMaxRequestSize += 16384;
const UINT32 MultifragMaxRequestSize = (tileNumX * tileNumY + 1u) * 16384u;
if (!freerdp_settings_set_uint32(settings, FreeRDP_MultifragMaxRequestSize,
MultifragMaxRequestSize))
return FALSE;
}
if (!Stream_EnsureRemainingCapacity(s, 32))
@@ -4791,7 +4806,9 @@ BOOL rdp_recv_confirm_active(rdpRdp* rdp, wStream* s, UINT16 pduLength)
if (!settings->ReceivedCapabilities[CAPSET_TYPE_MULTI_FRAGMENT_UPDATE])
{
/* client does not support multi fragment updates - make sure packages are not fragmented */
settings->MultifragMaxRequestSize = FASTPATH_FRAGMENT_SAFE_SIZE;
if (!freerdp_settings_set_uint32(settings, FreeRDP_MultifragMaxRequestSize,
FASTPATH_FRAGMENT_SAFE_SIZE))
return FALSE;
}
if (!settings->ReceivedCapabilities[CAPSET_TYPE_LARGE_POINTER])

View File

@@ -574,10 +574,13 @@ static int fastpath_recv_update_data(rdpFastPath* fastpath, wStream* s)
WINPR_ASSERT(context);
WINPR_ASSERT(context->settings);
if (totalSize > context->settings->MultifragMaxRequestSize)
if (totalSize >
freerdp_settings_get_uint32(context->settings, FreeRDP_MultifragMaxRequestSize))
{
WLog_ERR(TAG, "Total size (%" PRIuz ") exceeds MultifragMaxRequestSize (%" PRIu32 ")",
totalSize, context->settings->MultifragMaxRequestSize);
WLog_ERR(
TAG, "Total size (%" PRIuz ") exceeds MultifragMaxRequestSize (%" PRIu32 ")",
totalSize,
freerdp_settings_get_uint32(context->settings, FreeRDP_MultifragMaxRequestSize));
goto out_fail;
}
@@ -1211,12 +1214,13 @@ BOOL fastpath_send_update_pdu(rdpFastPath* fastpath, BYTE updateCode, wStream* s
}
/* check if the client's fast path pdu buffer is large enough */
if (totalLength > settings->MultifragMaxRequestSize)
if (totalLength > freerdp_settings_get_uint32(settings, FreeRDP_MultifragMaxRequestSize))
{
WLog_ERR(TAG,
"fast path update size (%" PRIuz
") exceeds the client's maximum request size (%" PRIu32 ")",
totalLength, settings->MultifragMaxRequestSize);
totalLength,
freerdp_settings_get_uint32(settings, FreeRDP_MultifragMaxRequestSize));
return FALSE;
}

View File

@@ -663,9 +663,10 @@ static BOOL vgids_read_do_fkt(void* data, size_t index, va_list ap)
return TRUE;
}
static void vgids_read_do(vgidsContext* context, UINT16 efID, UINT16 doID)
WINPR_ATTR_NODISCARD
static BOOL vgids_read_do(vgidsContext* context, UINT16 efID, UINT16 doID)
{
ArrayList_ForEach(context->files, vgids_read_do_fkt, context, efID, doID);
return ArrayList_ForEach(context->files, vgids_read_do_fkt, context, efID, doID);
}
static void vgids_reset_context_response(vgidsContext* context)
@@ -897,7 +898,8 @@ static BOOL vgids_ins_getdata(vgidsContext* context, wStream* s, BYTE** response
}
Stream_Read_UINT16_BE(s, doId);
vgids_read_do(context, fileId, doId);
if (!vgids_read_do(context, fileId, doId))
return FALSE;
break;
}
case 0xA:

View File

@@ -151,12 +151,15 @@ static BOOL start_irp_thread(pf_channel_client_context* scard,
work = CreateThreadpoolWork(irp_thread, arg, nullptr);
if (!work)
goto fail;
ArrayList_Append(scard->workObjects, work);
if (!ArrayList_Append(scard->workObjects, work))
goto fail;
SubmitThreadpoolWork(work);
return TRUE;
fail:
if (work)
CloseThreadpoolWork(work);
if (arg)
queue_free(arg->e);
free(arg);

View File

@@ -483,7 +483,7 @@ static BOOL pf_modules_print_ArrayList_ForEachFkt(void* data, size_t index, va_l
return TRUE;
}
void pf_modules_list_loaded_plugins(proxyModule* module)
BOOL pf_modules_list_loaded_plugins(proxyModule* module)
{
size_t count = 0;
@@ -495,7 +495,7 @@ void pf_modules_list_loaded_plugins(proxyModule* module)
if (count > 0)
WLog_INFO(TAG, "Loaded plugins:");
ArrayList_ForEach(module->plugins, pf_modules_print_ArrayList_ForEachFkt);
return ArrayList_ForEach(module->plugins, pf_modules_print_ArrayList_ForEachFkt);
}
WINPR_ATTR_NODISCARD

View File

@@ -970,7 +970,9 @@ proxyServer* pf_server_new(const proxyConfig* config)
goto out;
}
pf_modules_list_loaded_plugins(server->module);
if (!pf_modules_list_loaded_plugins(server->module))
goto out;
if (!are_all_required_modules_loaded(server->module, server->config))
goto out;

View File

@@ -90,7 +90,7 @@ extern "C"
WINPR_ATTR_NODISCARD BOOL pf_modules_is_plugin_loaded(proxyModule* module,
const char* plugin_name);
void pf_modules_list_loaded_plugins(proxyModule* module);
WINPR_ATTR_NODISCARD BOOL pf_modules_list_loaded_plugins(proxyModule* module);
WINPR_ATTR_NODISCARD BOOL pf_modules_run_filter(proxyModule* module, PF_FILTER_TYPE type,
proxyData* pdata, void* param);

View File

@@ -822,7 +822,6 @@ extern "C"
WINPR_API void MessagePipe_Free(wMessagePipe* pipe);
WINPR_ATTR_MALLOC(MessagePipe_Free, 1)
WINPR_ATTR_NODISCARD
WINPR_API wMessagePipe* MessagePipe_New(void);
/* Publisher/Subscriber Pattern */

View File

@@ -653,7 +653,10 @@ static void* clipboard_synthesize_html_format(wClipboard* clipboard, UINT32 form
/* Check the BOM (Byte Order Mark) */
if ((pSrcData.cpb[0] == 0xFE) && (pSrcData.cpb[1] == 0xFF))
ByteSwapUnicode(pSrcData.pv, (SrcSize / 2));
{
if (!ByteSwapUnicode(pSrcData.pv, (SrcSize / 2)))
goto fail;
}
/* Check if we have WCHAR, convert to UTF-8 */
if ((pSrcData.cpb[0] == 0xFF) && (pSrcData.cpb[1] == 0xFE))

View File

@@ -104,7 +104,8 @@ static struct synthetic_file* make_synthetic_file(const WCHAR* local_name, const
{
const size_t len = _wcslen(file->remote_name);
PathCchConvertStyleW(file->remote_name, len, PATH_STYLE_WINDOWS);
if (S_OK != PathCchConvertStyleW(file->remote_name, len, PATH_STYLE_WINDOWS))
goto fail;
}
file->dwFileAttributes = fd.dwFileAttributes;

View File

@@ -391,8 +391,12 @@ static SECURITY_STATUS SEC_ENTRY ntlm_AcquireCredentialsHandleW(
{
UINT32 identityFlags = sspi_GetAuthIdentityFlags(pAuthData);
sspi_CopyAuthIdentity(&(credentials->identity),
(const SEC_WINNT_AUTH_IDENTITY_INFO*)pAuthData);
if (sspi_CopyAuthIdentity(&(credentials->identity),
(const SEC_WINNT_AUTH_IDENTITY_INFO*)pAuthData) < 0)
{
sspi_CredentialsFree(credentials);
return SEC_E_INVALID_PARAMETER;
}
if (identityFlags & SEC_WINNT_AUTH_IDENTITY_EXTENDED)
settings = (((SEC_WINNT_AUTH_IDENTITY_WINPR*)pAuthData)->ntlmSettings);
@@ -1233,7 +1237,14 @@ static SECURITY_STATUS SEC_ENTRY ntlm_DecryptMessage(PCtxtHandle phContext, PSec
/* Decrypt message using with RC4, result overwrites original buffer */
if (context->confidentiality)
winpr_RC4_Update(context->RecvRc4Seal, length, (BYTE*)data, (BYTE*)data_buffer->pvBuffer);
{
if (!winpr_RC4_Update(context->RecvRc4Seal, length, (BYTE*)data,
(BYTE*)data_buffer->pvBuffer))
{
free(data);
return SEC_E_INSUFFICIENT_MEMORY;
}
}
else
CopyMemory(data_buffer->pvBuffer, data, length);

View File

@@ -18,6 +18,7 @@
*/
#include <winpr/config.h>
#include <winpr/assert.h>
#include <winpr/crt.h>
#include <winpr/sysinfo.h>
@@ -34,6 +35,7 @@
void MessagePipe_PostQuit(wMessagePipe* pipe, int nExitCode)
{
WINPR_ASSERT(pipe);
MessageQueue_PostQuit(pipe->In, nExitCode);
MessageQueue_PostQuit(pipe->Out, nExitCode);
}
@@ -44,27 +46,23 @@ void MessagePipe_PostQuit(wMessagePipe* pipe, int nExitCode)
wMessagePipe* MessagePipe_New(void)
{
wMessagePipe* pipe = nullptr;
pipe = (wMessagePipe*)malloc(sizeof(wMessagePipe));
wMessagePipe* pipe = (wMessagePipe*)calloc(1, sizeof(wMessagePipe));
if (!pipe)
return nullptr;
pipe->In = MessageQueue_New(nullptr);
if (!pipe->In)
goto error_in;
goto fail;
pipe->Out = MessageQueue_New(nullptr);
if (!pipe->Out)
goto error_out;
goto fail;
return pipe;
error_out:
MessageQueue_Free(pipe->In);
error_in:
free(pipe);
fail:
MessagePipe_Free(pipe);
return nullptr;
}