[BlueZ,1/1] btdev: Fix set PA data array overflow

Commit Message

Iulia Tanasescu Feb. 13, 2024, 3:58 p.m. UTC

This fixes an array overflow that can happen if the user issues the
LE Set Periodic Advertising Data command with data length exceeding
31 bytes.

The PA data set by the user is copied in an array of fixed length
(31 bytes). However, the data length might exceed 31 bytes. This will
cause an array overflow when the PA data is later processed (for
instance, when sending PA reports).

According to specification, the data length provided at LE Set Periodic
Advertising Data command can be maximum 252 bytes. The stored data len
should also be true to the length copied in the array.
---
 emulator/btdev.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

Comments

[e-mail] Feb. 13, 2024, 5:10 p.m. UTC | #1

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=825705

---Test result---

Test Summary:
CheckPatch                    PASS      0.31 seconds
GitLint                       PASS      0.22 seconds
BuildEll                      PASS      23.97 seconds
BluezMake                     PASS      716.04 seconds
MakeCheck                     PASS      11.60 seconds
MakeDistcheck                 PASS      162.51 seconds
CheckValgrind                 PASS      228.35 seconds
CheckSmatch                   WARNING   328.25 seconds
bluezmakeextell               PASS      106.95 seconds
IncrementalBuild              PASS      658.05 seconds
ScanBuild                     WARNING   947.23 seconds

Details
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
emulator/btdev.c:422:29: warning: Variable length array is used.
##############################
Test: ScanBuild - WARNING
Desc: Run Scan Build
Output:
emulator/btdev.c:1086:10: warning: Although the value stored to 'conn' is used in the enclosing expression, the value is never actually read from 'conn'
        while ((conn = queue_find(dev->conns, match_handle,
                ^      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
emulator/btdev.c:1369:24: warning: Access to field 'link' results in a dereference of a null pointer (loaded from variable 'conn')
        pending_conn_del(dev, conn->link->dev);
                              ^~~~~~~~~~
emulator/btdev.c:1491:13: warning: Access to field 'dev' results in a dereference of a null pointer (loaded from variable 'conn')
        send_event(conn->dev, BT_HCI_EVT_AUTH_COMPLETE, &ev, sizeof(ev));
                   ^~~~~~~~~
3 warnings generated.



---
Regards,
Linux Bluetooth

Patch

diff --git a/emulator/btdev.c b/emulator/btdev.c
index 4ddbae403..4c9f5d181 100644
--- a/emulator/btdev.c
+++ b/emulator/btdev.c
@@ -5,7 +5,7 @@ 
  *
  *  Copyright (C) 2011-2012  Intel Corporation
  *  Copyright (C) 2004-2010  Marcel Holtmann <[e-mail]>
- *  Copyright 2023 NXP
+ *  Copyright 2023-2024 NXP
  *
  *
  */
@@ -44,6 +44,8 @@ 
 #define BIS_SIZE		3
 #define CIG_SIZE		3
 
+#define MAX_PA_DATA_LEN	252
+
 #define has_bredr(btdev)	(!((btdev)->features[4] & 0x20))
 #define has_le(btdev)		(!!((btdev)->features[4] & 0x40))
 
@@ -207,7 +209,7 @@  struct btdev {
 	uint16_t le_pa_min_interval;
 	uint16_t le_pa_max_interval;
 	uint8_t  le_pa_data_len;
-	uint8_t  le_pa_data[31];
+	uint8_t  le_pa_data[MAX_PA_DATA_LEN];
 	struct bt_hci_cmd_le_pa_create_sync pa_sync_cmd;
 	uint16_t le_pa_sync_handle;
 	uint8_t  big_handle;
@@ -5210,9 +5212,13 @@  static int cmd_set_pa_data(struct btdev *dev, const void *data,
 {
 	const struct bt_hci_cmd_le_set_pa_data *cmd = data;
 	uint8_t status = BT_HCI_ERR_SUCCESS;
+	uint8_t data_len = cmd->data_len;
+
+	if (data_len > MAX_PA_DATA_LEN)
+		data_len = MAX_PA_DATA_LEN;
 
-	dev->le_pa_data_len = cmd->data_len;
-	memcpy(dev->le_pa_data, cmd->data, 31);
+	dev->le_pa_data_len = data_len;
+	memcpy(dev->le_pa_data, cmd->data, data_len);
 	cmd_complete(dev, BT_HCI_CMD_LE_SET_PA_DATA, &status,
 							sizeof(status));