diff --git a/.github/workflows/wf-cliprdr-ci.yml b/.github/workflows/wf-cliprdr-ci.yml deleted file mode 100644 index bc65d22e8..000000000 --- a/.github/workflows/wf-cliprdr-ci.yml +++ /dev/null @@ -1,85 +0,0 @@ -name: wf-cliprdr CI - -on: - workflow_dispatch: - pull_request: - paths: - - "libs/clipboard/src/windows/**" - - "tests/test_invariant_wf_cliprdr.c" - - ".github/workflows/wf-cliprdr-ci.yml" - push: - branches: - - master - paths: - - "libs/clipboard/src/windows/**" - - "tests/test_invariant_wf_cliprdr.c" - - ".github/workflows/wf-cliprdr-ci.yml" - -permissions: - contents: read - -concurrency: - group: ${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: true - -jobs: - test: - name: wf_cliprdr invariant test - runs-on: windows-2022 - - steps: - - name: Checkout source code - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - with: - persist-credentials: false - - - name: Set up MSVC - uses: ilammy/msvc-dev-cmd@0b201ec74fa43914dc39ae48a89fd1d8cb592756 - with: - arch: x64 - - - name: Setup vcpkg with GitHub Actions binary cache - uses: lukka/run-vcpkg@b1a0dd252f06b9e25b3c022a9a03bd7a427fb6a2 # v11 - with: - vcpkgDirectory: C:\vcpkg - doNotCache: false - - - name: Install vcpkg dependency - shell: pwsh - run: | - & "$env:VCPKG_ROOT\vcpkg.exe" install check:x64-windows --classic --x-install-root="$env:VCPKG_ROOT\installed" - - - name: Build test - shell: pwsh - run: | - $testRoot = Join-Path $env:GITHUB_WORKSPACE 'build\wf-cliprdr' - New-Item -ItemType Directory -Force $testRoot | Out-Null - - $testSource = (($env:GITHUB_WORKSPACE -replace '\\', '/') + '/tests/test_invariant_wf_cliprdr.c') - $cmakeLists = @( - 'cmake_minimum_required(VERSION 3.20)' - 'project(test_invariant_wf_cliprdr C)' - '' - 'set(CMAKE_C_STANDARD 11)' - 'set(CMAKE_C_STANDARD_REQUIRED ON)' - 'set(CMAKE_C_EXTENSIONS OFF)' - '' - 'find_package(check CONFIG REQUIRED)' - '' - 'add_executable(test_invariant_wf_cliprdr' - ' "TEST_SOURCE"' - ')' - '' - 'target_link_libraries(test_invariant_wf_cliprdr PRIVATE' - ' $<$:Check::check>' - ' $<$>:Check::checkShared>' - ')' - ) -join [Environment]::NewLine - $cmakeLists.Replace('TEST_SOURCE', $testSource) | Set-Content -NoNewline (Join-Path $testRoot 'CMakeLists.txt') - - cmake -S $testRoot -B (Join-Path $testRoot 'out') -G "Visual Studio 17 2022" -A x64 -DCMAKE_TOOLCHAIN_FILE="$env:VCPKG_ROOT\scripts\buildsystems\vcpkg.cmake" -DVCPKG_TARGET_TRIPLET=x64-windows - cmake --build (Join-Path $testRoot 'out') --config Release - - - name: Run test - shell: pwsh - run: .\build\wf-cliprdr\out\Release\test_invariant_wf_cliprdr.exe diff --git a/libs/clipboard/src/windows/wf_cliprdr.c b/libs/clipboard/src/windows/wf_cliprdr.c index 5fd08deeb..95d1d1a5c 100644 --- a/libs/clipboard/src/windows/wf_cliprdr.c +++ b/libs/clipboard/src/windows/wf_cliprdr.c @@ -39,28 +39,6 @@ #define CLIPRDR_SVC_CHANNEL_NAME "cliprdr" -/* Maximum number of clipboard streams accepted from a remote peer (integer overflow / DoS guard) */ -#define WF_CLIPRDR_MAX_STREAMS 16384 - -/* Validates the remote descriptor array size after cItems has been read safely. */ -static BOOL wf_cliprdr_file_group_descriptor_size_valid(SIZE_T size, UINT count) -{ - SIZE_T header_size = offsetof(FILEGROUPDESCRIPTORW, fgd); - SIZE_T descriptors_size; - - if (count == 0 || count > WF_CLIPRDR_MAX_STREAMS) - return FALSE; - - if (size < header_size) - return FALSE; - - if ((SIZE_T)count > (((SIZE_T)-1) - header_size) / sizeof(FILEDESCRIPTORW)) - return FALSE; - - descriptors_size = header_size + (SIZE_T)count * sizeof(FILEDESCRIPTORW); - return size >= descriptors_size; -} - /** * Clipboard Formats */ @@ -246,7 +224,6 @@ struct wf_clipboard HWND hwnd; HANDLE hmem; - SIZE_T hmem_data_len; HANDLE thread; HANDLE formatDataRespEvent; BOOL formatDataRespReceived; @@ -652,50 +629,6 @@ void CliprdrStream_Delete(CliprdrStream *instance) } } -static void wf_cliprdr_release_streams(IStream **streams, ULONG count) -{ - ULONG i; - - if (!streams) - return; - - for (i = 0; i < count; i++) - { - if (streams[i]) - CliprdrStream_Release(streams[i]); - } - - free(streams); -} - -static void wf_cliprdr_reset_streams(CliprdrDataObject *instance) -{ - if (!instance) - return; - - wf_cliprdr_release_streams(instance->m_pStream, instance->m_nStreams); - instance->m_pStream = NULL; - instance->m_nStreams = 0; -} - -/* Only call after clipboard->hmem has been locked by GlobalLock. */ -static HRESULT wf_cliprdr_fail_locked_file_descriptor_data(wfClipboard *clipboard, - STGMEDIUM *medium, - CliprdrDataObject *instance, - IStream **streams, - ULONG stream_count, - HRESULT error) -{ - GlobalUnlock(clipboard->hmem); - GlobalFree(clipboard->hmem); - clipboard->hmem = NULL; - clipboard->hmem_data_len = 0; - medium->hGlobal = NULL; - wf_cliprdr_release_streams(streams, stream_count); - wf_cliprdr_reset_streams(instance); - return error; -} - /** * IDataObject */ @@ -814,9 +747,6 @@ static HRESULT STDMETHODCALLTYPE CliprdrDataObject_GetData(IDataObject *This, FO { // FILEGROUPDESCRIPTOR *dsc; FILEGROUPDESCRIPTORW *dsc; - IStream **streams = NULL; - UINT stream_count = 0; - SIZE_T hmem_size; // DWORD remote_format_id = get_remote_format_id(clipboard, instance->m_pFormatEtc[idx].cfFormat); // FIXME: origin code may be failed here??? if (cliprdr_send_data_request(instance->m_connID, clipboard, instance->m_pFormatEtc[idx].cfFormat) != 0) @@ -834,48 +764,40 @@ static HRESULT STDMETHODCALLTYPE CliprdrDataObject_GetData(IDataObject *This, FO * is the number of FILEDESCRIPTOR's */ // dsc = (FILEGROUPDESCRIPTOR *)GlobalLock(clipboard->hmem); dsc = (FILEGROUPDESCRIPTORW *)GlobalLock(clipboard->hmem); - if (!dsc) + instance->m_nStreams = dsc->cItems; + GlobalUnlock(clipboard->hmem); + + if (instance->m_nStreams > 0) { - pMedium->hGlobal = NULL; - GlobalFree(clipboard->hmem); - clipboard->hmem = NULL; - clipboard->hmem_data_len = 0; - wf_cliprdr_reset_streams(instance); - return E_UNEXPECTED; - } - - hmem_size = clipboard->hmem_data_len; - /* cItems is remote-controlled; verify the fixed header exists before reading it. */ - if (hmem_size < offsetof(FILEGROUPDESCRIPTORW, fgd)) - return wf_cliprdr_fail_locked_file_descriptor_data( - clipboard, pMedium, instance, NULL, 0, E_UNEXPECTED); - - stream_count = dsc->cItems; - if (!wf_cliprdr_file_group_descriptor_size_valid(hmem_size, stream_count)) - return wf_cliprdr_fail_locked_file_descriptor_data( - clipboard, pMedium, instance, NULL, 0, E_UNEXPECTED); - - streams = (IStream **)calloc(stream_count, sizeof(IStream *)); - if (!streams) - return wf_cliprdr_fail_locked_file_descriptor_data( - clipboard, pMedium, instance, NULL, 0, E_OUTOFMEMORY); - - for (i = 0; i < stream_count; i++) - { - streams[i] = - (IStream *)CliprdrStream_New(instance->m_connID, i, clipboard, &dsc->fgd[i]); - if (!streams[i]) + if (!instance->m_pStream) { - return wf_cliprdr_fail_locked_file_descriptor_data( - clipboard, pMedium, instance, streams, i, E_OUTOFMEMORY); + instance->m_pStream = (LPSTREAM *)calloc(instance->m_nStreams, sizeof(LPSTREAM)); + + if (instance->m_pStream) + { + for (i = 0; i < instance->m_nStreams; i++) + { + instance->m_pStream[i] = + (IStream *)CliprdrStream_New(instance->m_connID, i, clipboard, &dsc->fgd[i]); + + if (!instance->m_pStream[i]) + return E_OUTOFMEMORY; + } + } } } - GlobalUnlock(clipboard->hmem); - wf_cliprdr_reset_streams(instance); - instance->m_pStream = streams; - instance->m_nStreams = stream_count; - return S_OK; + if (!instance->m_pStream) + { + if (clipboard->hmem) + { + GlobalFree(clipboard->hmem); + clipboard->hmem = NULL; + } + + pMedium->hGlobal = NULL; + return E_OUTOFMEMORY; + } } else if (instance->m_pFormatEtc[idx].cfFormat == RegisterClipboardFormat(CFSTR_FILECONTENTS)) { @@ -2239,16 +2161,16 @@ static BOOL wf_cliprdr_add_to_file_arrays(wfClipboard *clipboard, WCHAR *full_fi return FALSE; /* add to name array */ + clipboard->file_names[clipboard->nFiles] = (LPWSTR)malloc((size_t)MAX_PATH * sizeof(WCHAR)); + + if (!clipboard->file_names[clipboard->nFiles]) + return FALSE; + // `MAX_PATH` is long enough for the file name. // So we just return FALSE if the file name is too long, which is not a normal case. if ((wcslen(full_file_name) + 1) > MAX_PATH) return FALSE; - clipboard->file_names[clipboard->nFiles] = (LPWSTR)calloc(MAX_PATH, sizeof(WCHAR)); - - if (!clipboard->file_names[clipboard->nFiles]) - return FALSE; - wcsncpy_s(clipboard->file_names[clipboard->nFiles], MAX_PATH, full_file_name, wcslen(full_file_name) + 1); /* add to descriptor array */ clipboard->fileDescriptor[clipboard->nFiles] = @@ -2856,7 +2778,6 @@ wf_cliprdr_server_format_data_response(CliprdrClientContext *context, break; } clipboard->hmem = NULL; - clipboard->hmem_data_len = 0; if (formatDataResponse->msgFlags != CB_RESPONSE_OK) { @@ -2890,7 +2811,6 @@ wf_cliprdr_server_format_data_response(CliprdrClientContext *context, break; } - clipboard->hmem_data_len = formatDataResponse->dataLen; clipboard->hmem = hMem; rc = CHANNEL_RC_OK; } while (0); diff --git a/tests/test_invariant_wf_cliprdr.c b/tests/test_invariant_wf_cliprdr.c deleted file mode 100644 index 3a75a5f3e..000000000 --- a/tests/test_invariant_wf_cliprdr.c +++ /dev/null @@ -1,92 +0,0 @@ -#include -#include -#include - -#include "../libs/clipboard/src/windows/wf_cliprdr.c" - -static SIZE_T descriptor_size(UINT count) -{ - return offsetof(FILEGROUPDESCRIPTORW, fgd) + (SIZE_T)count * sizeof(FILEDESCRIPTORW); -} - -START_TEST(test_descriptor_size_rejects_buffer_smaller_than_header) -{ - ck_assert_int_eq(wf_cliprdr_file_group_descriptor_size_valid(0, 1), FALSE); - ck_assert_int_eq(wf_cliprdr_file_group_descriptor_size_valid( - offsetof(FILEGROUPDESCRIPTORW, fgd) - 1, 1), - FALSE); -} -END_TEST - -START_TEST(test_descriptor_size_rejects_zero_items) -{ - ck_assert_int_eq( - wf_cliprdr_file_group_descriptor_size_valid(offsetof(FILEGROUPDESCRIPTORW, fgd), 0), - FALSE); -} -END_TEST - -START_TEST(test_descriptor_size_accepts_max_stream_count) -{ - ck_assert_int_eq(wf_cliprdr_file_group_descriptor_size_valid( - descriptor_size(WF_CLIPRDR_MAX_STREAMS), WF_CLIPRDR_MAX_STREAMS), - TRUE); -} -END_TEST - -START_TEST(test_descriptor_size_rejects_stream_count_above_limit) -{ - ck_assert_int_eq(wf_cliprdr_file_group_descriptor_size_valid( - descriptor_size(WF_CLIPRDR_MAX_STREAMS), WF_CLIPRDR_MAX_STREAMS + 1), - FALSE); -} -END_TEST - -START_TEST(test_descriptor_size_rejects_truncated_descriptor_array) -{ - ck_assert_int_eq(wf_cliprdr_file_group_descriptor_size_valid(descriptor_size(2) - 1, 2), - FALSE); -} -END_TEST - -START_TEST(test_descriptor_size_rejects_extreme_count) -{ - ck_assert_int_eq(wf_cliprdr_file_group_descriptor_size_valid((SIZE_T)-1, (UINT)-1), - FALSE); -} -END_TEST - -Suite *wf_cliprdr_invariant_suite(void) -{ - Suite *s; - TCase *tc_core; - - s = suite_create("wf_cliprdr_invariants"); - tc_core = tcase_create("descriptor_size"); - - tcase_add_test(tc_core, test_descriptor_size_rejects_buffer_smaller_than_header); - tcase_add_test(tc_core, test_descriptor_size_rejects_zero_items); - tcase_add_test(tc_core, test_descriptor_size_accepts_max_stream_count); - tcase_add_test(tc_core, test_descriptor_size_rejects_stream_count_above_limit); - tcase_add_test(tc_core, test_descriptor_size_rejects_truncated_descriptor_array); - tcase_add_test(tc_core, test_descriptor_size_rejects_extreme_count); - - suite_add_tcase(s, tc_core); - return s; -} - -int main(void) -{ - int number_failed; - Suite *s; - SRunner *sr; - - s = wf_cliprdr_invariant_suite(); - sr = srunner_create(s); - - srunner_run_all(sr, CK_NORMAL); - number_failed = srunner_ntests_failed(sr); - srunner_free(sr); - - return (number_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE; -}