Loading android/app/src/com/android/bluetooth/btservice/AdapterService.java +5 −0 Original line number Diff line number Diff line Loading @@ -5099,6 +5099,7 @@ public class AdapterService extends Service { private static final String GATT_ROBUST_CACHING_CLIENT_FLAG = "INIT_gatt_robust_caching_client"; private static final String GATT_ROBUST_CACHING_SERVER_FLAG = "INIT_gatt_robust_caching_server"; private static final String IRK_ROTATION_FLAG = "INIT_irk_rotation"; private static final String PASS_PHY_UPDATE_CALLBACK_FLAG = "INIT_pass_phy_update_callback"; /** * Logging flags logic (only applies to DEBUG and VERBOSE levels): Loading Loading @@ -5162,6 +5163,10 @@ public class AdapterService extends Service { if (DeviceConfig.getBoolean(DeviceConfig.NAMESPACE_BLUETOOTH, IRK_ROTATION_FLAG, false)) { initFlags.add(String.format("%s=%s", IRK_ROTATION_FLAG, "true")); } if (DeviceConfig.getBoolean( DeviceConfig.NAMESPACE_BLUETOOTH, PASS_PHY_UPDATE_CALLBACK_FLAG, true)) { initFlags.add(String.format("%s=%s", PASS_PHY_UPDATE_CALLBACK_FLAG, "true")); } if (DeviceConfig.getBoolean(DeviceConfig.NAMESPACE_BLUETOOTH, LOGGING_DEBUG_ENABLED_FOR_ALL_FLAG, false)) { initFlags.add(String.format("%s=%s", LOGGING_DEBUG_ENABLED_FOR_ALL_FLAG, "true")); Loading android/app/src/com/android/bluetooth/gatt/GattService.java +15 −8 Original line number Diff line number Diff line Loading @@ -258,9 +258,9 @@ public class GattService extends ProfileService { /** * HashMap used to synchronize writeCharacteristic calls mapping remote device address to * available permit (either 1 or 0). * available permit (connectId or -1). */ private final HashMap<String, AtomicBoolean> mPermits = new HashMap<>(); private final HashMap<String, Integer> mPermits = new HashMap<>(); private AdapterService mAdapterService; private BluetoothAdapterProxy mBluetoothAdapterProxy; Loading Loading @@ -2025,7 +2025,7 @@ public class GattService extends ProfileService { synchronized (mPermits) { Log.d(TAG, "onConnected() - adding permit for address=" + address); mPermits.putIfAbsent(address, new AtomicBoolean(true)); mPermits.putIfAbsent(address, -1); } connectionState = BluetoothProtoEnums.CONNECTION_STATE_CONNECTED; Loading Loading @@ -2057,6 +2057,13 @@ public class GattService extends ProfileService { + address); mPermits.remove(address); } } else { synchronized (mPermits) { if (mPermits.get(address) == connId) { Log.d(TAG, "onDisconnected() - set permit -1 for address=" + address); mPermits.put(address, -1); } } } if (app != null) { Loading Loading @@ -2362,7 +2369,7 @@ public class GattService extends ProfileService { synchronized (mPermits) { Log.d(TAG, "onWriteCharacteristic() - increasing permit for address=" + address); mPermits.get(address).set(true); mPermits.put(address, -1); } if (VDBG) { Loading Loading @@ -3655,18 +3662,18 @@ public class GattService extends ProfileService { Log.d(TAG, "writeCharacteristic() - trying to acquire permit."); // Lock the thread until onCharacteristicWrite callback comes back. synchronized (mPermits) { AtomicBoolean atomicBoolean = mPermits.get(address); if (atomicBoolean == null) { Integer permit = mPermits.get(address); if (permit == null) { Log.d(TAG, "writeCharacteristic() - atomicBoolean uninitialized!"); return BluetoothStatusCodes.ERROR_DEVICE_NOT_CONNECTED; } boolean success = atomicBoolean.get(); boolean success = (permit == -1); if (!success) { Log.d(TAG, "writeCharacteristic() - no permit available."); return BluetoothStatusCodes.ERROR_GATT_WRITE_REQUEST_BUSY; } atomicBoolean.set(false); mPermits.put(address, connId); } gattClientWriteCharacteristicNative(connId, handle, writeType, authReq, value); Loading system/bta/Android.bp +33 −0 Original line number Diff line number Diff line Loading @@ -197,6 +197,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: [ Loading system/bta/hf_client/bta_hf_client_at.cc +7 −0 Original line number Diff line number Diff line Loading @@ -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; Loading system/bta/test/bta_hf_client_security_test.cc 0 → 100644 +79 −0 Original line number 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); } Loading
android/app/src/com/android/bluetooth/btservice/AdapterService.java +5 −0 Original line number Diff line number Diff line Loading @@ -5099,6 +5099,7 @@ public class AdapterService extends Service { private static final String GATT_ROBUST_CACHING_CLIENT_FLAG = "INIT_gatt_robust_caching_client"; private static final String GATT_ROBUST_CACHING_SERVER_FLAG = "INIT_gatt_robust_caching_server"; private static final String IRK_ROTATION_FLAG = "INIT_irk_rotation"; private static final String PASS_PHY_UPDATE_CALLBACK_FLAG = "INIT_pass_phy_update_callback"; /** * Logging flags logic (only applies to DEBUG and VERBOSE levels): Loading Loading @@ -5162,6 +5163,10 @@ public class AdapterService extends Service { if (DeviceConfig.getBoolean(DeviceConfig.NAMESPACE_BLUETOOTH, IRK_ROTATION_FLAG, false)) { initFlags.add(String.format("%s=%s", IRK_ROTATION_FLAG, "true")); } if (DeviceConfig.getBoolean( DeviceConfig.NAMESPACE_BLUETOOTH, PASS_PHY_UPDATE_CALLBACK_FLAG, true)) { initFlags.add(String.format("%s=%s", PASS_PHY_UPDATE_CALLBACK_FLAG, "true")); } if (DeviceConfig.getBoolean(DeviceConfig.NAMESPACE_BLUETOOTH, LOGGING_DEBUG_ENABLED_FOR_ALL_FLAG, false)) { initFlags.add(String.format("%s=%s", LOGGING_DEBUG_ENABLED_FOR_ALL_FLAG, "true")); Loading
android/app/src/com/android/bluetooth/gatt/GattService.java +15 −8 Original line number Diff line number Diff line Loading @@ -258,9 +258,9 @@ public class GattService extends ProfileService { /** * HashMap used to synchronize writeCharacteristic calls mapping remote device address to * available permit (either 1 or 0). * available permit (connectId or -1). */ private final HashMap<String, AtomicBoolean> mPermits = new HashMap<>(); private final HashMap<String, Integer> mPermits = new HashMap<>(); private AdapterService mAdapterService; private BluetoothAdapterProxy mBluetoothAdapterProxy; Loading Loading @@ -2025,7 +2025,7 @@ public class GattService extends ProfileService { synchronized (mPermits) { Log.d(TAG, "onConnected() - adding permit for address=" + address); mPermits.putIfAbsent(address, new AtomicBoolean(true)); mPermits.putIfAbsent(address, -1); } connectionState = BluetoothProtoEnums.CONNECTION_STATE_CONNECTED; Loading Loading @@ -2057,6 +2057,13 @@ public class GattService extends ProfileService { + address); mPermits.remove(address); } } else { synchronized (mPermits) { if (mPermits.get(address) == connId) { Log.d(TAG, "onDisconnected() - set permit -1 for address=" + address); mPermits.put(address, -1); } } } if (app != null) { Loading Loading @@ -2362,7 +2369,7 @@ public class GattService extends ProfileService { synchronized (mPermits) { Log.d(TAG, "onWriteCharacteristic() - increasing permit for address=" + address); mPermits.get(address).set(true); mPermits.put(address, -1); } if (VDBG) { Loading Loading @@ -3655,18 +3662,18 @@ public class GattService extends ProfileService { Log.d(TAG, "writeCharacteristic() - trying to acquire permit."); // Lock the thread until onCharacteristicWrite callback comes back. synchronized (mPermits) { AtomicBoolean atomicBoolean = mPermits.get(address); if (atomicBoolean == null) { Integer permit = mPermits.get(address); if (permit == null) { Log.d(TAG, "writeCharacteristic() - atomicBoolean uninitialized!"); return BluetoothStatusCodes.ERROR_DEVICE_NOT_CONNECTED; } boolean success = atomicBoolean.get(); boolean success = (permit == -1); if (!success) { Log.d(TAG, "writeCharacteristic() - no permit available."); return BluetoothStatusCodes.ERROR_GATT_WRITE_REQUEST_BUSY; } atomicBoolean.set(false); mPermits.put(address, connId); } gattClientWriteCharacteristicNative(connId, handle, writeType, authReq, value); Loading
system/bta/Android.bp +33 −0 Original line number Diff line number Diff line Loading @@ -197,6 +197,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: [ Loading
system/bta/hf_client/bta_hf_client_at.cc +7 −0 Original line number Diff line number Diff line Loading @@ -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; Loading
system/bta/test/bta_hf_client_security_test.cc 0 → 100644 +79 −0 Original line number 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); }