From e2a7635f2f7632c188420fa23e93e8cab0e80e65 Mon Sep 17 00:00:00 2001 From: Joey Yakimowich-Payne Date: Tue, 11 Aug 2026 12:51:36 +0900 Subject: [PATCH] Harden legacy bounds Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- switch_pro_bounds.h | 71 +++++++++++ switch_pro_driver.cpp | 131 +++++++++++++-------- tests/firmware/test_legacy_descriptors.cpp | 75 ++++++++++++ 3 files changed, 228 insertions(+), 49 deletions(-) create mode 100644 switch_pro_bounds.h diff --git a/switch_pro_bounds.h b/switch_pro_bounds.h new file mode 100644 index 0000000..183fb36 --- /dev/null +++ b/switch_pro_bounds.h @@ -0,0 +1,71 @@ +#pragma once + +#include +#include + +enum class SwitchProOutputReportKind : uint8_t { + Ignore, + Noop, + Rumble, + Feature, + Configuration, +}; + +inline SwitchProOutputReportKind switch_pro_classify_output_report( + const uint8_t* report, + std::size_t length) { + if (report == nullptr || length < 2 || length > 64) { + return SwitchProOutputReportKind::Ignore; + } + + switch (report[0]) { + case 0x00: + return SwitchProOutputReportKind::Noop; + case 0x01: + return length >= 16 + ? SwitchProOutputReportKind::Feature + : SwitchProOutputReportKind::Ignore; + case 0x10: + case 0x21: + return length >= 10 + ? SwitchProOutputReportKind::Rumble + : SwitchProOutputReportKind::Ignore; + case 0x80: + return SwitchProOutputReportKind::Configuration; + default: + return SwitchProOutputReportKind::Ignore; + } +} + +inline bool switch_pro_spi_read_size_fits(std::size_t size) { + return size <= 64 - 20; +} + +inline std::size_t switch_pro_fill_flash_read( + uint8_t* destination, + std::size_t destination_capacity, + const uint8_t* source, + std::size_t source_size, + std::size_t source_offset, + std::size_t requested) { + if (destination == nullptr) { + return 0; + } + + const std::size_t produced = requested < destination_capacity + ? requested + : destination_capacity; + for (std::size_t index = 0; index < produced; ++index) { + destination[index] = 0xFF; + } + + if (source != nullptr && source_offset < source_size) { + const std::size_t available = source_size - source_offset; + const std::size_t copied = available < produced ? available : produced; + for (std::size_t index = 0; index < copied; ++index) { + destination[index] = source[source_offset + index]; + } + } + + return produced; +} diff --git a/switch_pro_driver.cpp b/switch_pro_driver.cpp index 57580a9..a199e67 100644 --- a/switch_pro_driver.cpp +++ b/switch_pro_driver.cpp @@ -1,4 +1,5 @@ #include "switch_pro_driver.h" +#include "switch_pro_bounds.h" #include #include @@ -178,9 +179,14 @@ static const uint8_t user_calibration_data[0x3F] = { static const SwitchFactoryConfig* factory_config = reinterpret_cast(factory_config_data); static const SwitchUserCalibration* user_calibration [[maybe_unused]] = reinterpret_cast(user_calibration_data); -static std::map spi_flash_data = { - {0x6000, factory_config_data}, - {0x8000, user_calibration_data} +struct SpiFlashRegion { + const uint8_t* data; + std::size_t size; +}; + +static const std::map spi_flash_data = { + {0x6000, {factory_config_data, sizeof(factory_config_data)}}, + {0x8000, {user_calibration_data, sizeof(user_calibration_data)}} }; static inline uint16_t scale16To12(uint16_t pos) { return pos >> 4; } @@ -235,16 +241,32 @@ static bool send_report(uint8_t reportID, const void* reportData, uint16_t repor return result; } -static void read_spi_flash(uint8_t* dest, uint32_t address, uint8_t size) { +static void read_spi_flash( + uint8_t* destination, + std::size_t destination_capacity, + uint32_t address, + uint8_t size) { uint32_t addressBank = address & 0xFFFFFF00; uint32_t addressOffset = address & 0x000000FF; auto it = spi_flash_data.find(addressBank); if (it != spi_flash_data.end()) { - const uint8_t* data = it->second; - memcpy(dest, data + addressOffset, size); + const SpiFlashRegion& region = it->second; + switch_pro_fill_flash_read( + destination, + destination_capacity, + region.data, + region.size, + addressOffset, + size); } else { - memset(dest, 0xFF, size); + switch_pro_fill_flash_read( + destination, + destination_capacity, + nullptr, + 0, + 0, + size); } } @@ -371,7 +393,11 @@ static void handle_feature_report(uint8_t switchReportID, uint8_t switchReportSu report_buffer[17] = reportData[13]; report_buffer[18] = reportData[14]; report_buffer[19] = reportData[15]; - read_spi_flash(&report_buffer[20], spiReadAddress, spiReadSize); + read_spi_flash( + &report_buffer[20], + sizeof(report_buffer) - 20, + spiReadAddress, + spiReadSize); canSend = true; LOG_PRINTF("[HID] FEATURE SPI_READ addr=0x%08lx size=%u\n", (unsigned long)spiReadAddress, spiReadSize); break; @@ -642,6 +668,52 @@ bool switch_pro_is_ready() { return is_ready; } +static void dispatch_output_report( + uint8_t instance, + uint8_t report_id, + const uint8_t* buffer, + uint16_t length) { + if (instance != 0) { + return; + } + + const SwitchProOutputReportKind kind = + switch_pro_classify_output_report(buffer, length); + if (kind == SwitchProOutputReportKind::Ignore) { + return; + } + if (kind == SwitchProOutputReportKind::Feature && + buffer[10] == SPI_READ && + !switch_pro_spi_read_size_fits(buffer[15])) { + return; + } + + memset(report_buffer, 0x00, sizeof(report_buffer)); + + const uint8_t switchReportID = buffer[0]; + const uint8_t switchReportSubID = buffer[1]; + LOG_PRINTF("[HID] output_report id=%u switchRID=0x%02x sub=0x%02x len=%u\n", + report_id, switchReportID, switchReportSubID, length); + + switch (kind) { + case SwitchProOutputReportKind::Noop: + return; + case SwitchProOutputReportKind::Rumble: + forward_rumble_to_host(buffer, length); + return; + case SwitchProOutputReportKind::Feature: + queued_report_id = report_id; + handle_feature_report(switchReportID, switchReportSubID, buffer, length); + return; + case SwitchProOutputReportKind::Configuration: + queued_report_id = report_id; + handle_config_report(switchReportID, switchReportSubID, buffer, length); + return; + case SwitchProOutputReportKind::Ignore: + return; + } +} + // HID callbacks uint16_t tud_hid_get_report_cb(uint8_t instance, uint8_t report_id, hid_report_type_t report_type, uint8_t *buffer, uint16_t reqlen) { (void)instance; @@ -656,51 +728,12 @@ uint16_t tud_hid_get_report_cb(uint8_t instance, uint8_t report_id, hid_report_t } void tud_hid_set_report_cb(uint8_t instance, uint8_t report_id, hid_report_type_t report_type, uint8_t const *buffer, uint16_t bufsize) { - (void)instance; if (report_type != HID_REPORT_TYPE_OUTPUT) return; - - memset(report_buffer, 0x00, bufsize); - - uint8_t switchReportID = buffer[0]; - uint8_t switchReportSubID = buffer[1]; - LOG_PRINTF("[HID] set_report type=%d id=%u switchRID=0x%02x sub=0x%02x len=%u\n", - report_type, report_id, switchReportID, switchReportSubID, bufsize); - if (switchReportID == REPORT_OUTPUT_10 || switchReportID == REPORT_OUTPUT_21) { - forward_rumble_to_host(buffer, bufsize); - } - if (switchReportID == REPORT_OUTPUT_00) { - // No-op, just acknowledge to clear any stalls. - return; - } else if (switchReportID == REPORT_FEATURE) { - queued_report_id = report_id; - handle_feature_report(switchReportID, switchReportSubID, buffer, bufsize); - } else if (switchReportID == REPORT_CONFIGURATION) { - queued_report_id = report_id; - handle_config_report(switchReportID, switchReportSubID, buffer, bufsize); - } else { - } + dispatch_output_report(instance, report_id, buffer, bufsize); } void tud_hid_report_received_cb(uint8_t instance, uint8_t report_id, uint8_t const* buffer, uint16_t bufsize) { - (void)instance; - // Host sent data on interrupt OUT; mirror the control path handling. - memset(report_buffer, 0x00, bufsize); - uint8_t switchReportID = buffer[0]; - uint8_t switchReportSubID = buffer[1]; - LOG_PRINTF("[HID] report_received id=%u switchRID=0x%02x sub=0x%02x len=%u\n", - report_id, switchReportID, switchReportSubID, bufsize); - if (switchReportID == REPORT_OUTPUT_10 || switchReportID == REPORT_OUTPUT_21) { - forward_rumble_to_host(buffer, bufsize); - } - if (switchReportID == REPORT_OUTPUT_00) { - return; - } else if (switchReportID == REPORT_FEATURE) { - queued_report_id = report_id; - handle_feature_report(switchReportID, switchReportSubID, buffer, bufsize); - } else if (switchReportID == REPORT_CONFIGURATION) { - queued_report_id = report_id; - handle_config_report(switchReportID, switchReportSubID, buffer, bufsize); - } + dispatch_output_report(instance, report_id, buffer, bufsize); } uint8_t const * tud_hid_descriptor_report_cb(uint8_t itf) { diff --git a/tests/firmware/test_legacy_descriptors.cpp b/tests/firmware/test_legacy_descriptors.cpp index e66bd77..47c0064 100644 --- a/tests/firmware/test_legacy_descriptors.cpp +++ b/tests/firmware/test_legacy_descriptors.cpp @@ -5,6 +5,7 @@ #include #include "../../switch_pro_descriptors.h" +#include "../../switch_pro_bounds.h" namespace { @@ -88,6 +89,73 @@ bool legacy_string_descriptors_match_exact_bytes() { return true; } +bool legacy_output_classifier_rejects_invalid_report_framing() { + const uint8_t report[] = {0x01, 0x00}; + CHECK(switch_pro_classify_output_report(nullptr, 2) == SwitchProOutputReportKind::Ignore); + CHECK(switch_pro_classify_output_report(report, 0) == SwitchProOutputReportKind::Ignore); + CHECK(switch_pro_classify_output_report(report, 1) == SwitchProOutputReportKind::Ignore); + CHECK(switch_pro_classify_output_report(report, 65) == SwitchProOutputReportKind::Ignore); + return true; +} + +bool legacy_feature_reports_reject_short_payloads_and_accept_bounds() { + const uint8_t report[] = {0x01, 0x00}; + CHECK(switch_pro_classify_output_report(report, 15) == SwitchProOutputReportKind::Ignore); + CHECK(switch_pro_classify_output_report(report, 16) == SwitchProOutputReportKind::Feature); + CHECK(switch_pro_classify_output_report(report, 64) == SwitchProOutputReportKind::Feature); + return true; +} + +bool legacy_configuration_and_rumble_reports_reject_short_payloads() { + const uint8_t configuration[] = {0x80, 0x00}; + const uint8_t rumble[] = {0x10, 0x00}; + const uint8_t noop[] = {0x00, 0x00}; + CHECK(switch_pro_classify_output_report(configuration, 1) == SwitchProOutputReportKind::Ignore); + CHECK(switch_pro_classify_output_report(configuration, 2) == SwitchProOutputReportKind::Configuration); + CHECK(switch_pro_classify_output_report(rumble, 9) == SwitchProOutputReportKind::Ignore); + CHECK(switch_pro_classify_output_report(rumble, 10) == SwitchProOutputReportKind::Rumble); + CHECK(switch_pro_classify_output_report(noop, 2) == SwitchProOutputReportKind::Noop); + return true; +} + +bool legacy_spi_read_rejects_payload_overflow_at_forty_five_bytes() { + CHECK(switch_pro_spi_read_size_fits(0)); + CHECK(switch_pro_spi_read_size_fits(44)); + CHECK(!switch_pro_spi_read_size_fits(45)); + CHECK(!switch_pro_spi_read_size_fits(255)); + return true; +} + +bool legacy_flash_read_copies_in_range_data_through_exact_end() { + const uint8_t source[] = {1, 2, 3, 4}; + uint8_t destination[] = {0xAA, 0xAA, 0xAA, 0xAA}; + CHECK(switch_pro_fill_flash_read(destination, 4, source, 4, 0, 4) == 4); + CHECK(destination[0] == 1 && destination[3] == 4); + return true; +} + +bool legacy_flash_read_prefills_partial_source_end_with_ff() { + const uint8_t source[] = {1, 2}; + uint8_t destination[] = {0xAA, 0xAA, 0xAA, 0xAA}; + CHECK(switch_pro_fill_flash_read(destination, 4, source, 2, 0, 4) == 4); + CHECK(destination[0] == 1 && destination[1] == 2); + CHECK(destination[2] == 0xFF && destination[3] == 0xFF); + return true; +} + +bool legacy_flash_read_leaves_canaries_on_invalid_range_or_null_source() { + const uint8_t source[] = {1, 2}; + uint8_t destination[] = {0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA}; + CHECK(switch_pro_fill_flash_read(destination + 1, 4, source, 2, 3, 4) == 4); + CHECK(destination[0] == 0xAA && destination[5] == 0xAA); + CHECK(destination[1] == 0xFF && destination[4] == 0xFF); + CHECK(switch_pro_fill_flash_read(destination + 1, 4, nullptr, 2, 0, 4) == 4); + CHECK(destination[0] == 0xAA && destination[5] == 0xAA); + CHECK(destination[1] == 0xFF && destination[4] == 0xFF); + CHECK(switch_pro_fill_flash_read(nullptr, 4, source, 2, 0, 4) == 0); + return true; +} + } // namespace void run_legacy_descriptor_tests(TestRunner& runner) { @@ -95,4 +163,11 @@ void run_legacy_descriptor_tests(TestRunner& runner) { runner.run("legacy configuration descriptor exact bytes", legacy_configuration_descriptor_matches_exact_bytes); runner.run("legacy HID report descriptor exact bytes", legacy_hid_report_descriptor_matches_exact_bytes); runner.run("legacy string descriptors exact bytes", legacy_string_descriptors_match_exact_bytes); + runner.run("legacy output classifier rejects invalid report framing", legacy_output_classifier_rejects_invalid_report_framing); + runner.run("legacy feature reports reject short payloads and accept bounds", legacy_feature_reports_reject_short_payloads_and_accept_bounds); + runner.run("legacy configuration and rumble reports reject short payloads", legacy_configuration_and_rumble_reports_reject_short_payloads); + runner.run("legacy SPI read rejects payload overflow at 45 bytes", legacy_spi_read_rejects_payload_overflow_at_forty_five_bytes); + runner.run("legacy flash read copies in-range data through exact end", legacy_flash_read_copies_in_range_data_through_exact_end); + runner.run("legacy flash read prefills partial source end with FF", legacy_flash_read_prefills_partial_source_end_with_ff); + runner.run("legacy flash read leaves canaries on invalid range or null source", legacy_flash_read_leaves_canaries_on_invalid_range_or_null_source); }