From 1a46b450b185c252ebf7578dffab96ad8d6ddc72 Mon Sep 17 00:00:00 2001 From: rustdesk Date: Tue, 16 Jun 2026 17:38:38 +0800 Subject: [PATCH] try to fix https://github.com/rustdesk/rustdesk/issues/15291 --- libs/clipboard/src/windows/wf_cliprdr.c | 21 ++++-- tests/test_invariant_wf_cliprdr.c | 88 +++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 4 deletions(-) diff --git a/libs/clipboard/src/windows/wf_cliprdr.c b/libs/clipboard/src/windows/wf_cliprdr.c index 5fd08deeb..d17312aae 100644 --- a/libs/clipboard/src/windows/wf_cliprdr.c +++ b/libs/clipboard/src/windows/wf_cliprdr.c @@ -1406,10 +1406,10 @@ static UINT32 get_remote_format_id(wfClipboard *clipboard, UINT32 local_format) return local_format; } -static void map_ensure_capacity(wfClipboard *clipboard) +static BOOL map_ensure_capacity(wfClipboard *clipboard) { if (!clipboard) - return; + return FALSE; if (clipboard->map_size >= clipboard->map_capacity) { @@ -1420,11 +1420,20 @@ static void map_ensure_capacity(wfClipboard *clipboard) (formatMapping *)realloc(clipboard->format_mappings, sizeof(formatMapping) * new_size); if (!new_map) - return; + return FALSE; + + // `realloc` does not initialize the grown region. `clear_format_map()` frees + // `map->name` for every slot up to `map_capacity`, so the newly added slots + // must be zeroed; otherwise `free()` is called on uninitialized (garbage) + // pointers, which corrupts the heap (issue #15291). + ZeroMemory(new_map + clipboard->map_capacity, + (new_size - clipboard->map_capacity) * sizeof(formatMapping)); clipboard->format_mappings = new_map; clipboard->map_capacity = new_size; } + + return TRUE; } static BOOL clear_format_map(wfClipboard *clipboard) @@ -2448,6 +2457,11 @@ static UINT wf_cliprdr_server_format_list(CliprdrClientContext *context, for (i = 0; i < formatList->numFormats; i++) { + // Ensure room for slot `i` (== current map_size) BEFORE writing it, and bail + // out if the table cannot be grown, so we never write past the allocation. + if (!map_ensure_capacity(clipboard)) + return ERROR_INTERNAL_ERROR; + format = &(formatList->formats[i]); mapping = &(clipboard->format_mappings[i]); mapping->remote_format_id = format->formatId; @@ -2472,7 +2486,6 @@ static UINT wf_cliprdr_server_format_list(CliprdrClientContext *context, } clipboard->map_size++; - map_ensure_capacity(clipboard); } if (file_transferring(clipboard)) diff --git a/tests/test_invariant_wf_cliprdr.c b/tests/test_invariant_wf_cliprdr.c index 3a75a5f3e..7426ee87d 100644 --- a/tests/test_invariant_wf_cliprdr.c +++ b/tests/test_invariant_wf_cliprdr.c @@ -1,6 +1,7 @@ #include #include #include +#include #include "../libs/clipboard/src/windows/wf_cliprdr.c" @@ -56,10 +57,91 @@ START_TEST(test_descriptor_size_rejects_extreme_count) } END_TEST +/* Regression tests for issue #15291: map_ensure_capacity() must zero-initialize + * the region added by realloc(), because clear_format_map() frees map->name for + * every slot up to map_capacity. Without zeroing, those slots hold uninitialized + * (garbage) pointers and free() corrupts the heap. */ + +START_TEST(test_map_ensure_capacity_zeroes_grown_slots) +{ + wfClipboard cb; + size_t i; + size_t old_capacity; + + /* Dirty the heap so that a non-zeroing realloc would hand back non-NULL + * garbage for the grown region, making the regression deterministic. */ + for (i = 0; i < 64; i++) + { + void *p = malloc(256 * sizeof(formatMapping)); + if (p) + { + memset(p, 0xFF, 256 * sizeof(formatMapping)); + free(p); + } + } + + memset(&cb, 0, sizeof(cb)); + cb.map_capacity = 2; + cb.map_size = 0; + cb.format_mappings = (formatMapping *)calloc(cb.map_capacity, sizeof(formatMapping)); + ck_assert_ptr_nonnull(cb.format_mappings); + + old_capacity = cb.map_capacity; + cb.map_size = cb.map_capacity; /* force a grow on the next ensure */ + ck_assert_int_eq(map_ensure_capacity(&cb), TRUE); + ck_assert_uint_gt(cb.map_capacity, old_capacity); + + for (i = old_capacity; i < cb.map_capacity; i++) + { + ck_assert_ptr_null(cb.format_mappings[i].name); + ck_assert_uint_eq(cb.format_mappings[i].remote_format_id, 0); + ck_assert_uint_eq(cb.format_mappings[i].local_format_id, 0); + } + + clear_format_map(&cb); + free(cb.format_mappings); +} +END_TEST + +START_TEST(test_clear_format_map_safe_after_realloc_growth) +{ + wfClipboard cb; + UINT i; + + memset(&cb, 0, sizeof(cb)); + cb.map_capacity = 2; + cb.map_size = 0; + cb.format_mappings = (formatMapping *)calloc(cb.map_capacity, sizeof(formatMapping)); + ck_assert_ptr_nonnull(cb.format_mappings); + + /* Populate a format list larger than the initial capacity, the same way + * wf_cliprdr_server_format_list() does: ensure capacity, then write slot i. */ + for (i = 0; i < 10; i++) + { + ck_assert_int_eq(map_ensure_capacity(&cb), TRUE); + cb.format_mappings[i].remote_format_id = i + 1; + cb.format_mappings[i].local_format_id = i + 1; + cb.format_mappings[i].name = _wcsdup(L"fmt"); + ck_assert_ptr_nonnull(cb.format_mappings[i].name); + cb.map_size++; + } + + ck_assert_uint_ge(cb.map_capacity, 10); + + /* clear_format_map() frees name across the full capacity; the untouched grown + * slots must be NULL so this neither crashes nor corrupts the heap. */ + ck_assert_int_eq(clear_format_map(&cb), TRUE); + ck_assert_uint_eq(cb.map_size, 0); + + free(cb.format_mappings); +} +END_TEST + Suite *wf_cliprdr_invariant_suite(void) { Suite *s; TCase *tc_core; + TCase *tc_format_map; s = suite_create("wf_cliprdr_invariants"); tc_core = tcase_create("descriptor_size"); @@ -72,6 +154,12 @@ Suite *wf_cliprdr_invariant_suite(void) tcase_add_test(tc_core, test_descriptor_size_rejects_extreme_count); suite_add_tcase(s, tc_core); + + tc_format_map = tcase_create("format_map"); + tcase_add_test(tc_format_map, test_map_ensure_capacity_zeroes_grown_slots); + tcase_add_test(tc_format_map, test_clear_format_map_safe_after_realloc_growth); + suite_add_tcase(s, tc_format_map); + return s; }