Skip to content

Commit

Permalink
Fix OOB writes in gatt_sr.cc
Browse files Browse the repository at this point in the history
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:61f6c95083aa98c597f1fdf7c871dd826e810f2b)
Merged-In: I9b30499d4aed6ab42f3cdb2c0de7df2c1a827404
Change-Id: I9b30499d4aed6ab42f3cdb2c0de7df2c1a827404
  • Loading branch information
Brian Delwiche authored and aoleary committed Dec 30, 2024
1 parent c2a956b commit 2d3b891
Showing 1 changed file with 21 additions and 0 deletions.
21 changes: 21 additions & 0 deletions system/stack/gatt/gatt_sr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,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);
Expand Down Expand Up @@ -768,6 +773,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);

Expand Down Expand Up @@ -902,6 +913,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;
Expand Down Expand Up @@ -1049,6 +1065,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;

Expand Down

0 comments on commit 2d3b891

Please sign in to comment.