diff --git a/.github/workflows/wf-cliprdr-ci.yml b/.github/workflows/wf-cliprdr-ci.yml new file mode 100644 index 000000000..bc65d22e8 --- /dev/null +++ b/.github/workflows/wf-cliprdr-ci.yml @@ -0,0 +1,85 @@ +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 95d1d1a5c..5fd08deeb 100644 --- a/libs/clipboard/src/windows/wf_cliprdr.c +++ b/libs/clipboard/src/windows/wf_cliprdr.c @@ -39,6 +39,28 @@ #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 */ @@ -224,6 +246,7 @@ struct wf_clipboard HWND hwnd; HANDLE hmem; + SIZE_T hmem_data_len; HANDLE thread; HANDLE formatDataRespEvent; BOOL formatDataRespReceived; @@ -629,6 +652,50 @@ 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 */ @@ -747,6 +814,9 @@ 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) @@ -764,40 +834,48 @@ static HRESULT STDMETHODCALLTYPE CliprdrDataObject_GetData(IDataObject *This, FO * is the number of FILEDESCRIPTOR's */ // dsc = (FILEGROUPDESCRIPTOR *)GlobalLock(clipboard->hmem); dsc = (FILEGROUPDESCRIPTORW *)GlobalLock(clipboard->hmem); - instance->m_nStreams = dsc->cItems; - GlobalUnlock(clipboard->hmem); - - if (instance->m_nStreams > 0) + if (!dsc) { - if (!instance->m_pStream) - { - 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; - } - } - } - } - - if (!instance->m_pStream) - { - if (clipboard->hmem) - { - GlobalFree(clipboard->hmem); - clipboard->hmem = NULL; - } - pMedium->hGlobal = NULL; - return E_OUTOFMEMORY; + 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]) + { + return wf_cliprdr_fail_locked_file_descriptor_data( + clipboard, pMedium, instance, streams, i, E_OUTOFMEMORY); + } + } + + GlobalUnlock(clipboard->hmem); + wf_cliprdr_reset_streams(instance); + instance->m_pStream = streams; + instance->m_nStreams = stream_count; + return S_OK; } else if (instance->m_pFormatEtc[idx].cfFormat == RegisterClipboardFormat(CFSTR_FILECONTENTS)) { @@ -2161,16 +2239,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] = @@ -2778,6 +2856,7 @@ wf_cliprdr_server_format_data_response(CliprdrClientContext *context, break; } clipboard->hmem = NULL; + clipboard->hmem_data_len = 0; if (formatDataResponse->msgFlags != CB_RESPONSE_OK) { @@ -2811,6 +2890,7 @@ 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 new file mode 100644 index 000000000..3a75a5f3e --- /dev/null +++ b/tests/test_invariant_wf_cliprdr.c @@ -0,0 +1,92 @@ +#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; +}