Fix buffer overflow in BRSF am: f8adec665a (f9ef56c6) · Commits · e / os / android_packages_modules_Bluetooth

system/bta/Android.bp

+33 −0

Original line number	Original line	Diff line number	Diff line
	@@ -195,6 +195,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 {		cc_test {
	name: "bt_host_test_bta",		name: "bt_host_test_bta",
	defaults: [		defaults: [

system/bta/hf_client/bta_hf_client_at.cc

+7 −0

Original line number	Original line	Diff line number	Diff line
	@@ -1721,6 +1721,13 @@ void bta_hf_client_at_parse(tBTA_HF_CLIENT_CB* client_cb, char* buf,
	client_cb->at_cb.offset += tmp;		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);		memcpy(client_cb->at_cb.buf + client_cb->at_cb.offset, buf, len);
	client_cb->at_cb.offset += len;		client_cb->at_cb.offset += len;

system/bta/test/bta_hf_client_security_test.cc

0 → 100644

+79 −0

Original line number	Original line	Diff line number	Diff line
			/******************************************************************************
			*
			* 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);
			}

system/test/mock/mock_device_esco_parameters.cc

+5 −0

Original line number	Original line	Diff line number	Diff line
	@@ -49,5 +49,10 @@ enh_esco_params_t esco_parameters_for_codec(esco_codec_t codec) {
	mock_function_count_map[__func__]++;		mock_function_count_map[__func__]++;
	return test::mock::device_esco_parameters::esco_parameters_for_codec(codec);		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		// Mocked functions complete
	// END mockcify generation		// END mockcify generation

system/test/run_unit_tests.sh

+4 −0

Original line number	Original line	Diff line number	Diff line
	@@ -10,6 +10,7 @@ known_tests=(
	net_test_bluetooth		net_test_bluetooth
	net_test_btcore		net_test_btcore
	net_test_bta		net_test_bta
			net_test_bta_security
	net_test_btif		net_test_btif
	net_test_btif_profile_queue		net_test_btif_profile_queue
	net_test_btif_config_cache		net_test_btif_config_cache
	@@ -118,6 +119,9 @@ do
	name="${spec%%.*}"		name="${spec%%.*}"
	if [[ $target_arch == "64" ]]; then		if [[ $target_arch == "64" ]]; then
	binary="/data/nativetest64/${name}/${name}"		binary="/data/nativetest64/${name}/${name}"
			if [ -e "${ANDROID_PRODUCT_OUT}${binary}64" ]; then
			binary="/data/nativetest64/${name}/${name}64"
			fi
	else		else
	binary="/data/nativetest/${name}/${name}"		binary="/data/nativetest/${name}/${name}"
	fi		fi