From 7ba60a6c50d951885622584b4a8d964d51f3d10c Mon Sep 17 00:00:00 2001 From: Sungwoo Kim Date: Thu, 10 Oct 2024 00:36:37 +0000 Subject: [PATCH] Bluetooth: userchan: fix buffer overflow in hci_packet_complete() hci_packet_complete(buf, buf_size) should check whether buf_size is enough. For instance, hci_packet_complete can receive buf with buf_size 1, leading to the buffer overflow in cmd->param_len, which is buf[3]. This can happen when rx_thread() receives two frames in 512 bytes and the first frame size is 511. Then, rx_thread() will call hci_packet_complete() with 1. ==5==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000000ad81c2 at pc 0x0000005279b3 bp 0x7fffe74f5b70 sp 0x7fffe74f5b68 READ of size 2 at 0x000000ad81c2 thread T6 #0 0x5279b2 (/root/zephyr.exe+0x5279b2) #1 0x4d697d (/root/zephyr.exe+0x4d697d) #2 0x7ffff60e5daa (/lib/x86_64-linux-gnu/libc.so.6+0x89daa) (BuildId: 2e01923fea4ad9f7fa50fe24e0f3385a45a6cd1c) 0x000000ad81c2 is located 2 bytes to the right of global variable 'rx_thread.frame' defined in 'zephyr/drivers/bluetooth/hci/userchan.c' (0xad7fc0) of size 512 SUMMARY: AddressSanitizer: global-buffer-overflow (/root/zephyr.exe+0x5279b2) Thread T6 created by T2 here: #0 0x48c17c (/root/zephyr.exe+0x48c17c) #1 0x530192 (/root/zephyr.exe+0x530192) #2 0x4dcc22 (/root/zephyr.exe+0x4dcc22) Thread T2 created by T1 here: #0 0x48c17c (/root/zephyr.exe+0x48c17c) #1 0x530192 (/root/zephyr.exe+0x530192) #2 0x4dcc22 (/root/zephyr.exe+0x4dcc22) Thread T1 created by T0 here: #0 0x48c17c (/root/zephyr.exe+0x48c17c) #1 0x52f36c (/root/zephyr.exe+0x52f36c) #2 0x5371dc (/root/zephyr.exe+0x5371dc) #3 0x5312a6 (/root/zephyr.exe+0x5312a6) #4 0x52ed7b (/root/zephyr.exe+0x52ed7b) #5 0x52eddd (/root/zephyr.exe+0x52eddd) #6 0x7ffff6083c89 (/lib/x86_64-linux-gnu/libc.so.6+0x27c89) (BuildId: 2e01923fea4ad9f7fa50fe24e0f3385a45a6cd1c) ==5==ABORTING Signed-off-by: Sungwoo Kim --- drivers/bluetooth/hci/userchan.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/drivers/bluetooth/hci/userchan.c b/drivers/bluetooth/hci/userchan.c index b3d401960df11cd..aedb71a8430c5e8 100644 --- a/drivers/bluetooth/hci/userchan.c +++ b/drivers/bluetooth/hci/userchan.c @@ -111,6 +111,9 @@ static int32_t hci_packet_complete(const uint8_t *buf, uint16_t buf_len) switch (type) { case BT_HCI_H4_CMD: { + if (buf_len < header_len + BT_HCI_CMD_HDR_SIZE) { + return 0; + } const struct bt_hci_cmd_hdr *cmd = (const struct bt_hci_cmd_hdr *)hdr; /* Parameter Total Length */ @@ -119,6 +122,9 @@ static int32_t hci_packet_complete(const uint8_t *buf, uint16_t buf_len) break; } case BT_HCI_H4_ACL: { + if (buf_len < header_len + BT_HCI_ACL_HDR_SIZE) { + return 0; + } const struct bt_hci_acl_hdr *acl = (const struct bt_hci_acl_hdr *)hdr; /* Data Total Length */ @@ -127,6 +133,9 @@ static int32_t hci_packet_complete(const uint8_t *buf, uint16_t buf_len) break; } case BT_HCI_H4_SCO: { + if (buf_len < header_len + BT_HCI_SCO_HDR_SIZE) { + return 0; + } const struct bt_hci_sco_hdr *sco = (const struct bt_hci_sco_hdr *)hdr; /* Data_Total_Length */ @@ -135,6 +144,9 @@ static int32_t hci_packet_complete(const uint8_t *buf, uint16_t buf_len) break; } case BT_HCI_H4_EVT: { + if (buf_len < header_len + BT_HCI_EVT_HDR_SIZE) { + return 0; + } const struct bt_hci_evt_hdr *evt = (const struct bt_hci_evt_hdr *)hdr; /* Parameter Total Length */ @@ -143,6 +155,9 @@ static int32_t hci_packet_complete(const uint8_t *buf, uint16_t buf_len) break; } case BT_HCI_H4_ISO: { + if (buf_len < header_len + BT_HCI_ISO_HDR_SIZE) { + return 0; + } const struct bt_hci_iso_hdr *iso = (const struct bt_hci_iso_hdr *)hdr; /* ISO_Data_Load_Length parameter */ @@ -157,7 +172,7 @@ static int32_t hci_packet_complete(const uint8_t *buf, uint16_t buf_len) } /* Request more data */ - if (buf_len < header_len || buf_len - header_len < payload_len) { + if (buf_len < header_len + payload_len) { return 0; }