diff options
author | Pauli Virtanen <pav@iki.fi> | 2024-04-13 13:04:26 +0300 |
---|---|---|
committer | Luiz Augusto von Dentz <luiz.von.dentz@intel.com> | 2024-04-16 11:30:34 -0400 |
commit | b411b98bf4f51c18c77626f786a4f2b8cdc28982 (patch) | |
tree | f3e083f93b1ca9e545f4d777969fe17f71fb2ccc | |
parent | d3a6a6459cbda91693106fb8d43de319b334a3a4 (diff) |
set: don't modify input sirk key in btd_set_add_device()
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
...
=======================================================================
-rw-r--r-- | src/set.c | 10 | ||||
-rw-r--r-- | src/set.h | 3 |
2 files changed, 9 insertions, 4 deletions
@@ -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) { @@ -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); |