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 <iam@sung-woo.kim>
This commit is contained in:
Sungwoo Kim 2024-10-10 00:36:37 +00:00 committed by Alberto Escolar
commit 88de2711ca

View file

@ -111,6 +111,9 @@ static int32_t hci_packet_complete(const uint8_t *buf, uint16_t buf_len)
switch (type) { switch (type) {
case BT_HCI_H4_CMD: { 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; const struct bt_hci_cmd_hdr *cmd = (const struct bt_hci_cmd_hdr *)hdr;
/* Parameter Total Length */ /* Parameter Total Length */
@ -119,6 +122,9 @@ static int32_t hci_packet_complete(const uint8_t *buf, uint16_t buf_len)
break; break;
} }
case BT_HCI_H4_ACL: { 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; const struct bt_hci_acl_hdr *acl = (const struct bt_hci_acl_hdr *)hdr;
/* Data Total Length */ /* Data Total Length */
@ -127,6 +133,9 @@ static int32_t hci_packet_complete(const uint8_t *buf, uint16_t buf_len)
break; break;
} }
case BT_HCI_H4_SCO: { 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; const struct bt_hci_sco_hdr *sco = (const struct bt_hci_sco_hdr *)hdr;
/* Data_Total_Length */ /* Data_Total_Length */
@ -135,6 +144,9 @@ static int32_t hci_packet_complete(const uint8_t *buf, uint16_t buf_len)
break; break;
} }
case BT_HCI_H4_EVT: { 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; const struct bt_hci_evt_hdr *evt = (const struct bt_hci_evt_hdr *)hdr;
/* Parameter Total Length */ /* Parameter Total Length */
@ -143,6 +155,9 @@ static int32_t hci_packet_complete(const uint8_t *buf, uint16_t buf_len)
break; break;
} }
case BT_HCI_H4_ISO: { 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; const struct bt_hci_iso_hdr *iso = (const struct bt_hci_iso_hdr *)hdr;
/* ISO_Data_Load_Length parameter */ /* 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 */ /* Request more data */
if (buf_len < header_len || buf_len - header_len < payload_len) { if (buf_len < header_len + payload_len) {
return 0; return 0;
} }