Skip to content

Commit

Permalink
Bluetooth: userchan: fix buffer overflow in hci_packet_complete()
Browse files Browse the repository at this point in the history
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)
    zephyrproject-rtos#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)
    zephyrproject-rtos#2 0x4dcc22  (/root/zephyr.exe+0x4dcc22)

Thread T2 created by T1 here:
    #0 0x48c17c  (/root/zephyr.exe+0x48c17c)
    #1 0x530192  (/root/zephyr.exe+0x530192)
    zephyrproject-rtos#2 0x4dcc22  (/root/zephyr.exe+0x4dcc22)

Thread T1 created by T0 here:
    #0 0x48c17c  (/root/zephyr.exe+0x48c17c)
    #1 0x52f36c  (/root/zephyr.exe+0x52f36c)
    zephyrproject-rtos#2 0x5371dc  (/root/zephyr.exe+0x5371dc)
    zephyrproject-rtos#3 0x5312a6  (/root/zephyr.exe+0x5312a6)
    zephyrproject-rtos#4 0x52ed7b  (/root/zephyr.exe+0x52ed7b)
    zephyrproject-rtos#5 0x52eddd  (/root/zephyr.exe+0x52eddd)
    zephyrproject-rtos#6 0x7ffff6083c89  (/lib/x86_64-linux-gnu/libc.so.6+0x27c89)
(BuildId: 2e01923fea4ad9f7fa50fe24e0f3385a45a6cd1c)

==5==ABORTING

Signed-off-by: Sungwoo Kim <[email protected]>
  • Loading branch information
swkim101 authored and aescolar committed Oct 25, 2024
1 parent 98805a2 commit 88de271
Showing 1 changed file with 16 additions and 1 deletion.
17 changes: 16 additions & 1 deletion drivers/bluetooth/hci/userchan.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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 */
Expand All @@ -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 */
Expand All @@ -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 */
Expand All @@ -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 */
Expand All @@ -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;
}

Expand Down

0 comments on commit 88de271

Please sign in to comment.