From b61774b2d5f76ce181606e0100d12eed9ab41e0b Mon Sep 17 00:00:00 2001 From: simon-wh Date: Tue, 29 Oct 2024 16:42:17 +0100 Subject: [PATCH 1/2] Ensure valid DeviceType when converting DeviceInfo_FFI -> DeviceInfo --- wooting-analog-common/src/lib.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/wooting-analog-common/src/lib.rs b/wooting-analog-common/src/lib.rs index a599e92..afaed14 100644 --- a/wooting-analog-common/src/lib.rs +++ b/wooting-analog-common/src/lib.rs @@ -84,6 +84,15 @@ impl Drop for DeviceInfo_FFI { impl DeviceInfo_FFI { pub fn into_device_info(&self) -> DeviceInfo { + let raw = self.device_type.clone() as i32; + let device_type = DeviceType::from_i32(raw); + if device_type.is_none() { + log::error!( + "Invalid Device Type when converting DeviceInfo_FFI into DeviceInfo: {}", + raw + ); + } + DeviceInfo { vendor_id: self.vendor_id.clone(), product_id: self.product_id.clone(), @@ -102,7 +111,7 @@ impl DeviceInfo_FFI { .to_owned() }, device_id: self.device_id.clone(), - device_type: self.device_type.clone(), + device_type: device_type.unwrap_or(DeviceType::Other), } } } From e852ea0b9c0c6a6f3ea24998607327cd7e6bf60e Mon Sep 17 00:00:00 2001 From: simon-wh Date: Wed, 30 Oct 2024 11:43:01 +0100 Subject: [PATCH 2/2] Use c_int on Rust side of DeviceInfo_FFI.device_type so we can properly bounds check it to prevent undefined behaviour --- includes/wooting-analog-common.h | 4 ++-- wooting-analog-common/cbindgen.toml | 12 ++++++++++-- wooting-analog-common/src/lib.rs | 17 +++++++++++------ 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/includes/wooting-analog-common.h b/includes/wooting-analog-common.h index a7da57d..6c80970 100644 --- a/includes/wooting-analog-common.h +++ b/includes/wooting-analog-common.h @@ -130,7 +130,7 @@ typedef struct WootingAnalog_DeviceInfo_FFI { */ WootingAnalog_DeviceID device_id; /** - * Hardware type of the Device + * Hardware type of the Device see `DeviceType` enum */ - enum WootingAnalog_DeviceType device_type; + WootingAnalog_DeviceType device_type; } WootingAnalog_DeviceInfo_FFI; diff --git a/wooting-analog-common/cbindgen.toml b/wooting-analog-common/cbindgen.toml index 6a12dbf..6679abb 100644 --- a/wooting-analog-common/cbindgen.toml +++ b/wooting-analog-common/cbindgen.toml @@ -4,7 +4,14 @@ header = "/* This is a generated header file providing the common items to every autogen_warning = "/* Warning, this file is autogenerated by cbindgen. Don't modify this manually. */" [export] -include = ["DeviceInfoBlank", "DeviceInfo_FFI", "DeviceEventType", "WootingAnalogResult", "KeycodeType"] +include = [ + "DeviceInfoBlank", + "DeviceInfo_FFI", + "DeviceEventType", + "WootingAnalogResult", + "KeycodeType", + "DeviceType", +] prefix = "WootingAnalog_" renaming_overrides_prefixing = true item_types = ["enums", "structs", "typedefs", "functions", "opaque"] @@ -12,9 +19,10 @@ item_types = ["enums", "structs", "typedefs", "functions", "opaque"] [export.rename] "FfiStr" = "const char*" "WootingAnalogResult" = "WootingAnalogResult" +"DeviceType_FFI" = "WootingAnalog_DeviceType" [parse] expand = ["wooting-analog-common"] [enum] -prefix_with_name=true +prefix_with_name = true diff --git a/wooting-analog-common/src/lib.rs b/wooting-analog-common/src/lib.rs index afaed14..91e669f 100644 --- a/wooting-analog-common/src/lib.rs +++ b/wooting-analog-common/src/lib.rs @@ -39,6 +39,12 @@ pub struct DeviceInfo { pub device_type: DeviceType, } +// We do this little alias so that we can force cbindgen to rename it to point to the DeviceType enum. +// For the rust side, we want to have it as a c_int so we can ensure it's valid and within bounds. +// As just taking it as the enum straight up can cause undefined behaviour if an invalid value is provided +/// cbindgen:ignore +type DeviceType_FFI = c_int; + /// The core `DeviceInfo` struct which contains all the interesting information /// for a particular device. This is the version which the consumer of the SDK will receive /// through the wrapper. This is not for use in the Internal workings of the SDK, that is what @@ -55,8 +61,8 @@ pub struct DeviceInfo_FFI { pub device_name: *mut c_char, /// Unique device ID, which should be generated using `generate_device_id` pub device_id: DeviceID, - /// Hardware type of the Device - pub device_type: DeviceType, + /// Hardware type of the Device see `DeviceType` enum + pub device_type: DeviceType_FFI, } impl From for DeviceInfo_FFI { @@ -67,7 +73,7 @@ impl From for DeviceInfo_FFI { manufacturer_name: CString::new(device.manufacturer_name).unwrap().into_raw(), device_name: CString::new(device.device_name).unwrap().into_raw(), device_id: device.device_id, - device_type: device.device_type, + device_type: device.device_type as c_int, } } } @@ -84,12 +90,11 @@ impl Drop for DeviceInfo_FFI { impl DeviceInfo_FFI { pub fn into_device_info(&self) -> DeviceInfo { - let raw = self.device_type.clone() as i32; - let device_type = DeviceType::from_i32(raw); + let device_type = DeviceType::from_i32(self.device_type); if device_type.is_none() { log::error!( "Invalid Device Type when converting DeviceInfo_FFI into DeviceInfo: {}", - raw + self.device_type ); }