commit 83072e7ea62f10112bab31143f6a7f5c1ba78a37
author Brian Delwiche <delwiche@google.com> Tue Apr 18 23:58:50 2023 +0000
committer Android Build Coastguard Worker <android-build-coastguard-worker@google.com> Fri Sep 01 12:06:40 2023 +0000
tree db83fc3b15467372c41dc52206e76a1bddf011b6
parent 29954462e6b2c61ef9ebd275742ace44407562eb

Fix integer overflow in build_read_multi_rsp

Local variables tracking structure size in build_read_multi_rsp are of
uint16 type but accept a full uint16 range from function arguments while
appending a fixed-length offset.  This can lead to an integer overflow
and unexpected behavior.

Change the locals to size_t, and add a check during reasssignment.

Bug: 273966636
Test: atest bluetooth_test_gd_unit, net_test_stack_btm
Tag: #security
Ignore-AOSP-First: Security
(cherry picked from commit 70a4d628fa016a9487fae07f211644b95e1f0000)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:badb8ffce06b517cbcfdbfa68cb7b7e02d22494a)
Merged-In: I3a74bdb0d003cb6bf4f282615be8c68836676715
Change-Id: I3a74bdb0d003cb6bf4f282615be8c68836676715

system/stack/gatt/gatt_sr.cc

diff --git a/system/stack/gatt/gatt_sr.cc b/system/stack/gatt/gatt_sr.cc
index 9f48d83..f2a3e22 100644
--- a/system/stack/gatt/gatt_sr.cc
+++ b/system/stack/gatt/gatt_sr.cc
@@ -142,7 +142,8 @@
 }
 
 static void build_read_multi_rsp(tGATT_SR_CMD* p_cmd, uint16_t mtu) {
-  uint16_t ii, total_len, len;
+  uint16_t ii;
+  size_t total_len, len;
   uint8_t* p;
   bool is_overflow = false;
 
@@ -187,7 +188,7 @@
         len = p_rsp->attr_value.len - (total_len - mtu);
         is_overflow = true;
         VLOG(1) << StringPrintf(
-            "multi read overflow available len=%d val_len=%d", len,
+            "multi read overflow available len=%zu val_len=%d", len,
             p_rsp->attr_value.len);
       } else {
         len = p_rsp->attr_value.len;
@@ -199,9 +200,15 @@
       }
 
       if (p_rsp->attr_value.handle == p_cmd->multi_req.handles[ii]) {
-        memcpy(p, p_rsp->attr_value.value, len);
-        if (!is_overflow) p += len;
-        p_buf->len += len;
+        // check for possible integer overflow
+        if (p_buf->len + len <= UINT16_MAX) {
+          memcpy(p, p_rsp->attr_value.value, len);
+          if (!is_overflow) p += len;
+          p_buf->len += len;
+        } else {
+          p_cmd->status = GATT_NOT_FOUND;
+          break;
+        }
       } else {
         p_cmd->status = GATT_NOT_FOUND;
         break;