commit f91c35c5d8964975f5a46607fa90c5320975c1e4
author Hui Peng <phui@google.com> Thu May 11 01:10:04 2023 +0000
committer Android Build Coastguard Worker <android-build-coastguard-worker@google.com> Fri Sep 01 12:06:23 2023 +0000
tree eb2529667e5267a29bf347cc3c1d6d0b9b006557
parent e090a3dfb8ab5cb7216587daea8ecac8ece7e511

Fix multiple OOB bugs resulted from tx mtu in EATT

The tx mtu in EATT can be controlled by remote device. With malicious
mtu values, it is possible to trigger integer overflow and
OOB write at multiple places (see the bug below).

This fix enforces a max tx mtu in EATT.

Bug: 271335899
Test: manual
Ignore-AOSP-First: security
Tag: #security
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:282d4a182ea6a7d2e6e0f6901d2bc1e75b49e52f)
Merged-In: Ia06c9a17f2daa5ce4c32cffa536777f47774cf31
Change-Id: Ia06c9a17f2daa5ce4c32cffa536777f47774cf31

system/stack/eatt/eatt.h

diff --git a/system/stack/eatt/eatt.h b/system/stack/eatt/eatt.h
index 6ef3d33..1310f65 100644
--- a/system/stack/eatt/eatt.h
+++ b/system/stack/eatt/eatt.h
@@ -17,6 +17,7 @@
 
 #pragma once
 
+#include <algorithm>
 #include <deque>
 
 #include "stack/gatt/gatt_int.h"
@@ -24,6 +25,7 @@
 
 #define EATT_MIN_MTU_MPS (64)
 #define EATT_DEFAULT_MTU (256)
+#define EATT_MAX_TX_MTU  (1024)
 #define EATT_ALL_CIDS (0xFFFF)
 
 namespace bluetooth {
@@ -59,13 +61,13 @@
   EattChannel(RawAddress& bda, uint16_t cid, uint16_t tx_mtu, uint16_t rx_mtu)
       : bda_(bda),
         cid_(cid),
-        tx_mtu_(tx_mtu),
         rx_mtu_(rx_mtu),
         state_(EattChannelState::EATT_CHANNEL_PENDING),
         indicate_handle_(0),
         ind_ack_timer_(NULL),
         ind_confirmation_timer_(NULL) {
     cl_cmd_q_ = std::deque<tGATT_CMD_Q>();
+    EattChannelSetTxMTU(tx_mtu);
   }
 
   ~EattChannel() {
@@ -94,7 +96,10 @@
     }
     state_ = state;
   }
-  void EattChannelSetTxMTU(uint16_t tx_mtu) { this->tx_mtu_ = tx_mtu; }
+
+  void EattChannelSetTxMTU(uint16_t tx_mtu) {
+    this->tx_mtu_ = std::min<uint16_t>(tx_mtu, EATT_MAX_TX_MTU);
+  }
 };
 
 /* Interface class */

system/stack/eatt/eatt_impl.h

diff --git a/system/stack/eatt/eatt_impl.h b/system/stack/eatt/eatt_impl.h
index 998fc10..c5a7855 100644
--- a/system/stack/eatt/eatt_impl.h
+++ b/system/stack/eatt/eatt_impl.h
@@ -447,7 +447,7 @@
     if (is_local_cfg)
       channel->rx_mtu_ = p_cfg->mtu;
     else
-      channel->tx_mtu_ = p_cfg->mtu;
+      channel->EattChannelSetTxMTU(p_cfg->mtu);
 
     /* Go back to open state */
     channel->EattChannelSetState(EattChannelState::EATT_CHANNEL_OPENED);