From 1c8e11b2cb74656d0dcde1ba5bec8fabb8b0461b Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Fri, 14 Aug 2020 23:38:58 -0400 Subject: [PATCH] bug in char get_value; raise NotImpl; better arg validation --- devices/ble_hci/common-hal/_bleio/Adapter.c | 8 ++++++++ devices/ble_hci/common-hal/_bleio/Characteristic.c | 4 +++- devices/ble_hci/common-hal/_bleio/Descriptor.c | 2 +- shared-bindings/_bleio/Characteristic.c | 12 ++++++++++-- shared-bindings/_bleio/Descriptor.c | 11 +++++++++-- 5 files changed, 31 insertions(+), 6 deletions(-) diff --git a/devices/ble_hci/common-hal/_bleio/Adapter.c b/devices/ble_hci/common-hal/_bleio/Adapter.c index fe32241d17..b197659ac2 100644 --- a/devices/ble_hci/common-hal/_bleio/Adapter.c +++ b/devices/ble_hci/common-hal/_bleio/Adapter.c @@ -346,6 +346,8 @@ void common_hal_bleio_adapter_set_name(bleio_adapter_obj_t *self, const char* na // } mp_obj_t common_hal_bleio_adapter_start_scan(bleio_adapter_obj_t *self, uint8_t* prefixes, size_t prefix_length, bool extended, mp_int_t buffer_size, mp_float_t timeout, mp_float_t interval, mp_float_t window, mp_int_t minimum_rssi, bool active) { + // TODO + mp_raise_NotImplementedError(NULL); check_enabled(self); if (self->scan_results != NULL) { @@ -392,6 +394,8 @@ mp_obj_t common_hal_bleio_adapter_start_scan(bleio_adapter_obj_t *self, uint8_t* } void common_hal_bleio_adapter_stop_scan(bleio_adapter_obj_t *self) { + // TODO + mp_raise_NotImplementedError(NULL); check_enabled(self); // If not already scanning, no problem. @@ -430,6 +434,8 @@ void common_hal_bleio_adapter_stop_scan(bleio_adapter_obj_t *self) { // } mp_obj_t common_hal_bleio_adapter_connect(bleio_adapter_obj_t *self, bleio_address_obj_t *address, mp_float_t timeout) { + // TODO + mp_raise_NotImplementedError(NULL); check_enabled(self); @@ -742,6 +748,8 @@ mp_obj_t common_hal_bleio_adapter_get_connections(bleio_adapter_obj_t *self) { } void common_hal_bleio_adapter_erase_bonding(bleio_adapter_obj_t *self) { + // TODO + mp_raise_NotImplementedError(NULL); check_enabled(self); //FIX bonding_erase_storage(); diff --git a/devices/ble_hci/common-hal/_bleio/Characteristic.c b/devices/ble_hci/common-hal/_bleio/Characteristic.c index 5f9b96b354..393b80459a 100644 --- a/devices/ble_hci/common-hal/_bleio/Characteristic.c +++ b/devices/ble_hci/common-hal/_bleio/Characteristic.c @@ -93,7 +93,9 @@ size_t common_hal_bleio_characteristic_get_value(bleio_characteristic_obj_t *sel if (!mp_get_buffer(self->value, &bufinfo, MP_BUFFER_READ)) { return 0; } - memcpy(buf, bufinfo.buf, MIN(len, bufinfo.len)); + const size_t actual_length = MIN(len, bufinfo.len); + memcpy(buf, bufinfo.buf, actual_length); + return actual_length; } } diff --git a/devices/ble_hci/common-hal/_bleio/Descriptor.c b/devices/ble_hci/common-hal/_bleio/Descriptor.c index cc45a89857..645273e285 100644 --- a/devices/ble_hci/common-hal/_bleio/Descriptor.c +++ b/devices/ble_hci/common-hal/_bleio/Descriptor.c @@ -74,7 +74,7 @@ size_t common_hal_bleio_descriptor_get_value(bleio_descriptor_obj_t *self, uint8 if (!mp_get_buffer(self->value, &bufinfo, MP_BUFFER_READ)) { return 0; } - size_t actual_length = MIN(len, bufinfo.len); + const size_t actual_length = MIN(len, bufinfo.len); memcpy(buf, bufinfo.buf, actual_length); return actual_length; } diff --git a/shared-bindings/_bleio/Characteristic.c b/shared-bindings/_bleio/Characteristic.c index 24b127e4a3..8372321fbf 100644 --- a/shared-bindings/_bleio/Characteristic.c +++ b/shared-bindings/_bleio/Characteristic.c @@ -109,11 +109,14 @@ STATIC mp_obj_t bleio_characteristic_add_to_service(size_t n_args, const mp_obj_ const bleio_attribute_security_mode_t write_perm = args[ARG_write_perm].u_int; common_hal_bleio_attribute_security_mode_check_valid(write_perm); - const mp_int_t max_length = args[ARG_max_length].u_int; + const mp_int_t max_length_int = args[ARG_max_length].u_int; + if (max_length_int <= 0) { + mp_raise_ValueError(translate("max_length must be > 0")); + } + const size_t max_length = (size_t) max_length_int; const bool fixed_length = args[ARG_fixed_length].u_bool; mp_obj_t initial_value = args[ARG_initial_value].u_obj; - // Length will be validated in common_hal. mp_buffer_info_t initial_value_bufinfo; if (initial_value == mp_const_none) { if (fixed_length && max_length > 0) { @@ -122,7 +125,12 @@ STATIC mp_obj_t bleio_characteristic_add_to_service(size_t n_args, const mp_obj_ initial_value = mp_const_empty_bytes; } } + mp_get_buffer_raise(initial_value, &initial_value_bufinfo, MP_BUFFER_READ); + if (initial_value_bufinfo.len > max_length || + (fixed_length && initial_value_bufinfo.len != max_length)) { + mp_raise_ValueError(translate("initial_value length is wrong")); + } bleio_characteristic_obj_t *characteristic = m_new_obj(bleio_characteristic_obj_t); characteristic->base.type = &bleio_characteristic_type; diff --git a/shared-bindings/_bleio/Descriptor.c b/shared-bindings/_bleio/Descriptor.c index e24751f759..dce618ecec 100644 --- a/shared-bindings/_bleio/Descriptor.c +++ b/shared-bindings/_bleio/Descriptor.c @@ -100,11 +100,14 @@ STATIC mp_obj_t bleio_descriptor_add_to_characteristic(size_t n_args, const mp_o const bleio_attribute_security_mode_t write_perm = args[ARG_write_perm].u_int; common_hal_bleio_attribute_security_mode_check_valid(write_perm); - const mp_int_t max_length = args[ARG_max_length].u_int; + const mp_int_t max_length_int = args[ARG_max_length].u_int; + if (max_length_int <= 0) { + mp_raise_ValueError(translate("max_length must be > 0")); + } + const size_t max_length = (size_t) max_length_int; const bool fixed_length = args[ARG_fixed_length].u_bool; mp_obj_t initial_value = args[ARG_initial_value].u_obj; - // Length will be validated in common_hal. mp_buffer_info_t initial_value_bufinfo; if (initial_value == mp_const_none) { if (fixed_length && max_length > 0) { @@ -114,6 +117,10 @@ STATIC mp_obj_t bleio_descriptor_add_to_characteristic(size_t n_args, const mp_o } } mp_get_buffer_raise(initial_value, &initial_value_bufinfo, MP_BUFFER_READ); + if (initial_value_bufinfo.len > max_length || + (fixed_length && initial_value_bufinfo.len != max_length)) { + mp_raise_ValueError(translate("initial_value length is wrong")); + } bleio_descriptor_obj_t *descriptor = m_new_obj(bleio_descriptor_obj_t); descriptor->base.type = &bleio_descriptor_type;