[PATCH bluez] monitor: Fix stack-buffer-overflow in packet_hci_scodata()

Zijun Hu posted 1 patch an hour ago
monitor/packet.c | 41 +++++++++++++++++++++++++----------------
1 file changed, 25 insertions(+), 16 deletions(-)
[PATCH bluez] monitor: Fix stack-buffer-overflow in packet_hci_scodata()
Posted by Zijun Hu an hour ago
Fix below handle_str[] OOB by:
- Increase buffer size.
- Use snprintf() instead of sprintf().
Also access SCO header fields after validating header size.

AddressSanitizer: stack-buffer-overflow on address 0x7bd1443dedba at pc 0x7bd1474ced69 bp 0x7ffdb0324880 sp 0x7ffdb0324010
WRITE of size 40 at 0x7bd1443dedba thread T0
    #0 0x7bd1474ced68 in vsprintf ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1671
    #1 0x7bd1474d0643 in __sprintf_chk ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1719
    #2 0x5f5c39d02ea7 in sprintf /usr/include/x86_64-linux-gnu/bits/stdio2.h:30
    #3 0x5f5c39d02ea7 in handle_str_append_addr ../monitor/packet.c:14995
    #4 0x5f5c39d58f19 in packet_hci_scodata ../monitor/packet.c:15125
    #5 0x5f5c39d5eaf9 in packet_monitor ../monitor/packet.c:4841
    #6 0x5f5c39cc4094 in data_callback ../monitor/control.c:991
    #7 0x5f5c39e7fa7d in mainloop_run ../src/shared/mainloop.c:104
    #8 0x7bd1474e81d86 in mainloop_run_with_signal ../src/shared/mainloop-notify.c:196
    #9 0x5f5c39cbbb4c in main ../monitor/main.c:315
    #10 0x7bd14662a1c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #11 0x7bd14662a28a in __libc_start_main_impl ../sysdeps/nptl/libc-start.c:360
    #12 0x5f5c39cbc664 in _start (/usr/bin/btmon+0x2b2664) (BuildId: 370a6386c9fceae18f9a456c0c029ac378b0b070)

Fixes: 611f84a0ff0e ("monitor: Annotate ACL/SCO/ISO data with device address")
---
 monitor/packet.c | 41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/monitor/packet.c b/monitor/packet.c
index fc280fef8c57..776d4bb833f8 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -14966,48 +14966,54 @@ static void packet_enqueue_tx(struct timeval *tv, uint16_t handle,
 	frame = new0(struct packet_frame, 1);
 	if (tv)
 		memcpy(&frame->tv, tv, sizeof(*tv));
 	frame->num = num;
 	frame->len = len;
 	queue_push_tail(conn->tx_q, frame);
 }
 
