From 1ee9c729254cf4fd0c0a89399f45d91316c1f9ed Mon Sep 17 00:00:00 2001 From: Bhautik Ardeshana Date: Wed, 15 Mar 2023 08:40:25 +0530 Subject: uwb(hal): Fix security vulnarability reported in uwb hal changes are mainly to resolve OOB, arrayIndex out of box security issues Bug: 269746228,268485040,268192935,267311318 Test: Android Security Assessment Change-Id: Id3f43b048ded859ee7b14b6dba40d9b6a1ea54af --- halimpl/hal/phNxpUciHal.cc | 44 +++++++++++++++++++++++++++++++++++++----- halimpl/hal/phNxpUciHal_ext.cc | 6 +++++- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/halimpl/hal/phNxpUciHal.cc b/halimpl/hal/phNxpUciHal.cc index 9479236..5fccbcf 100644 --- a/halimpl/hal/phNxpUciHal.cc +++ b/halimpl/hal/phNxpUciHal.cc @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019, 2022 NXP + * Copyright 2012-2019, 2022-2023 NXP * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -79,6 +79,7 @@ static void phNxpUciHal_print_response_status(uint8_t *p_rx_data, *******************************************************************************/ bool get_input_map(const uint8_t* i_data, uint16_t iData_len) { vector input_vec; + bool ret = true; uint16_t i = 0, j = 0, tag = 0, len = 0; i = UCI_PKT_HDR_LEN + UCI_PKT_PAYLOAD_STATUS_LEN + UCI_PKT_NUM_CAPS_LEN; if (i_data == NULL) { @@ -86,20 +87,36 @@ bool get_input_map(const uint8_t* i_data, uint16_t iData_len) { return false; } while (i < iData_len) { + if (i + 1 >= iData_len) { + ret = false; + break; + } tag = i_data[i++]; // Tag IDs from 0xE0 to 0xE2 are extended tag IDs with 2 bytes length. if((tag >= 0xE0) && (tag <= 0xE2)) { + if (i + 1 >= iData_len) { + ret = false; + break; + } tag = (tag << 8) | i_data[i++]; } + if (i + 1 >= iData_len) { + ret = false; + break; + } len = i_data[i++]; input_vec.insert(input_vec.begin(), len); + if (i + len > iData_len) { + ret = false; + break; + } for (j = 1; j <= len; j++) { input_vec.insert(input_vec.begin() + j, i_data[i++]); } input_map[tag] = input_vec; input_vec.clear(); } - return true; + return ret; } /******************************************************************************* @@ -113,26 +130,43 @@ bool get_input_map(const uint8_t* i_data, uint16_t iData_len) { *******************************************************************************/ bool get_conf_map(uint8_t* c_data, uint16_t cData_len) { vector conf_vec; + bool ret = true; uint16_t i = 0, j = 0, tag = 0, len = 0; if (c_data == NULL) { NXPLOG_UCIHAL_D("Country code conf map creation failed, c_data is NULL" ); return false; } while (i < cData_len) { + if (i + 1 >= cData_len) { + ret = false; + break; + } tag = c_data[i++]; // Tag IDs from 0xE0 to 0xE2 are extended tag IDs with 2 bytes length. if ((tag >= 0xE0) && (tag <= 0xE2)) { + if (i + 1 >= cData_len) { + ret = false; + break; + } tag = (tag<<8) | c_data[i++]; } + if (i + 1 >= cData_len) { + ret = false; + break; + } len = c_data[i++]; conf_vec.insert(conf_vec.begin(),len); + if (i + len > cData_len) { + ret = false; + break; + } for (j = 1; j <= len; j++) { conf_vec.insert(conf_vec.begin() + j, c_data[i++]); } conf_map[tag] = conf_vec; conf_vec.clear(); } - return true; + return ret; } /****************************************************************************** @@ -642,8 +676,8 @@ tHAL_UWB_STATUS phNxpUciHal_write_unlocked(uint16_t data_len, const uint8_t* p_d goto clean_and_return; } - if(data_len > UCI_MAX_DATA_LEN){ - NXPLOG_UCIHAL_E("data_lensize exceeds the UCI_MAX_DATA_LEN"); + if ((data_len > UCI_MAX_DATA_LEN) || (data_len < UCI_PKT_HDR_LEN)) { + NXPLOG_UCIHAL_E("Invalid data_len"); data_len = 0; goto clean_and_return; } diff --git a/halimpl/hal/phNxpUciHal_ext.cc b/halimpl/hal/phNxpUciHal_ext.cc index 3dfd1a3..4c47a91 100644 --- a/halimpl/hal/phNxpUciHal_ext.cc +++ b/halimpl/hal/phNxpUciHal_ext.cc @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020,2022 NXP + * Copyright 2012-2019, 2022-2023 NXP * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -167,6 +167,10 @@ tHAL_UWB_STATUS phNxpUciHal_write_ext(uint16_t* cmd_len, uint8_t* p_cmd_data, tHAL_UWB_STATUS phNxpUciHal_send_ext_cmd(uint16_t cmd_len, const uint8_t* p_cmd) { tHAL_UWB_STATUS status; + if (cmd_len >= UCI_MAX_DATA_LEN) { + status = UWBSTATUS_FAILED; + return status; + } HAL_ENABLE_EXT(); nxpucihal_ctrl.cmd_len = cmd_len; memcpy(nxpucihal_ctrl.p_cmd_data, p_cmd, cmd_len); -- cgit v1.2.3 From e38f33ede2528cc47abedf7a1d0f7260416c0b73 Mon Sep 17 00:00:00 2001 From: Ikjoon Jang Date: Tue, 4 Apr 2023 11:42:29 +0000 Subject: Add NXP_UWB_DEVICE_NODE specifying a device node SR100S kernel driver is using /dev/sr100, make it possible to use alternative device node name specified in a configuration file. Bug: 263423931 Test: Check logcat with and without NXP_UWB_DEVICE_NODE Change-Id: I8dd807ec1a867d899b2aed0f4dec020bf2bc6f32 --- halimpl/config/SR1XX/libuwb-nxp-SR100S.conf | 4 ++++ halimpl/config/SR1XX/libuwb-nxp.conf | 4 ++++ halimpl/hal/phNxpUciHal.cc | 11 +++++++---- halimpl/utils/phNxpConfig.h | 1 + 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/halimpl/config/SR1XX/libuwb-nxp-SR100S.conf b/halimpl/config/SR1XX/libuwb-nxp-SR100S.conf index 5879d2b..a938f07 100644 --- a/halimpl/config/SR1XX/libuwb-nxp-SR100S.conf +++ b/halimpl/config/SR1XX/libuwb-nxp-SR100S.conf @@ -9,6 +9,10 @@ UWB_BOARD_VARIANT_CONFIG=0x01 UWB_BOARD_VARIANT_VERSION=0x01 +#Default device node is /dev/srxxx, +#you can override it if your kernel driver is exposing it as another name. +#NXP_UWB_DEVICE_NODE="/dev/sr100" + ############################################################################### # Extended CofigID #DELAY_CALIBRATION_VALUE E400 diff --git a/halimpl/config/SR1XX/libuwb-nxp.conf b/halimpl/config/SR1XX/libuwb-nxp.conf index a07fa04..fa31233 100644 --- a/halimpl/config/SR1XX/libuwb-nxp.conf +++ b/halimpl/config/SR1XX/libuwb-nxp.conf @@ -9,6 +9,10 @@ UWB_BOARD_VARIANT_CONFIG=0x01 UWB_BOARD_VARIANT_VERSION=0x01 +#Default device node is /dev/srxxx, +#you can override it if your kernel driver is exposing it as another name. +#NXP_UWB_DEVICE_NODE="/dev/sr100" +# ############################################################################### # Extended CofigID #DELAY_CALIBRATION_VALUE E400 diff --git a/halimpl/hal/phNxpUciHal.cc b/halimpl/hal/phNxpUciHal.cc index 5fccbcf..fd5ad99 100644 --- a/halimpl/hal/phNxpUciHal.cc +++ b/halimpl/hal/phNxpUciHal.cc @@ -329,10 +329,13 @@ tHAL_UWB_STATUS phNxpUciHal_open(uwb_stack_callback_t* p_cback, if (uwb_dev_node == NULL) { NXPLOG_UCIHAL_E("malloc of uwb_dev_node failed "); goto clean_and_return; - } else { - NXPLOG_UCIHAL_E("Assinging the default helios Node: dev/srxxx"); - strcpy(uwb_dev_node, "/dev/srxxx"); - } + } + + if (!GetNxpConfigStrValue(NAME_NXP_UWB_DEVICE_NODE, uwb_dev_node, max_len)) { + strcpy(uwb_dev_node, "/dev/srxxx"); + } + NXPLOG_UCIHAL_E("Assigning the helios Node: %s", uwb_dev_node); + /* By default HAL status is HAL_STATUS_OPEN */ nxpucihal_ctrl.halStatus = HAL_STATUS_OPEN; diff --git a/halimpl/utils/phNxpConfig.h b/halimpl/utils/phNxpConfig.h index a3537da..9d91ff1 100644 --- a/halimpl/utils/phNxpConfig.h +++ b/halimpl/utils/phNxpConfig.h @@ -51,6 +51,7 @@ int GetNxpConfigCountryCodeByteArrayValue(const char* name,const char* fName, ch #define NAME_UWB_CORE_EXT_DEVICE_SR1XX_T_CONFIG "UWB_CORE_EXT_DEVICE_SR1XX_T_CONFIG" #define NAME_UWB_CORE_EXT_DEVICE_SR1XX_S_CONFIG "UWB_CORE_EXT_DEVICE_SR1XX_S_CONFIG" +#define NAME_NXP_UWB_DEVICE_NODE "NXP_UWB_DEVICE_NODE" #define NAME_NXP_UWB_PROD_FW_FILENAME "NXP_UWB_PROD_FW_FILENAME" #define NAME_NXP_UWB_DEV_FW_FILENAME "NXP_UWB_DEV_FW_FILENAME" #define NAME_NXP_UWB_FW_FILENAME "NXP_UWB_FW_FILENAME" -- cgit v1.2.3