fix: add integer overflow check in wf_cliprdr.c (#15142)

* fix: V-003 security vulnerability

Automated security fix generated by OrbisAI Security

Signed-off-by: orbisai0security <mediratta01.pally@gmail.com>

* fix: add integer overflow check in wf_cliprdr.c

At line 774, memory is allocated using calloc with instance->m_nStreams as the count parameter

Signed-off-by: orbisai0security <mediratta01.pally@gmail.com>

* Apply code changes: @orbisai0security can you address code review comm...

* fix(cliprdr): ci

Signed-off-by: fufesou <linlong1266@gmail.com>

* fix(cliprdr): ci, use msvc

Signed-off-by: fufesou <linlong1266@gmail.com>

* fix(cliprdr): ci

Signed-off-by: fufesou <linlong1266@gmail.com>

* fix(cliprdr): ci, test

Signed-off-by: fufesou <linlong1266@gmail.com>

* fix(cliprdr): fix ci

Signed-off-by: fufesou <linlong1266@gmail.com>

* fix(cliprdr): fix ci

Signed-off-by: fufesou <linlong1266@gmail.com>

* fix(cliprdr): fix ci

Signed-off-by: fufesou <linlong1266@gmail.com>

* fix(cliprdr): ci

Signed-off-by: fufesou <linlong1266@gmail.com>

* Apply code changes: @orbisai0security can you address code review comm...

* adding bounds check and tests

* Apply code changes: @orbisai0security can you address code review comm...

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

---------

Signed-off-by: orbisai0security <mediratta01.pally@gmail.com>
Signed-off-by: fufesou <linlong1266@gmail.com>
Co-authored-by: fufesou <linlong1266@gmail.com>
Co-authored-by: RustDesk <71636191+rustdesk@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
OrbisAI Security
2026-06-01 12:56:39 +05:30
committed by GitHub
parent 5eed50961d
commit fabeae4180
3 changed files with 293 additions and 36 deletions

85
.github/workflows/wf-cliprdr-ci.yml vendored Normal file
View File

@@ -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'
' $<$<TARGET_EXISTS:Check::check>:Check::check>'
' $<$<NOT:$<TARGET_EXISTS: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

View File

@@ -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);

View File

@@ -0,0 +1,92 @@
#include <check.h>
#include <stdlib.h>
#include <stddef.h>
#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;
}