From c7468e64bb5e821563a910ccd8e5693c179c9da4 Mon Sep 17 00:00:00 2001
From: Brian Delwiche <delwiche@google.com>
Date: Thu, 12 Sep 2024 17:26:55 +0000
Subject: [PATCH] Fix OOB writes in gatt_sr.cc
At various points in gatt_sr.cc, the output of the
gatt_tcb_get_payload_size function is used without checking for a
positive length. However, in exceptional cases it is possible for the
channel to be closed at the time the function is called, which will lead
to a zero length and cause an OOB write in subsequent processing.
Fix all of these.
Bug: 364026473
Bug: 364027038
Bug: 364027949
Bug: 364025411
Test: m libbluetooth
Test: researcher POC
Flag: EXEMPT trivial validity checks
Tag: #security
Ignore-AOSP-First: Security
(cherry picked from commit 7de5617f7d5266fe57c990c428621b5d4e92728a)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:130861eadc3d9eda593df949666e561dd1f020fc)
Merged-In: I9b30499d4aed6ab42f3cdb2c0de7df2c1a827404
Change-Id: I9b30499d4aed6ab42f3cdb2c0de7df2c1a827404
---
system/stack/gatt/gatt_sr.cc | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/system/stack/gatt/gatt_sr.cc b/system/stack/gatt/gatt_sr.cc
index 3b3fb342a71..46aa4bb469e 100644
--- a/system/stack/gatt/gatt_sr.cc
+++ b/system/stack/gatt/gatt_sr.cc
@@ -734,6 +734,11 @@ void gatts_process_primary_service_req(tGATT_TCB& tcb, uint16_t cid,
uint16_t payload_size = gatt_tcb_get_payload_size_tx(tcb, cid);
+ // This can happen if the channel is already closed.
+ if (payload_size == 0) {
+ return;
+ }
+
uint16_t msg_len =
(uint16_t)(sizeof(BT_HDR) + payload_size + L2CAP_MIN_OFFSET);
BT_HDR* p_msg = (BT_HDR*)osi_calloc(msg_len);
@@ -769,6 +774,12 @@ static void gatts_process_find_info(tGATT_TCB& tcb, uint16_t cid,
}
uint16_t payload_size = gatt_tcb_get_payload_size_tx(tcb, cid);
+
+ // This can happen if the channel is already closed.
+ if (payload_size == 0) {
+ return;
+ }
+
uint16_t buf_len =
(uint16_t)(sizeof(BT_HDR) + payload_size + L2CAP_MIN_OFFSET);
@@ -909,6 +920,11 @@ static void gatts_process_read_by_type_req(tGATT_TCB& tcb, uint16_t cid,
uint16_t payload_size = gatt_tcb_get_payload_size_tx(tcb, cid);
+ // This can happen if the channel is already closed.
+ if (payload_size == 0) {
+ return;
+ }
+
size_t msg_len = sizeof(BT_HDR) + payload_size + L2CAP_MIN_OFFSET;
BT_HDR* p_msg = (BT_HDR*)osi_calloc(msg_len);
uint8_t* p = (uint8_t*)(p_msg + 1) + L2CAP_MIN_OFFSET;
@@ -1056,6 +1072,11 @@ static void gatts_process_read_req(tGATT_TCB& tcb, uint16_t cid,
uint8_t* p_data) {
uint16_t payload_size = gatt_tcb_get_payload_size_tx(tcb, cid);
+ // This can happen if the channel is already closed.
+ if (payload_size == 0) {
+ return;
+ }
+
size_t buf_len = sizeof(BT_HDR) + payload_size + L2CAP_MIN_OFFSET;
uint16_t offset = 0;
--
GitLab