diff --git a/system/bta/Android.bp b/system/bta/Android.bp index ff1d304cfd4022af5990bd05e7d5c35bd169eab6..939f0bec72569cf8bf7a8fa309b82dc6c9ed3dd4 100644 --- a/system/bta/Android.bp +++ b/system/bta/Android.bp @@ -194,6 +194,39 @@ cc_test { ], } +// bta unit tests for target +cc_test { + name: "net_test_bta_security", + defaults: [ + "fluoride_bta_defaults", + "mts_defaults" + ], + test_suites: ["device-tests"], + srcs: [ + ":TestCommonMockFunctions", + ":TestMockDevice", + ":TestMockStack", + ":TestMockBtif", + "test/bta_hf_client_security_test.cc", + ], + shared_libs: [ + "android.hardware.bluetooth.audio@2.0", + "android.hardware.bluetooth.audio@2.1", + "libcrypto", + "liblog", + "libprotobuf-cpp-lite", + ], + static_libs: [ + "crypto_toolbox_for_tests", + "libbtcore", + "libbt-bta", + "libbt-audio-hal-interface", + "libbluetooth-types", + "libbt-protos-lite", + "libosi", + "libbt-common", + ], +} cc_test { name: "bt_host_test_bta", defaults: [ diff --git a/system/bta/hf_client/bta_hf_client_at.cc b/system/bta/hf_client/bta_hf_client_at.cc index 5d3569bc57cebd3b4dc228b468aac8509e5330b9..5c08331c076db9dbed048a9a56f19525dde6dcd7 100644 --- a/system/bta/hf_client/bta_hf_client_at.cc +++ b/system/bta/hf_client/bta_hf_client_at.cc @@ -1721,6 +1721,13 @@ void bta_hf_client_at_parse(tBTA_HF_CLIENT_CB* client_cb, char* buf, client_cb->at_cb.offset += tmp; } + /* prevent buffer overflow in cases where LEN exceeds available buffer space + */ + if (len > BTA_HF_CLIENT_AT_PARSER_MAX_LEN - client_cb->at_cb.offset) { + android_errorWriteWithInfoLog(0x534e4554, "231156521", -1, NULL, 0); + return; + } + memcpy(client_cb->at_cb.buf + client_cb->at_cb.offset, buf, len); client_cb->at_cb.offset += len; diff --git a/system/bta/test/bta_hf_client_security_test.cc b/system/bta/test/bta_hf_client_security_test.cc new file mode 100644 index 0000000000000000000000000000000000000000..a33c52ee2841a5151186f87e4dd450475591b087 --- /dev/null +++ b/system/bta/test/bta_hf_client_security_test.cc @@ -0,0 +1,79 @@ +/****************************************************************************** + * + * Copyright 2022 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 <gtest/gtest.h> + +#include "bta/hf_client/bta_hf_client_int.h" +#include "bta/include/bta_hf_client_api.h" +#include "common/message_loop_thread.h" +#include "device/include/esco_parameters.h" +#include "test/mock/mock_device_controller.h" +#include "types/raw_address.h" + +namespace base { +class MessageLoop; +} // namespace base + +bluetooth::common::MessageLoopThread* get_main_thread() { return nullptr; } +void do_in_main_thread(base::Location const&, base::OnceCallback<void()>) { + return; +} + +namespace { +const RawAddress bdaddr1({0x11, 0x22, 0x33, 0x44, 0x55, 0x66}); +} // namespace + +// TODO(jpawlowski): there is some weird dependency issue in tests, and the +// tests here fail to compile without this definition. +void LogMsg(uint32_t trace_set_mask, const char* fmt_str, ...) {} + +class BtaHfClientSecurityTest : public testing::Test { + protected: + void SetUp() override { + // Reset the memory block, this is the state on which the allocate handle + // would start operating + bta_hf_client_cb_arr_init(); + } +}; + +// Attempt to parse a buffer which exceeds available buffer space. +// This should fail but not crash +TEST_F(BtaHfClientSecurityTest, test_parse_overflow_buffer) { + uint16_t p_handle; + bool status = bta_hf_client_allocate_handle(bdaddr1, &p_handle); + + tBTA_HF_CLIENT_CB* cb; + + // Allocation should succeed + ASSERT_EQ(true, status); + ASSERT_GT(p_handle, 0); + + cb = bta_hf_client_find_cb_by_bda(bdaddr1); + + ASSERT_TRUE(cb != NULL); + + uint16_t len = BTA_HF_CLIENT_AT_PARSER_MAX_LEN * 2 + 3; + char buf[BTA_HF_CLIENT_AT_PARSER_MAX_LEN * 2 + 3] = {'\n'}; + + bta_hf_client_at_parse(cb, (char*)(&buf[0]), len); + + ASSERT_TRUE(len); + ASSERT_TRUE(buf != NULL); + + ASSERT_TRUE(1); +} diff --git a/system/test/mock/mock_device_esco_parameters.cc b/system/test/mock/mock_device_esco_parameters.cc index 86410fe16fdca9e47d24f0e3aa52a1f471e838d7..a7725686450a53d9028abf35f5fbb5c37d855f9e 100644 --- a/system/test/mock/mock_device_esco_parameters.cc +++ b/system/test/mock/mock_device_esco_parameters.cc @@ -49,5 +49,10 @@ enh_esco_params_t esco_parameters_for_codec(esco_codec_t codec) { mock_function_count_map[__func__]++; return test::mock::device_esco_parameters::esco_parameters_for_codec(codec); } + +enh_esco_params_t esco_parameters_for_codec(esco_codec_t codec, bool b) { + mock_function_count_map[__func__]++; + return test::mock::device_esco_parameters::esco_parameters_for_codec(codec); +} // Mocked functions complete // END mockcify generation diff --git a/system/test/run_unit_tests.sh b/system/test/run_unit_tests.sh index 3b17dd159c9706066bfaec795e45a58ef388c830..ace51b419dc20751566754e87bde0df60d797c6a 100755 --- a/system/test/run_unit_tests.sh +++ b/system/test/run_unit_tests.sh @@ -10,6 +10,7 @@ known_tests=( net_test_bluetooth net_test_btcore net_test_bta + net_test_bta_security net_test_btif net_test_btif_profile_queue net_test_btif_config_cache @@ -118,6 +119,9 @@ do name="${spec%%.*}" if [[ $target_arch == *"64"* ]]; then binary="/data/nativetest64/${name}/${name}" + if [ -e "${ANDROID_PRODUCT_OUT}${binary}64" ]; then + binary="/data/nativetest64/${name}/${name}64" + fi else binary="/data/nativetest/${name}/${name}" fi