CVE-2021-3658: Incorrect Authorization (CWE-863)
value: bluetoothd from bluez incorrectly saves adapters' Discoverable status when a device is powered down, and restores it when powered up. If a device is powered down while discoverable, it will be discoverable when powered on again. This could lead to inadvertent exposure of the bluetooth stack to physically nearby attackers.
CWE description: The product performs an authorization check when an actor attempts to access a resource or perform an action, but it does not correctly perform the check. This allows attackers to bypass intended access restrictions.
CVSS v3 base score: 6.5
Published: 2022-03-02T23:15Z
Repository: https://github.com/bluez/bluez
Language: C
Files changed: src/adapter.c
Commit date: 2021-07-26 09:30:06 -0700
Commit message: adapter: Fix storing discoverable setting

discoverable setting shall only be store when changed via Discoverable
property and not when discovery client set it as that be considered
temporary just for the lifetime of the discovery.

Diff: diff --git a/src/adapter.c b/src/adapter.c
index 12e4ff5c0..663b778e4 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -558,11 +558,15 @@ static void settings_changed(struct btd_adapter *adapter, uint32_t settings)
 		trigger_passive_scanning(adapter);
 
 	if (changed_mask & MGMT_SETTING_DISCOVERABLE) {
 		g_dbus_emit_property_changed(dbus_conn, adapter->path,
 					ADAPTER_INTERFACE, "Discoverable");
-		store_adapter_info(adapter);
+		/* Only persist discoverable setting if it was not set
+		 * temporarily by discovery.
+		 */
+		if (!adapter->discovery_discoverable)
+			store_adapter_info(adapter);
 		btd_adv_manager_refresh(adapter->adv_manager);
 	}
 
 	if (changed_mask & MGMT_SETTING_BONDABLE) {
 		g_dbus_emit_property_changed(dbus_conn, adapter->path,
@@ -2160,32 +2164,37 @@ static bool filters_equal(struct mgmt_cp_start_service_discovery *a,
 }
 
 static int update_discovery_filter(struct btd_adapter *adapter)
 {
 	struct mgmt_cp_start_service_discovery *sd_cp;
-	GSList *l;
-
 
 	DBG("");
 
 	if (discovery_filter_to_mgmt_cp(adapter, &sd_cp)) {
 		btd_error(adapter->dev_id,
 				"discovery_filter_to_mgmt_cp returned error");
 		return -ENOMEM;
 	}
 
-	for (l = adapter->discovery_list; l; l = g_slist_next(l)) {
-		struct discovery_client *client = l->data;
+	/* Only attempt to overwrite current discoverable setting when not
+	 * discoverable.
+	 */
+	if (!(adapter->current_settings & MGMT_OP_SET_DISCOVERABLE)) {
+		GSList *l;
 
-		if (!client->discovery_filter)
-			continue;
+		for (l = adapter->discovery_list; l; l = g_slist_next(l)) {
+			struct discovery_client *client = l->data;
 
-		if (client->discovery_filter->discoverable)
-			break;
-	}
+			if (!client->discovery_filter)
+				continue;
 
-	set_discovery_discoverable(adapter, l ? true : false);
+			if (client->discovery_filter->discoverable) {
+				set_discovery_discoverable(adapter, true);
+				break;
+			}
+		}
+	}
 
 	/*
 	 * If filters are equal, then don't update scan, except for when
 	 * starting discovery.
 	 */
@@ -2214,12 +2223,11 @@ static int discovery_stop(struct discovery_client *client)
 		discovery_remove(client);
 		update_discovery_filter(adapter);
 		return 0;
 	}
 
-	if (adapter->discovery_discoverable)
-		set_discovery_discoverable(adapter, false);
+	set_discovery_discoverable(adapter, false);
 
 	/*
 	 * In the idle phase of a discovery, there is no need to stop it
 	 * and so it is enough to send out the signal and just return.
 	 */
@@ -6911,10 +6919,11 @@ static void adapter_stop(struct btd_adapter *adapter)
 	adapter->filtered_discovery = false;
 	adapter->no_scan_restart_delay = false;
 	g_free(adapter->current_discovery_filter);
 	adapter->current_discovery_filter = NULL;
 
+	set_discovery_discoverable(adapter, false);
 	adapter->discovering = false;
 
 	while (adapter->connections) {
 		struct btd_device *device = adapter->connections->data;
 		uint8_t addr_type = btd_device_get_bdaddr_type(device);

Vulnerable code: 		store_adapter_info(adapter);
	GSList *l;

	for (l = adapter->discovery_list; l; l = g_slist_next(l)) {
		struct discovery_client *client = l->data;
		if (!client->discovery_filter)
			continue;
		if (client->discovery_filter->discoverable)
			break;
	}
	set_discovery_discoverable(adapter, l ? true : false);
	if (adapter->discovery_discoverable)
		set_discovery_discoverable(adapter, false);
Fixed code: 		/* Only persist discoverable setting if it was not set
		 * temporarily by discovery.
		 */
		if (!adapter->discovery_discoverable)
			store_adapter_info(adapter);
	/* Only attempt to overwrite current discoverable setting when not
	 * discoverable.
	 */
	if (!(adapter->current_settings & MGMT_OP_SET_DISCOVERABLE)) {
		GSList *l;
		for (l = adapter->discovery_list; l; l = g_slist_next(l)) {
			struct discovery_client *client = l->data;
			if (!client->discovery_filter)
				continue;
			if (client->discovery_filter->discoverable) {
				set_discovery_discoverable(adapter, true);
				break;
			}
		}
	}
	set_discovery_discoverable(adapter, false);
	set_discovery_discoverable(adapter, false);