[BlueZ 1/2] adv_monitor: Fix buffer overflow caused by integer overflow

Bastien Nocera <[email protected]>
Newsgroups org.kernel.vger.linux-bluetooth
Message-ID <[email protected]>
In src/adv_monitor.c, merged_pattern_send_add_pattern(), and with a
large merged_pattern->patterns list, it's possible to overflow cp_len,
an 8-bit integer:
pattern_count = queue_length(merged_pattern->patterns);
cp_len = sizeof(*cp) + pattern_count * sizeof(struct mgmt_adv_pattern);

Eight patterns require 273 bytes of command storage, but assigning
that result to uint8_t cp_len wraps it to 17 before allocation.

The loop that follows does not use the truncated size; it still copies
all eight 34-byte struct mgmt_adv_pattern records into the heap object.
The mismatch between the wrapped allocation size and the full copy
volume produces a large, deterministic heap overwrite.

Fix this in 2 ways in the function itself:
1) increase the size of cp_len, as 8 patterns would overflow it, and some
   typical Bluetooth devices can support 16 patterns
2) Store the result of the multiplication in a 64-bit integer before
   checking whether it's bigger than our 16-bit cp_len

A similar bug exists in merged_pattern_send_add_pattern_rssi().

Reported-by: @ax-nnlabs (for merged_pattern_send_add_pattern())
Reported-by: Aisle Research (for merged_pattern_send_add_pattern_rssi())
---
 src/adv_monitor.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/src/adv_monitor.c b/src/adv_monitor.c
index 87fa824b35d8..7f437a0e7c8f 100644
--- a/src/adv_monitor.c
+++ b/src/adv_monitor.c
@@ -1106,12 +1106,15 @@ static bool merged_pattern_send_add_pattern(
 			struct adv_monitor_merged_pattern *merged_pattern)
 {
 	struct mgmt_cp_add_adv_monitor *cp = NULL;
-	uint8_t pattern_count, cp_len;
+	uint64_t cp_len;
+	uint8_t pattern_count;
 	const struct queue_entry *e;
 	bool success = true;
 
 	pattern_count = queue_length(merged_pattern->patterns);
 	cp_len = sizeof(*cp) + pattern_count * sizeof(struct mgmt_adv_pattern);
+	if (cp_len > UINT16_MAX)
+		return false;
 
 	cp = malloc0(cp_len);
 	if (!cp)
@@ -1126,8 +1129,9 @@ static bool merged_pattern_send_add_pattern(
 
 	if (!mgmt_send(merged_pattern->manager->mgmt,
 			MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
-			merged_pattern->manager->adapter_id, cp_len, cp,
-			add_adv_patterns_monitor_cb, merged_pattern, NULL)) {
+			merged_pattern->manager->adapter_id, (uint16_t) cp_len,
+			cp, add_adv_patterns_monitor_cb, merged_pattern,
+			NULL)) {
 		error("Unable to send Add Adv Patterns Monitor command");
 		success = false;
 	}
@@ -1141,12 +1145,15 @@ static bool merged_pattern_send_add_pattern_rssi(
 			struct adv_monitor_merged_pattern *merged_pattern)
 {
 	struct mgmt_cp_add_adv_patterns_monitor_rssi *cp = NULL;
-	uint8_t pattern_count, cp_len;
+	uint64_t cp_len;
+	uint8_t pattern_count;
 	const struct queue_entry *e;
 	bool success = true;
 
 	pattern_count = queue_length(merged_pattern->patterns);
 	cp_len = sizeof(*cp) + pattern_count * sizeof(struct mgmt_adv_pattern);
+	if (cp_len > UINT16_MAX)
+		return false;
 
 	cp = malloc0(cp_len);
 	if (!cp)
@@ -1169,8 +1176,9 @@ static bool merged_pattern_send_add_pattern_rssi(
 
 	if (!mgmt_send(merged_pattern->manager->mgmt,
 			MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI,
-			merged_pattern->manager->adapter_id, cp_len, cp,
-			add_adv_patterns_monitor_cb, merged_pattern, NULL)) {
+			merged_pattern->manager->adapter_id, (uint16_t) cp_len,
+			cp, add_adv_patterns_monitor_cb, merged_pattern,
+			NULL)) {
 		error("Unable to send Add Adv Patterns Monitor RSSI command");
 		success = false;
 	}
-- 
2.55.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.