Bluetooth: BAP: BA: Add validation of add/mod src

The functions did not verify the provided parameters before
sending it as a control point operation.

This also fixes a bug where BT_BAP_BIS_SYNC_NO_PREF was not
allowed when pa_sync=0.

Signed-off-by: Emil Gydesen <[email protected]>

Author: Thalley

subsys/bluetooth/audio/bap_broadcast_assistant.c

@@ -1238,6 +1238,112 @@ int bt_bap_broadcast_assistant_scan_stop(struct bt_conn *conn)
 	return bt_bap_broadcast_assistant_common_cp(conn, &att_buf);
 }
 
+static bool bis_syncs_unique_or_no_pref(uint32_t requested_bis_syncs, uint32_t aggregated_bis_syncs)
+{
+	if (requested_bis_syncs == 0U || aggregated_bis_syncs == 0U) {
+		return true;
+	}
+
+	if (requested_bis_syncs == BT_BAP_BIS_SYNC_NO_PREF &&
+	    aggregated_bis_syncs == BT_BAP_BIS_SYNC_NO_PREF) {
+		return true;
+	}
+
+	return (requested_bis_syncs & aggregated_bis_syncs) != 0U;
+}
+
+static bool valid_subgroup_params(uint8_t pa_sync, const struct bt_bap_bass_subgroup subgroups[],
+				  uint8_t num_subgroups)
+{
+	uint32_t aggregated_bis_syncs = 0U;
+
+	for (uint8_t i = 0U; i < num_subgroups; i++) {
+		/* BIS sync values of 0 and BT_BAP_BIS_SYNC_NO_PREF are allowed at any time, but any
+		 * other values are only allowed if PA sync state is also set
+		 */
+		CHECKIF(pa_sync == 0 && (subgroups[i].bis_sync != 0U &&
+					 subgroups[i].bis_sync != BT_BAP_BIS_SYNC_NO_PREF)) {
+			LOG_DBG("[%u]: Only syncing to BIS is not allowed", i);
+
+			return false;
+		}
+
+		/* Verify that the request BIS sync indexes are unique or no preference */
+		if (!bis_syncs_unique_or_no_pref(subgroups[i].bis_sync, aggregated_bis_syncs)) {
+			LOG_DBG("[%u]: Duplicate BIS index 0x%08x (aggregated 0x%08x)", i,
+				subgroups[i].bis_sync, aggregated_bis_syncs);
+
+			return false;
+		}
+
+		/* Keep track of BIS sync values to ensure that we do not have duplicates */
+		if (subgroups[i].bis_sync != BT_BAP_BIS_SYNC_NO_PREF) {
+			aggregated_bis_syncs |= subgroups[i].bis_sync;
+		}
+
+#if defined(CONFIG_BT_AUDIO_CODEC_CFG_MAX_METADATA_SIZE)
+		if (subgroups[i].metadata_len > CONFIG_BT_AUDIO_CODEC_CFG_MAX_METADATA_SIZE) {
+			LOG_DBG("[%u]: Invalid metadata_len: %u", i, subgroups[i].metadata_len);
+
+			return false;
+		}
+	}
+#endif /* CONFIG_BT_AUDIO_CODEC_CFG_MAX_METADATA_SIZE */
+
+	return true;
+}
+
+static bool valid_add_src_param(const struct bt_bap_broadcast_assistant_add_src_param *param)
+{
+	CHECKIF(param == NULL) {
+		LOG_DBG("param is NULL");
+		return false;
+	}
+
+	CHECKIF(param->addr.type > BT_ADDR_LE_RANDOM) {
+		LOG_DBG("Invalid address type %u", param->addr.type);
+		return false;
+	}
+
+	CHECKIF(param->adv_sid > BT_GAP_SID_MAX) {
+		LOG_DBG("Invalid adv_sid %u", param->adv_sid);
+		return false;
+	}
+
+	CHECKIF(!(param->pa_interval != BT_BAP_PA_INTERVAL_UNKNOWN) &&
+		!IN_RANGE(param->pa_interval, BT_GAP_PER_ADV_MIN_INTERVAL,
+			  BT_GAP_PER_ADV_MAX_INTERVAL)) {
+		LOG_DBG("Invalid pa_interval 0x%04X", param->pa_interval);
+		return false;
+	}
+
+	CHECKIF(param->broadcast_id > BT_AUDIO_BROADCAST_ID_MAX) {
+		LOG_DBG("Invalid broadcast_id 0x%08X", param->broadcast_id);
+		return false;
+	}
+
+	CHECKIF(param->num_subgroups != 0 && param->subgroups == NULL) {
+		LOG_DBG("Subgroups are NULL when num_subgroups = %u", param->num_subgroups);
+		return false;
+	}
+
+	CHECKIF(param->num_subgroups > CONFIG_BT_BAP_BASS_MAX_SUBGROUPS) {
+		LOG_DBG("Too many subgroups %u/%u", param->num_subgroups,
+			CONFIG_BT_BAP_BASS_MAX_SUBGROUPS);
+
+		return false;
+	}
+
+	CHECKIF(param->subgroups != NULL) {
+		if (!valid_subgroup_params(param->pa_sync, param->subgroups,
+					   param->num_subgroups)) {
+			return false;
+		}
+	}
+
+	return true;
+}
+
 int bt_bap_broadcast_assistant_add_src(struct bt_conn *conn,
 				       const struct bt_bap_broadcast_assistant_add_src_param *param)
 {
@@ -1264,6 +1370,10 @@ int bt_bap_broadcast_assistant_add_src(struct bt_conn *conn,
 		return -EINVAL;
 	}
 
+	if (!valid_add_src_param(param)) {
+		return -EINVAL;
+	}
+
 	if (inst->cp_handle == 0) {
 		LOG_DBG("handle not set");
 
@@ -1315,28 +1425,58 @@ int bt_bap_broadcast_assistant_add_src(struct bt_conn *conn,
 
 		subgroup->bis_sync = param->subgroups[i].bis_sync;
 
-		CHECKIF(param->pa_sync == 0 && subgroup->bis_sync != 0) {
-			LOG_DBG("Only syncing to BIS is not allowed");
-
-			atomic_clear_bit(inst->flags, BAP_BA_FLAG_BUSY);
-
-			return -EINVAL;
-		}
-
+#if defined(CONFIG_BT_AUDIO_CODEC_CFG_MAX_METADATA_SIZE)
 		if (param->subgroups[i].metadata_len != 0) {
-			(void)memcpy(subgroup->metadata,
-				     param->subgroups[i].metadata,
+			(void)memcpy(subgroup->metadata, param->subgroups[i].metadata,
 				     param->subgroups[i].metadata_len);
 			subgroup->metadata_len = param->subgroups[i].metadata_len;
 		} else {
-			subgroup->metadata_len = 0;
+			subgroup->metadata_len = 0U;
 		}
-
+#else
+		subgroup->metadata_len = 0U;
+#endif /* CONFIG_BT_AUDIO_CODEC_CFG_MAX_METADATA_SIZE */
 	}
 
 	return bt_bap_broadcast_assistant_common_cp(conn, &att_buf);
 }
 
+static bool valid_add_mod_param(const struct bt_bap_broadcast_assistant_mod_src_param *param)
+{
+	CHECKIF(param == NULL) {
+		LOG_DBG("param is NULL");
+		return false;
+	}
+
+	CHECKIF(!(param->pa_interval != BT_BAP_PA_INTERVAL_UNKNOWN) &&
+		!IN_RANGE(param->pa_interval, BT_GAP_PER_ADV_MIN_INTERVAL,
+			  BT_GAP_PER_ADV_MAX_INTERVAL)) {
+		LOG_DBG("Invalid pa_interval 0x%04X", param->pa_interval);
+		return false;
+	}
+
+	CHECKIF(param->num_subgroups != 0 && param->subgroups == NULL) {
+		LOG_DBG("Subgroups are NULL when num_subgroups = %u", param->num_subgroups);
+		return false;
+	}
+
+	CHECKIF(param->num_subgroups > CONFIG_BT_BAP_BASS_MAX_SUBGROUPS) {
+		LOG_DBG("Too many subgroups %u/%u", param->num_subgroups,
+			CONFIG_BT_BAP_BASS_MAX_SUBGROUPS);
+
+		return false;
+	}
+
+	CHECKIF(param->subgroups != NULL) {
+		if (!valid_subgroup_params(param->pa_sync, param->subgroups,
+					   param->num_subgroups)) {
+			return false;
+		}
+	}
+
+	return true;
+}
+
 int bt_bap_broadcast_assistant_mod_src(struct bt_conn *conn,
 				       const struct bt_bap_broadcast_assistant_mod_src_param *param)
 {
@@ -1351,6 +1491,10 @@ int bt_bap_broadcast_assistant_mod_src(struct bt_conn *conn,
 		return -EINVAL;
 	}
 
+	if (!valid_add_mod_param(param)) {
+		return -EINVAL;
+	}
+
 	inst = inst_by_conn(conn);
 	if (inst == NULL) {
 		return -EINVAL;
@@ -1423,22 +1567,18 @@ int bt_bap_broadcast_assistant_mod_src(struct bt_conn *conn,
 
 		subgroup->bis_sync = param->subgroups[i].bis_sync;
 
-		CHECKIF(param->pa_sync == 0 && subgroup->bis_sync != 0) {
-			LOG_DBG("Only syncing to BIS is not allowed");
-
-			atomic_clear_bit(inst->flags, BAP_BA_FLAG_BUSY);
-
-			return -EINVAL;
-		}
-
+#if defined(CONFIG_BT_AUDIO_CODEC_CFG_MAX_METADATA_SIZE)
 		if (param->subgroups[i].metadata_len != 0) {
 			(void)memcpy(subgroup->metadata,
 				     param->subgroups[i].metadata,
 				     param->subgroups[i].metadata_len);
 			subgroup->metadata_len = param->subgroups[i].metadata_len;
 		} else {
-			subgroup->metadata_len = 0;
+			subgroup->metadata_len = 0U;
 		}
+#else
+		subgroup->metadata_len = 0U;
+#endif /* CONFIG_BT_AUDIO_CODEC_CFG_MAX_METADATA_SIZE */
 	}
 
 	return bt_bap_broadcast_assistant_common_cp(conn, &att_buf);