All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pauli Virtanen <pav@iki.fi>
To: linux-bluetooth@vger.kernel.org
Cc: Pauli Virtanen <pav@iki.fi>
Subject: [PATCH BlueZ] set: don't modify input sirk key in btd_set_add_device()
Date: Sat, 13 Apr 2024 13:04:26 +0300	[thread overview]
Message-ID: <b468e20424d271295c84c72059952acaf65968a5.1713002640.git.pav@iki.fi> (raw)

Currently, btd_set_add_device decrypts the sirk in-place, modifying the
key passed to it.

This causes store_sirk() later on to save the wrong (decrypted) key
value, resulting to invalid duplicate device set.

It also allows devices->sirk list to contain same set multiple times,
which crashes later on as sirks-set are assumed to be 1-to-1 in
btd_set_add/remove_device().

Fixes:
=======================================================================
ERROR: AddressSanitizer: heap-use-after-free on address 0x60600001c068
READ of size 8 at 0x60600001c068 thread T0
    #0 0x762721 in btd_set_remove_device src/set.c:347
    #1 0x7341e7 in remove_sirk_info src/device.c:7145
    #2 0x7f2cee in queue_foreach src/shared/queue.c:207
    #3 0x734499 in btd_device_unref src/device.c:7159
    #4 0x719f65 in device_remove src/device.c:4788
    #5 0x682382 in adapter_remove src/adapter.c:6959
    ...
0x60600001c068 is located 40 bytes inside of 56-byte region [0x60600001c040,0x60600001c078)
freed by thread T0 here:
    #1 0x7605a6 in set_free src/set.c:170
    #2 0x7d4eff in remove_interface gdbus/object.c:660
    #3 0x7dcbb3 in g_dbus_unregister_interface gdbus/object.c:1394
    #4 0x762990 in btd_set_remove_device src/set.c:362
    #5 0x7341e7 in remove_sirk_info src/device.c:7145
    #6 0x7f2cee in queue_foreach src/shared/queue.c:207
    #7 0x734499 in btd_device_unref src/device.c:7159
    #8 0x719f65 in device_remove src/device.c:4788
    #9 0x682382 in adapter_remove src/adapter.c:6959
    ...
previously allocated by thread T0 here:
    #1 0x7f5429 in util_malloc src/shared/util.c:46
    #2 0x7605f1 in set_new src/set.c:178
    #3 0x7625b9 in btd_set_add_device src/set.c:324
    #4 0x6f8fc8 in add_set src/device.c:1916
    #5 0x7f2cee in queue_foreach src/shared/queue.c:207
    #6 0x6f982c in device_set_ltk src/device.c:1940
    #7 0x667b97 in load_ltks src/adapter.c:4478
    ...
=======================================================================
---
 src/set.c | 10 +++++++---
 src/set.h |  3 ++-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/set.c b/src/set.c
index ad64fb163..bf35ee403 100644
--- a/src/set.c
+++ b/src/set.c
@@ -171,7 +171,7 @@ static void set_free(void *data)
 }
 
 static struct btd_device_set *set_new(struct btd_device *device,
-					uint8_t sirk[16], uint8_t size)
+					const uint8_t sirk[16], uint8_t size)
 {
 	struct btd_device_set *set;
 
@@ -206,7 +206,7 @@ static struct btd_device_set *set_new(struct btd_device *device,
 }
 
 static struct btd_device_set *set_find(struct btd_device *device,
-						uint8_t sirk[16])
+						const uint8_t sirk[16])
 {
 	struct btd_adapter *adapter = device_get_adapter(device);
 	const struct queue_entry *entry;
@@ -295,10 +295,14 @@ static void foreach_device(struct btd_device *device, void *data)
 }
 
 struct btd_device_set *btd_set_add_device(struct btd_device *device,
-						uint8_t *key, uint8_t sirk[16],
+						const uint8_t *key,
+						const uint8_t sirk_value[16],
 						uint8_t size)
 {
 	struct btd_device_set *set;
+	uint8_t sirk[16];
+
+	memcpy(sirk, sirk_value, sizeof(sirk));
 
 	/* In case key has been set it means SIRK is encrypted */
 	if (key) {
diff --git a/src/set.h b/src/set.h
index 67177e8c7..2307218c4 100644
--- a/src/set.h
+++ b/src/set.h
@@ -13,7 +13,8 @@
 struct btd_device_set;
 
 struct btd_device_set *btd_set_add_device(struct btd_device *device,
-						uint8_t *ltk, uint8_t sirk[16],
+						const uint8_t *ltk,
+						const uint8_t sirk[16],
 						uint8_t size);
 bool btd_set_remove_device(struct btd_device_set *set,
 						struct btd_device *device);
-- 
2.44.0


             reply	other threads:[~2024-04-13 10:04 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-13 10:04 Pauli Virtanen [this message]
2024-04-13 11:38 ` [BlueZ] set: don't modify input sirk key in btd_set_add_device() bluez.test.bot
2024-04-16 15:40 ` [PATCH BlueZ] " patchwork-bot+bluetooth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b468e20424d271295c84c72059952acaf65968a5.1713002640.git.pav@iki.fi \
    --to=pav@iki.fi \
    --cc=linux-bluetooth@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.