From efa23116677e0d64cbbcafd1fab67414cf2b773b Mon Sep 17 00:00:00 2001 From: Chris Manton Date: Wed, 4 Mar 2020 11:26:31 -0800 Subject: [PATCH] Fix potential overflow for gatt process write Bug: 143604331 Test: net_test_stack_gatt_native Change-Id: I88704d74ba0c265872fbfd20c8bcd3bc1dd60360 Merged-In: I88704d74ba0c265872fbfd20c8bcd3bc1dd60360 --- TEST_MAPPING | 4 + stack/Android.bp | 35 +++++ stack/gatt/gatt_sr.cc | 13 +- stack/test/gatt/gatt_sr_test.cc | 320 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 366 insertions(+), 6 deletions(-) create mode 100644 stack/test/gatt/gatt_sr_test.cc diff --git a/TEST_MAPPING b/TEST_MAPPING index 1ca5c4b62..bbaf4576f 100755 --- a/TEST_MAPPING +++ b/TEST_MAPPING @@ -86,6 +86,10 @@ "host" : true }, { + "name" : "net_test_stack_gatt_native", + "host" : true + }, + { "name" : "net_test_stack_a2dp_native", "host" : true }, diff --git a/stack/Android.bp b/stack/Android.bp index fbe5ea181..c8f4bad99 100644 --- a/stack/Android.bp +++ b/stack/Android.bp @@ -541,3 +541,38 @@ cc_test { }, }, } + +cc_test { + name: "net_test_stack_gatt_native", + defaults: ["fluoride_defaults"], + test_suites: ["device-tests"], + host_supported: true, + include_dirs: [ + "system/bt", + "system/bt/stack/include", + "system/bt/stack/l2cap", + "system/bt/stack/btm", + "system/bt/utils/include", + ], + srcs: [ + "test/gatt/gatt_sr_test.cc", + "gatt/gatt_utils.cc", + ], + shared_libs: [ + "libcutils", + "libprotobuf-cpp-lite", + "libcrypto", + ], + static_libs: [ + "liblog", + "libosi", + "libbt-common", + "libbt-protos-lite", + "libosi-AllocationTestHarness", + ], + sanitize: { + address: true, + cfi: true, + misc_undefined: ["bounds"], + }, +} diff --git a/stack/gatt/gatt_sr.cc b/stack/gatt/gatt_sr.cc index e5fdd1b92..6336adae6 100644 --- a/stack/gatt/gatt_sr.cc +++ b/stack/gatt/gatt_sr.cc @@ -860,10 +860,10 @@ void gatts_process_read_by_type_req(tGATT_TCB& tcb, uint8_t op_code, /** * This function is called to process the write request from client. */ -void gatts_process_write_req(tGATT_TCB& tcb, tGATT_SRV_LIST_ELEM& el, - uint16_t handle, uint8_t op_code, uint16_t len, - uint8_t* p_data, - bt_gatt_db_attribute_type_t gatt_type) { +static void gatts_process_write_req(tGATT_TCB& tcb, tGATT_SRV_LIST_ELEM& el, + uint16_t handle, uint8_t op_code, + uint16_t len, uint8_t* p_data, + bt_gatt_db_attribute_type_t gatt_type) { tGATTS_DATA sr_data; uint32_t trans_id; tGATT_STATUS status; @@ -874,7 +874,7 @@ void gatts_process_write_req(tGATT_TCB& tcb, tGATT_SRV_LIST_ELEM& el, switch (op_code) { case GATT_REQ_PREPARE_WRITE: - if (len < 2) { + if (len < 2 || p == nullptr) { LOG(ERROR) << __func__ << ": Prepare write request was invalid - missing offset, " "sending error response"; @@ -896,8 +896,9 @@ void gatts_process_write_req(tGATT_TCB& tcb, tGATT_SRV_LIST_ELEM& el, if (op_code == GATT_REQ_WRITE || op_code == GATT_REQ_PREPARE_WRITE) sr_data.write_req.need_rsp = true; sr_data.write_req.handle = handle; + if (len > GATT_MAX_ATTR_LEN) len = GATT_MAX_ATTR_LEN; sr_data.write_req.len = len; - if (len != 0 && p != NULL) { + if (len != 0 && p != nullptr) { memcpy(sr_data.write_req.value, p, len); } break; diff --git a/stack/test/gatt/gatt_sr_test.cc b/stack/test/gatt/gatt_sr_test.cc new file mode 100644 index 000000000..a5fe4cd40 --- /dev/null +++ b/stack/test/gatt/gatt_sr_test.cc @@ -0,0 +1,320 @@ +/* + * Copyright 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include +#include + +#include "osi/test/AllocationTestHarness.h" +#include "stack/gatt/gatt_int.h" +#undef LOG_TAG +#include "stack/gatt/gatt_sr.cc" +#include "types/raw_address.h" + +#define MAX_UINT16 ((uint16_t)0xffff) + +tGATT_CB gatt_cb; + +namespace { + +struct TestMutables { + struct { + uint8_t op_code_; + } attp_build_sr_msg; + struct { + uint16_t conn_id_{0}; + uint32_t trans_id_{0}; + tGATTS_REQ_TYPE type_{0xff}; + tGATTS_DATA data_; + } application_request_callback; + struct { + int access_count_{0}; + tGATT_STATUS return_status_{GATT_SUCCESS}; + } gatts_write_attr_perm_check; +}; + +TestMutables test_state_; +} // namespace + +namespace connection_manager { +bool background_connect_remove(uint8_t app_id, const RawAddress& address) { + return false; +} +bool direct_connect_remove(uint8_t app_id, const RawAddress& address) { + return false; +} +} // namespace connection_manager + +BT_HDR* attp_build_sr_msg(tGATT_TCB& tcb, uint8_t op_code, + tGATT_SR_MSG* p_msg) { + test_state_.attp_build_sr_msg.op_code_ = op_code; + return nullptr; +} +tGATT_STATUS attp_send_cl_msg(tGATT_TCB& tcb, tGATT_CLCB* p_clcb, + uint8_t op_code, tGATT_CL_MSG* p_msg) { + return 0; +} +tGATT_STATUS attp_send_sr_msg(tGATT_TCB& tcb, BT_HDR* p_msg) { return 0; } +uint8_t btm_ble_read_sec_key_size(const RawAddress& bd_addr) { return 0; } +bool BTM_GetSecurityFlagsByTransport(const RawAddress& bd_addr, + uint8_t* p_sec_flags, + tBT_TRANSPORT transport) { + return false; +} +void gatt_act_discovery(tGATT_CLCB* p_clcb) {} +bool gatt_disconnect(tGATT_TCB* p_tcb) { return false; } +tGATT_CH_STATE gatt_get_ch_state(tGATT_TCB* p_tcb) { return 0; } +tGATT_STATUS gatts_db_read_attr_value_by_type( + tGATT_TCB& tcb, tGATT_SVC_DB* p_db, uint8_t op_code, BT_HDR* p_rsp, + uint16_t s_handle, uint16_t e_handle, const Uuid& type, uint16_t* p_len, + tGATT_SEC_FLAG sec_flag, uint8_t key_size, uint32_t trans_id, + uint16_t* p_cur_handle) { + return 0; +} +void gatt_set_ch_state(tGATT_TCB* p_tcb, tGATT_CH_STATE ch_state) {} +Uuid* gatts_get_service_uuid(tGATT_SVC_DB* p_db) { return nullptr; } +tGATT_STATUS GATTS_HandleValueIndication(uint16_t conn_id, uint16_t attr_handle, + uint16_t val_len, uint8_t* p_val) { + return GATT_SUCCESS; +} +tGATT_STATUS gatts_read_attr_perm_check(tGATT_SVC_DB* p_db, bool is_long, + uint16_t handle, + tGATT_SEC_FLAG sec_flag, + uint8_t key_size) { + return GATT_SUCCESS; +} +tGATT_STATUS gatts_read_attr_value_by_handle( + tGATT_TCB& tcb, tGATT_SVC_DB* p_db, uint8_t op_code, uint16_t handle, + uint16_t offset, uint8_t* p_value, uint16_t* p_len, uint16_t mtu, + tGATT_SEC_FLAG sec_flag, uint8_t key_size, uint32_t trans_id) { + return GATT_SUCCESS; +} +tGATT_STATUS gatts_write_attr_perm_check(tGATT_SVC_DB* p_db, uint8_t op_code, + uint16_t handle, uint16_t offset, + uint8_t* p_data, uint16_t len, + tGATT_SEC_FLAG sec_flag, + uint8_t key_size) { + test_state_.gatts_write_attr_perm_check.access_count_++; + return test_state_.gatts_write_attr_perm_check.return_status_; +} +void gatt_update_app_use_link_flag(tGATT_IF gatt_if, tGATT_TCB* p_tcb, + bool is_add, bool check_acl_link) {} +base::MessageLoop* get_main_message_loop() { return nullptr; } +void l2cble_set_fixed_channel_tx_data_length(const RawAddress& remote_bda, + uint16_t fix_cid, + uint16_t tx_mtu) {} +bool SDP_AddAttribute(uint32_t handle, uint16_t attr_id, uint8_t attr_type, + uint32_t attr_len, uint8_t* p_val) { + return false; +} +bool SDP_AddProtocolList(uint32_t handle, uint16_t num_elem, + tSDP_PROTOCOL_ELEM* p_elem_list) { + return false; +} +bool SDP_AddServiceClassIdList(uint32_t handle, uint16_t num_services, + uint16_t* p_service_uuids) { + return false; +} +bool SDP_AddUuidSequence(uint32_t handle, uint16_t attr_id, uint16_t num_uuids, + uint16_t* p_uuids) { + return false; +} +uint32_t SDP_CreateRecord(void) { return 0; } + +void ApplicationRequestCallback(uint16_t conn_id, uint32_t trans_id, + tGATTS_REQ_TYPE type, tGATTS_DATA* p_data) { + test_state_.application_request_callback.conn_id_ = conn_id; + test_state_.application_request_callback.trans_id_ = trans_id; + test_state_.application_request_callback.type_ = type; + test_state_.application_request_callback.data_ = *p_data; +} + +/** + * Test class to test selected functionality in stack/gatt/gatt_sr.cc + */ +extern void allocation_tracker_uninit(void); +namespace { +uint16_t kHandle = 1; +bt_gatt_db_attribute_type_t kGattCharacteristicType = BTGATT_DB_CHARACTERISTIC; +} // namespace +class GattSrTest : public AllocationTestHarness { + protected: + void SetUp() override { + AllocationTestHarness::SetUp(); + // Disable our allocation tracker to allow ASAN full range + allocation_tracker_uninit(); + memset(&tcb_, 0, sizeof(tcb_)); + memset(&el_, 0, sizeof(el_)); + + tcb_.trans_id = 0x12345677; + el_.gatt_if = 1; + gatt_cb.cl_rcb[el_.gatt_if - 1].in_use = true; + gatt_cb.cl_rcb[el_.gatt_if - 1].app_cb.p_req_cb = + ApplicationRequestCallback; + + test_state_ = TestMutables(); + } + + void TearDown() override { AllocationTestHarness::TearDown(); } + + tGATT_TCB tcb_; + tGATT_SRV_LIST_ELEM el_; +}; + +TEST_F(GattSrTest, gatts_process_write_req_request_prepare_write_no_data) { + gatts_process_write_req(tcb_, el_, kHandle, GATT_REQ_PREPARE_WRITE, 0, + nullptr, kGattCharacteristicType); +} + +TEST_F(GattSrTest, + gatts_process_write_req_request_prepare_write_max_len_no_data) { + gatts_process_write_req(tcb_, el_, kHandle, GATT_REQ_PREPARE_WRITE, + MAX_UINT16, nullptr, kGattCharacteristicType); +} + +TEST_F(GattSrTest, + gatts_process_write_req_request_prepare_write_zero_len_max_data) { + uint8_t max_mem[MAX_UINT16]; + gatts_process_write_req(tcb_, el_, kHandle, GATT_REQ_PREPARE_WRITE, 0, + max_mem, kGattCharacteristicType); +} + +TEST_F(GattSrTest, gatts_process_write_req_request_prepare_write_typical) { + uint8_t p_data[2] = {0x34, 0x12}; + uint16_t length = static_cast(sizeof(p_data) / sizeof(p_data[0])); + gatts_process_write_req(tcb_, el_, kHandle, GATT_REQ_PREPARE_WRITE, length, + p_data, kGattCharacteristicType); + + CHECK(test_state_.gatts_write_attr_perm_check.access_count_ == 1); + CHECK(test_state_.application_request_callback.conn_id_ == el_.gatt_if); + CHECK(test_state_.application_request_callback.trans_id_ == 0x12345678); + CHECK(test_state_.application_request_callback.type_ == + GATTS_REQ_TYPE_WRITE_CHARACTERISTIC); + CHECK(test_state_.application_request_callback.data_.write_req.offset == + 0x1234); + CHECK(test_state_.application_request_callback.data_.write_req.is_prep == + true); + CHECK(test_state_.application_request_callback.data_.write_req.len == 0); +} + +TEST_F(GattSrTest, gatts_process_write_req_signed_command_write_no_data) { + gatts_process_write_req(tcb_, el_, kHandle, GATT_SIGN_CMD_WRITE, 0, nullptr, + kGattCharacteristicType); +} + +TEST_F(GattSrTest, + gatts_process_write_req_signed_command_write_max_len_no_data) { + gatts_process_write_req(tcb_, el_, kHandle, GATT_SIGN_CMD_WRITE, MAX_UINT16, + nullptr, kGattCharacteristicType); +} + +TEST_F(GattSrTest, + gatts_process_write_req_signed_command_write_zero_len_max_data) { + uint8_t max_mem[MAX_UINT16]; + gatts_process_write_req(tcb_, el_, kHandle, GATT_SIGN_CMD_WRITE, 0, max_mem, + kGattCharacteristicType); +} + +TEST_F(GattSrTest, gatts_process_write_req_signed_command_write_typical) { + static constexpr size_t kDataLength = 4; + uint8_t p_data[GATT_AUTH_SIGN_LEN + kDataLength] = { + 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, + 0x99, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff, 0x01}; + uint16_t length = static_cast(sizeof(p_data) / sizeof(p_data[0])); + gatts_process_write_req(tcb_, el_, kHandle, GATT_SIGN_CMD_WRITE, length, + p_data, kGattCharacteristicType); + + CHECK(test_state_.gatts_write_attr_perm_check.access_count_ == 1); + CHECK(test_state_.application_request_callback.conn_id_ == el_.gatt_if); + CHECK(test_state_.application_request_callback.trans_id_ == 0x12345678); + CHECK(test_state_.application_request_callback.type_ == + GATTS_REQ_TYPE_WRITE_CHARACTERISTIC); + CHECK(test_state_.application_request_callback.data_.write_req.offset == 0x0); + CHECK(test_state_.application_request_callback.data_.write_req.is_prep == + false); + CHECK(test_state_.application_request_callback.data_.write_req.len == + kDataLength); +} + +TEST_F(GattSrTest, gatts_process_write_req_command_write_no_data) { + gatts_process_write_req(tcb_, el_, kHandle, GATT_CMD_WRITE, 0, nullptr, + kGattCharacteristicType); +} + +TEST_F(GattSrTest, gatts_process_write_req_command_write_max_len_no_data) { + gatts_process_write_req(tcb_, el_, kHandle, GATT_CMD_WRITE, MAX_UINT16, + nullptr, kGattCharacteristicType); +} + +TEST_F(GattSrTest, gatts_process_write_req_command_write_zero_len_max_data) { + uint8_t max_mem[MAX_UINT16]; + gatts_process_write_req(tcb_, el_, kHandle, GATT_CMD_WRITE, 0, max_mem, + kGattCharacteristicType); +} + +TEST_F(GattSrTest, gatts_process_write_req_command_write_typical) { + uint8_t p_data[16] = {0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, + 0x99, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff, 0x01}; + uint16_t length = static_cast(sizeof(p_data) / sizeof(p_data[0])); + gatts_process_write_req(tcb_, el_, kHandle, GATT_CMD_WRITE, length, p_data, + kGattCharacteristicType); + + CHECK(test_state_.gatts_write_attr_perm_check.access_count_ == 1); + CHECK(test_state_.application_request_callback.conn_id_ == el_.gatt_if); + CHECK(test_state_.application_request_callback.trans_id_ == 0x12345678); + CHECK(test_state_.application_request_callback.type_ == + GATTS_REQ_TYPE_WRITE_CHARACTERISTIC); + CHECK(test_state_.application_request_callback.data_.write_req.offset == 0x0); + CHECK(test_state_.application_request_callback.data_.write_req.is_prep == + false); + CHECK(test_state_.application_request_callback.data_.write_req.len == length); +} + +TEST_F(GattSrTest, gatts_process_write_req_request_write_no_data) { + gatts_process_write_req(tcb_, el_, kHandle, GATT_REQ_WRITE, 0, nullptr, + kGattCharacteristicType); +} + +TEST_F(GattSrTest, gatts_process_write_req_request_write_max_len_no_data) { + gatts_process_write_req(tcb_, el_, kHandle, GATT_REQ_WRITE, MAX_UINT16, + nullptr, kGattCharacteristicType); +} + +TEST_F(GattSrTest, gatts_process_write_req_request_write_zero_len_max_data) { + uint8_t max_mem[MAX_UINT16]; + gatts_process_write_req(tcb_, el_, kHandle, GATT_REQ_WRITE, 0, max_mem, + kGattCharacteristicType); +} + +TEST_F(GattSrTest, gatts_process_write_req_request_write_typical) { + uint8_t p_data[16] = {0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, + 0x99, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff, 0x01}; + uint16_t length = static_cast(sizeof(p_data) / sizeof(p_data[0])); + + gatts_process_write_req(tcb_, el_, kHandle, GATT_REQ_WRITE, length, p_data, + kGattCharacteristicType); + + CHECK(test_state_.gatts_write_attr_perm_check.access_count_ == 1); + CHECK(test_state_.application_request_callback.conn_id_ == el_.gatt_if); + CHECK(test_state_.application_request_callback.trans_id_ == 0x12345678); + CHECK(test_state_.application_request_callback.type_ == + GATTS_REQ_TYPE_WRITE_CHARACTERISTIC); + CHECK(test_state_.application_request_callback.data_.write_req.offset == 0x0); + CHECK(test_state_.application_request_callback.data_.write_req.is_prep == + false); + CHECK(test_state_.application_request_callback.data_.write_req.len == length); +} -- 2.11.0