From 585f583ef5e6c2446df7700d8959774771d2a9d8 Mon Sep 17 00:00:00 2001 From: Hui Peng Date: Thu, 11 May 2023 01:10:04 +0000 Subject: [PATCH] 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:ea76b7d99e6366e2043c5621eda630d559104d36) Merged-In: Ia06c9a17f2daa5ce4c32cffa536777f47774cf31 Change-Id: Ia06c9a17f2daa5ce4c32cffa536777f47774cf31 --- system/stack/eatt/eatt.h | 9 +++++++-- system/stack/eatt/eatt_impl.h | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/system/stack/eatt/eatt.h b/system/stack/eatt/eatt.h index 6ef3d3359a..1310f65480 100644 --- a/system/stack/eatt/eatt.h +++ b/system/stack/eatt/eatt.h @@ -17,6 +17,7 @@ #pragma once +#include #include #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 @@ class EattChannel { 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(); + EattChannelSetTxMTU(tx_mtu); } ~EattChannel() { @@ -94,7 +96,10 @@ class EattChannel { } state_ = state; } - void EattChannelSetTxMTU(uint16_t tx_mtu) { this->tx_mtu_ = tx_mtu; } + + void EattChannelSetTxMTU(uint16_t tx_mtu) { + this->tx_mtu_ = std::min(tx_mtu, EATT_MAX_TX_MTU); + } }; /* Interface class */ diff --git a/system/stack/eatt/eatt_impl.h b/system/stack/eatt/eatt_impl.h index 998fc10905..c5a78550ce 100644 --- a/system/stack/eatt/eatt_impl.h +++ b/system/stack/eatt/eatt_impl.h @@ -447,7 +447,7 @@ struct eatt_impl { 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);