-static void handle_str_append_addr(char *handle_str,
+static void handle_str_append_addr(char *handle_str, size_t handle_str_size,
 					struct packet_conn_data *conn)
 {
+	size_t len;
+
 	if (!conn)
 		return;
 
+	len = strlen(handle_str);
+	if (len >= handle_str_size)
+		return;
+
 	switch (conn->dst_type) {
 	case 0x00:
 	case 0x02:
 		if (conn->dst_oui) {
-			sprintf(handle_str + strlen(handle_str),
+			snprintf(handle_str + len, handle_str_size - len,
 				" [%2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X (%.16s)]",
 				conn->dst[5], conn->dst[4], conn->dst[3],
 				conn->dst[2], conn->dst[1], conn->dst[0],
 				conn->dst_oui);
 			return;
 		}
 		break;
 	case 0x01:
 	case 0x03:
 		if (conn->dst_rtype) {
-			sprintf(handle_str + strlen(handle_str),
+			snprintf(handle_str + len, handle_str_size - len,
 				" [%2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X (%.16s)]",
 				conn->dst[5], conn->dst[4], conn->dst[3],
 				conn->dst[2], conn->dst[1], conn->dst[0],
 				conn->dst_rtype);
 			return;
 		}
 		break;
 	}
 
-	sprintf(handle_str + strlen(handle_str),
+	snprintf(handle_str + len, handle_str_size - len,
 			" [%2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X]",
 			conn->dst[5], conn->dst[4], conn->dst[3],
 			conn->dst[2], conn->dst[1], conn->dst[0]);
 }
 
 void packet_hci_acldata(struct timeval *tv, struct ucred *cred, uint16_t index,
 				bool in, const void *data, uint16_t size)
 {
@@ -15041,22 +15047,22 @@ void packet_hci_acldata(struct timeval *tv, struct ucred *cred, uint16_t index,
 	data += HCI_ACL_HDR_SIZE;
 	size -= HCI_ACL_HDR_SIZE;
 
 	conn = packet_get_conn_data(handle);
 	if (conn && conn->type == 0x01 && index_list[index].le.total)
 		pool = &index_list[index].le;
 
 	if (!in && pool && pool->total)
-		sprintf(handle_str, "Handle %d [%u/%u]", acl_handle(handle),
+		snprintf(handle_str, sizeof(handle_str), "Handle %d [%u/%u]", acl_handle(handle),
 				++pool->tx, pool->total);
 	else
-		sprintf(handle_str, "Handle %d", acl_handle(handle));
+		snprintf(handle_str, sizeof(handle_str), "Handle %d", acl_handle(handle));
 
-	handle_str_append_addr(handle_str, conn);
+	handle_str_append_addr(handle_str, sizeof(handle_str), conn);
 
 	sprintf(extra_str, "flags 0x%2.2x dlen %d", flags, dlen);
 
 	if (conn)
 		sprintf(label, "%s", conn_type_str(conn->type));
 	else
 		sprintf(label, "ACL");
 
@@ -15083,20 +15089,20 @@ void packet_hci_acldata(struct timeval *tv, struct ucred *cred, uint16_t index,
 
 	packet_set_context(NULL, 0);
 }
 
 void packet_hci_scodata(struct timeval *tv, struct ucred *cred, uint16_t index,
 				bool in, const void *data, uint16_t size)
 {
 	const hci_sco_hdr *hdr = data;
-	uint16_t handle = le16_to_cpu(hdr->handle);
-	uint8_t flags = acl_flags(handle);
+	uint16_t handle;
+	uint8_t flags;
 	char label[8];
-	char handle_str[42], extra_str[32];
+	char handle_str[64], extra_str[32];
 	struct packet_conn_data *conn;
 
 	if (index >= MAX_INDEX) {
 		print_field("Invalid index (%d).", index);
 		return;
 	}
 
 	index_list[index].frame++;
@@ -15107,27 +15113,30 @@ void packet_hci_scodata(struct timeval *tv, struct ucred *cred, uint16_t index,
 				"Malformed SCO Data RX packet", NULL, NULL);
 		else
 			print_packet(tv, cred, '*', index, NULL, COLOR_ERROR,
 				"Malformed SCO Data TX packet", NULL, NULL);
 		packet_hexdump(data, size);
 		return;
 	}
 
+	handle = le16_to_cpu(hdr->handle);
+	flags = acl_flags(handle);
+
 	data += HCI_SCO_HDR_SIZE;
 	size -= HCI_SCO_HDR_SIZE;
 	conn = packet_get_conn_data(handle);
 
 	if (index_list[index].sco.total && !in)
-		sprintf(handle_str, "Handle %d [%u/%u]", acl_handle(handle),
+		snprintf(handle_str, sizeof(handle_str), "Handle %d [%u/%u]", acl_handle(handle),
 			index_list[index].sco.total, index_list[index].sco.tx);
 	else
-		sprintf(handle_str, "Handle %d", acl_handle(handle));
+		snprintf(handle_str, sizeof(handle_str), "Handle %d", acl_handle(handle));
 
-	handle_str_append_addr(handle_str, conn);
+	handle_str_append_addr(handle_str, sizeof(handle_str), conn);
 
 	sprintf(extra_str, "flags 0x%2.2x dlen %d", flags, hdr->dlen);
 
 	if (conn)
 		sprintf(label, "%s", conn_type_str(conn->type));
 	else
 		sprintf(label, "SCO");
 
@@ -15222,22 +15231,22 @@ void packet_hci_isodata(struct timeval *tv, struct ucred *cred, uint16_t index,
 	}
 
 	conn = packet_get_conn_data(handle);
 
 	if (in && have_hdr && conn)
 		packet_loss_add(&conn->rx_loss, sn, sflags);
 
 	if (!in && pool->total)
-		sprintf(handle_str, "Handle %d [%u/%u]%s",
+		snprintf(handle_str, sizeof(handle_str), "Handle %d [%u/%u]%s",
 			acl_handle(handle), ++pool->tx, pool->total, sn_str);
 	else
-		sprintf(handle_str, "Handle %u%s", acl_handle(handle), sn_str);
+		snprintf(handle_str, sizeof(handle_str), "Handle %u%s", acl_handle(handle), sn_str);
 
-	handle_str_append_addr(handle_str, conn);
+	handle_str_append_addr(handle_str, sizeof(handle_str), conn);
 
 	sprintf(extra_str, "flags 0x%2.2x dlen %u%s%s", flags, dlen, slen_str,
 									ts_str);
 
 	if (conn)
 		sprintf(label, "%s", conn_type_str(conn->type));
 	else
 		sprintf(label, "ISO");

---
base-commit: 8b4a4176063831476bc9244d20f43b552e4b5554
change-id: 20260924-fix_sco-059769173ee0

Best regards,
--  
Zijun Hu <zijun.hu@oss.qualcomm.com>