From 7e616536efa7933efa5f3f6b25a996deef8a4d60 Mon Sep 17 00:00:00 2001 From: David Rebbe Date: Mon, 9 Dec 2024 07:33:49 -0500 Subject: [PATCH] fixed unsafe warnings and added icsneo_get_bus_type_name --- api/icsneo/icsneo.cpp | 64 +++++++++++++++++++++++++++++++---------- include/icsneo/icsneo.h | 12 +++++++- 2 files changed, 60 insertions(+), 16 deletions(-) diff --git a/api/icsneo/icsneo.cpp b/api/icsneo/icsneo.cpp index 65ff836..cbf6d31 100644 --- a/api/icsneo/icsneo.cpp +++ b/api/icsneo/icsneo.cpp @@ -55,6 +55,37 @@ typedef struct icsneo_event_t { static std::vector> g_devices; static std::vector g_events; +/** + * Safely copies a std::string to a char array. + * + * @param dest The buffer to copy the string into + * @param dest_size The size of the buffer + * @param src The string to copy + * + * @return true if the string was successfully copied, false otherwise + * + * @note This function always null terminates the buffer, even if the string is too long. + * This is done for security reasons, not performance. + */ +bool safe_str_copy(const char* dest, uint32_t dest_size, std::string src) { + // zero terminate the entire string, this is done for security not for performance + memset(const_cast(dest), 0, dest_size); + // Need to save room for the null terminator + dest_size -= 1; + try { + auto copied = src.copy(const_cast(dest), static_cast(dest_size), 0); + // Somehow we didn't copy the number of bytes we needed to? This check probably isn't needed. + if (copied != dest_size) { + return false; + } + return true; + } catch (std::out_of_range& ex) { + // if pos > size() + (void)ex; + return false; + } +} + ICSNEO_API icsneo_error_t icsneo_get_error_code(icsneo_error_t error_code, const char* value, uint32_t* value_length) { if (!value || !value_length) { return icsneo_error_invalid_parameters; @@ -95,9 +126,7 @@ ICSNEO_API icsneo_error_t icsneo_get_error_code(icsneo_error_t error_code, const auto min_length = std::minmax(static_cast(error.length()), *value_length).first; *value_length = min_length; // Copy the string into value - strncpy(const_cast(value), error.c_str(), min_length); - - return icsneo_error_success; + return safe_str_copy(value, min_length, error) ? icsneo_error_success : icsneo_error_invalid_parameters; } ICSNEO_API icsneo_error_t icsneo_device_type_from_type(icsneo_devicetype_t device_type, const char* value, uint32_t* value_length) { @@ -110,12 +139,11 @@ ICSNEO_API icsneo_error_t icsneo_device_type_from_type(icsneo_devicetype_t devic auto min_length = std::minmax(static_cast(device_type_str.length()), *value_length).first; *value_length = min_length; // Copy the string into value - strncpy(const_cast(value), device_type_str.c_str(), min_length); - - return icsneo_error_success; + return safe_str_copy(value, min_length, device_type_str) ? icsneo_error_success : icsneo_error_invalid_parameters; } ICSNEO_API icsneo_error_t icsneo_device_find_all(icsneo_device_t** devices, uint32_t* devices_count, void* reserved) { + (void)reserved; if (!devices || !devices_count) { return icsneo_error_invalid_parameters; } @@ -235,9 +263,7 @@ ICSNEO_API icsneo_error_t icsneo_device_get_description(icsneo_device_t* device, auto min_length = std::minmax(static_cast(dev->describe().length()), *value_length).first; *value_length = min_length; // Copy the string into value - strncpy(const_cast(value), dev->describe().c_str(), min_length); - - return icsneo_error_success; + return safe_str_copy(value, min_length, dev->describe()) ? icsneo_error_success : icsneo_error_invalid_parameters; } ICSNEO_API icsneo_error_t icsneo_device_get_type(icsneo_device_t* device, icsneo_devicetype_t* value) { @@ -259,9 +285,7 @@ ICSNEO_API icsneo_error_t icsneo_device_get_serial(icsneo_device_t* device, cons auto min_length = std::minmax(static_cast(dev->getSerial().length()), *value_length).first; *value_length = min_length; // Copy the string into value - strncpy(const_cast(value), dev->getSerial().c_str(), min_length); - - return icsneo_error_success; + return safe_str_copy(value, min_length, dev->getSerial()) ? icsneo_error_success : icsneo_error_invalid_parameters; } @@ -472,6 +496,18 @@ ICSNEO_API icsneo_error_t icsneo_message_get_bus_type(icsneo_device_t* device, i return icsneo_error_success; } +ICSNEO_API icsneo_error_t icsneo_get_bus_type_name(icsneo_msg_bus_type_t* bus_type, const char* value, uint32_t* value_length) { + if (!bus_type || !value || !value_length) { + return icsneo_error_invalid_parameters; + } + auto bus_type_str = std::string(Network::GetTypeString(*bus_type)); + // Find the minimum length of the bus type string and set value_length + auto min_length = std::minmax(static_cast(bus_type_str.length()), *value_length).first; + *value_length = min_length; + // Copy the string into value + return safe_str_copy(value, min_length, bus_type_str) ? icsneo_error_success : icsneo_error_invalid_parameters; +} + ICSNEO_API icsneo_error_t icsneo_message_get_data(icsneo_device_t* device, icsneo_message_t* message, uint8_t* data, uint32_t* data_length) { if (!device || !message || !data || !data_length) { return icsneo_error_invalid_parameters; @@ -694,9 +730,7 @@ ICSNEO_API icsneo_error_t icsneo_event_get_description(icsneo_event_t* event, co auto min_length = std::minmax(static_cast(ev.describe().length()), *value_length).first; *value_length = min_length; // Copy the string into value - strncpy(const_cast(value), ev.describe().c_str(), min_length); - - return icsneo_error_success; + return safe_str_copy(value, min_length, ev.describe()) ? icsneo_error_success : icsneo_error_invalid_parameters; } ICSNEO_API icsneo_error_t icsneo_device_get_rtc(icsneo_device_t* device, int64_t* unix_epoch) { diff --git a/include/icsneo/icsneo.h b/include/icsneo/icsneo.h index fbdddfa..237c56d 100644 --- a/include/icsneo/icsneo.h +++ b/include/icsneo/icsneo.h @@ -349,10 +349,20 @@ ICSNEO_API icsneo_error_t icsneo_message_get_type(icsneo_device_t* device, icsne * * @return icsneo_error_t icsneo_error_success if successful, icsneo_error_invalid_parameters or icsneo_error_invalid_type otherwise. * - * @see icsneo_msg_bus_type_t + * @see icsneo_msg_bus_type_t, icsneo_get_bus_type_name */ ICSNEO_API icsneo_error_t icsneo_message_get_bus_type(icsneo_device_t* device, icsneo_message_t* message, icsneo_msg_bus_type_t* bus_type); +/** @brief Get the bus type string for a icsneo_msg_bus_type_t. + * + * @param[in] icsneo_event_t* event The event to get the description of. + * @param[out] const char* value Pointer to a buffer to copy the description into. Null terminated. + * @param[in,out] uint32_t* value_length Size of the value buffer. Modified with the length of the description. + * + * @return icsneo_error_t icsneo_error_success if successful, icsneo_error_invalid_parameters otherwise. + */ +ICSNEO_API icsneo_error_t icsneo_get_bus_type_name(icsneo_msg_bus_type_t* bus_type, const char* value, uint32_t* value_length); + /** @brief Get the data bytes of a message * * @param[in] icsneo_device_t* device The device to check against.