From 74868217839a34ab8b5d43a6eda9b90f1ce13e4d Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Wed, 21 Oct 2026 14:40:44 -0400 Subject: [PATCH] fix(windows): prevent VHF report buffer overruns --- .../windows/driver/libvirtualhid_umdf.cpp | 37 +++++---- .../windows/shared/vhf_report_response.hpp | 49 ++++++++++++ tests/unit/test_windows_driver_protocol.cpp | 77 +++++++++++++++++++ 3 files changed, 148 insertions(+), 15 deletions(-) create mode 100644 src/platform/windows/shared/vhf_report_response.hpp diff --git a/src/platform/windows/driver/libvirtualhid_umdf.cpp b/src/platform/windows/driver/libvirtualhid_umdf.cpp index 70d1562a..91302f8a 100644 --- a/src/platform/windows/driver/libvirtualhid_umdf.cpp +++ b/src/platform/windows/driver/libvirtualhid_umdf.cpp @@ -60,6 +60,7 @@ #include "shared/switch_pro_protocol.hpp" #include "unique_win32_handle.hpp" #include "vhf_input_report_queue.hpp" +#include "vhf_report_response.hpp" #include "windows_device_identity.hpp" using VhfContext = PVOID; // NOSONAR(cpp:S5008): VHF callback ABI requires PVOID; client context narrows to DeviceRecord. @@ -905,6 +906,13 @@ namespace { return event; } + /** + * @brief Return a feature report without padding to VHF's advertised request length. + * + * @param record Device whose feature report is requested. + * @param packet VHF-owned destination and reported request length. + * @return Success, unsupported report, or insufficient reported buffer length. + */ NTSTATUS copy_vhf_feature_report(DeviceRecord &record, HID_XFER_PACKET &packet) { auto report_number = packet.reportId; if (report_number == 0U && packet.reportBufferLen > 0U) { @@ -924,15 +932,18 @@ namespace { if (!report.has_value()) { return STATUS_NOT_SUPPORTED; } - if (packet.reportBufferLen < report->size()) { - return STATUS_BUFFER_TOO_SMALL; - } - - std::fill_n(packet.reportBuffer, packet.reportBufferLen, UCHAR {}); - std::copy(report->begin(), report->end(), packet.reportBuffer); - return STATUS_SUCCESS; + return lvh::detail::windows::copy_vhf_report_response(packet.reportBuffer, packet.reportBufferLen, *report) ? + STATUS_SUCCESS : + STATUS_BUFFER_TOO_SMALL; } + /** + * @brief Return the latest input report, adding a zero ID when requested for an unnumbered report. + * + * @param record Device whose last input report is requested. + * @param packet VHF-owned destination; bytes beyond the returned report are untouched. + * @return Success, unsupported report, or insufficient reported buffer length. + */ NTSTATUS copy_vhf_input_report(DeviceRecord &record, HID_XFER_PACKET &packet) { auto report_number = packet.reportId; if ( @@ -953,14 +964,10 @@ namespace { } const auto unnumbered_report = report_number == 0U; - const auto leading_report_id_size = unnumbered_report && packet.reportBufferLen > report.size() ? 1U : 0U; - if (packet.reportBufferLen < report.size() + leading_report_id_size) { - return STATUS_BUFFER_TOO_SMALL; - } - - std::fill_n(packet.reportBuffer, packet.reportBufferLen, UCHAR {}); - std::copy(report.begin(), report.end(), packet.reportBuffer + leading_report_id_size); - return STATUS_SUCCESS; + const auto prepend_zero_id = unnumbered_report && packet.reportBufferLen > report.size(); + return lvh::detail::windows::copy_vhf_report_response(packet.reportBuffer, packet.reportBufferLen, report, prepend_zero_id) ? + STATUS_SUCCESS : + STATUS_BUFFER_TOO_SMALL; } bool handle_vhf_set_feature(DeviceRecord &record, const HID_XFER_PACKET &packet) { diff --git a/src/platform/windows/shared/vhf_report_response.hpp b/src/platform/windows/shared/vhf_report_response.hpp new file mode 100644 index 00000000..37b91d7d --- /dev/null +++ b/src/platform/windows/shared/vhf_report_response.hpp @@ -0,0 +1,49 @@ +// SPDX-FileCopyrightText: 2026 LIZARDBYTE LLC +// SPDX-License-Identifier: LicenseRef-LizardByte-SAL-1.0 + +/** + * @file src/platform/windows/shared/vhf_report_response.hpp + * @brief Copy HID responses into VHF-owned report buffers. + */ +#pragma once + +// standard includes +#include +#include +#include +#include + +namespace lvh::detail::windows { + + /** + * @brief Copy only the returned report bytes into a VHF callback buffer. + * + * Windows 10 VHF can advertise a reportBufferLen greater than its own + * allocation. Do not clear or pad to that advertised length. + * The response must fit the report descriptor used to create the device. + * + * @param buffer VHF-owned buffer with room for the response and optional ID byte. + * @param reported_length Length advertised by VHF, used to reject short requests. + * @param report Complete report bytes, including the ID for a numbered report. + * @param prepend_zero_id Whether to prepend the zero ID of an unnumbered report. + * @return True if the response fits the advertised length; false without writing otherwise. + */ + [[nodiscard]] inline bool copy_vhf_report_response( + std::uint8_t *buffer, + std::size_t reported_length, + std::span report, + bool prepend_zero_id = false + ) { + const auto leading_report_id_size = prepend_zero_id ? 1U : 0U; + if (report.size() > reported_length || leading_report_id_size > reported_length - report.size()) { + return false; + } + + if (prepend_zero_id) { + buffer[0] = 0; + } + std::ranges::copy(report, buffer + leading_report_id_size); + return true; + } + +} // namespace lvh::detail::windows diff --git a/tests/unit/test_windows_driver_protocol.cpp b/tests/unit/test_windows_driver_protocol.cpp index 50a72d89..42f9199c 100644 --- a/tests/unit/test_windows_driver_protocol.cpp +++ b/tests/unit/test_windows_driver_protocol.cpp @@ -7,6 +7,7 @@ #include "fixtures/fixtures.hpp" #include "playstation_feature_protocol.hpp" #include "shared/switch_pro_protocol.hpp" +#include "vhf_report_response.hpp" #include "windows_device_identity.hpp" // standard includes @@ -82,6 +83,82 @@ namespace { } // namespace +TEST_F(WindowsDriverProtocolTest, VhfFeatureResponseDoesNotWriteToAdvertisedPadding) { + const auto request = playstation_request(LVH_WINDOWS_GAMEPAD_DUALSENSE); + for (const auto report_id : {0x05U, 0x09U, 0x20U}) { + SCOPED_TRACE(report_id); + const auto report = lvh::detail::windows::make_playstation_feature_report(request, report_id); + ASSERT_TRUE(report.has_value()); + ASSERT_LE(report->size(), 64U); + + // VHF allocates 64 bytes but advertises 128 in issue #199's PageHeap dump. + // Extra backing storage lets the test detect the overrun without invoking UB. + auto buffer = std::array {}; + buffer.fill(0xA5); + ASSERT_TRUE(lvh::detail::windows::copy_vhf_report_response(buffer.data(), 128U, *report)); + EXPECT_TRUE(std::ranges::equal(*report, std::span {buffer}.first(report->size()))); + EXPECT_TRUE(std::ranges::all_of(std::span {buffer}.subspan(report->size()), [](auto byte) { + return byte == 0xA5; + })); + } +} + +TEST_F(WindowsDriverProtocolTest, VhfInputResponseDoesNotWritePastTheReport) { + auto report = std::array {}; + report.fill(0x5A); + report[0] = 0x01; + auto buffer = std::array {}; + buffer.fill(0xA5); + + ASSERT_TRUE(lvh::detail::windows::copy_vhf_report_response(buffer.data(), buffer.size(), report)); + EXPECT_TRUE(std::ranges::equal(report, std::span {buffer}.first(report.size()))); + EXPECT_TRUE(std::ranges::all_of(std::span {buffer}.subspan(report.size()), [](auto byte) { + return byte == 0xA5; + })); +} + +TEST_F(WindowsDriverProtocolTest, VhfUnnumberedResponseWritesOnlyTheOptionalIdAndReport) { + const auto report = std::array {1, 2, 3, 4, 5, 6, 7, 8}; + for (const auto prepend_zero_id : {false, true}) { + SCOPED_TRACE(prepend_zero_id); + const auto prefix_size = prepend_zero_id ? 1U : 0U; + auto buffer = std::array {}; + buffer.fill(0xA5); + + ASSERT_TRUE(lvh::detail::windows::copy_vhf_report_response(buffer.data(), buffer.size(), report, prepend_zero_id)); + if (prepend_zero_id) { + EXPECT_EQ(buffer[0], 0U); + } + EXPECT_TRUE(std::ranges::equal(report, std::span {buffer}.subspan(prefix_size, report.size()))); + EXPECT_TRUE(std::ranges::all_of(std::span {buffer}.subspan(prefix_size + report.size()), [](auto byte) { + return byte == 0xA5; + })); + } +} + +TEST_F(WindowsDriverProtocolTest, VhfResponseRejectsShortRequestsWithoutWriting) { + const auto report = std::array {1, 2, 3, 4, 5, 6, 7, 8}; + auto buffer = std::array {}; + buffer.fill(0xA5); + EXPECT_FALSE(lvh::detail::windows::copy_vhf_report_response(buffer.data(), report.size() - 1U, report)); + EXPECT_FALSE(lvh::detail::windows::copy_vhf_report_response(buffer.data(), report.size(), report, true)); + EXPECT_TRUE(std::ranges::all_of(buffer, [](auto byte) { + return byte == 0xA5; + })); +} + +TEST_F(WindowsDriverProtocolTest, VhfResponseAcceptsExactlySizedRequests) { + const auto report = std::array {1, 2, 3}; + auto numbered_buffer = std::array {}; + ASSERT_TRUE(lvh::detail::windows::copy_vhf_report_response(numbered_buffer.data(), numbered_buffer.size(), report)); + EXPECT_EQ(numbered_buffer, report); + + auto unnumbered_buffer = std::array {}; + unnumbered_buffer.fill(0xA5); + ASSERT_TRUE(lvh::detail::windows::copy_vhf_report_response(unnumbered_buffer.data(), unnumbered_buffer.size(), report, true)); + EXPECT_EQ(unnumbered_buffer, (std::array {0, 1, 2, 3})); +} + TEST_F(WindowsDriverProtocolTest, SeriesUsesCapturedXInputHidIdentity) { auto request = series_request(); request.hardware_ids.product_id = 0x0B12;