Skip to content

Commit

Permalink
Fix multiple OOB bugs resulted from tx mtu in EATT
Browse files Browse the repository at this point in the history
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
  • Loading branch information
benquike authored and thestinger committed Sep 6, 2023
1 parent ce2776f commit 585f583
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 3 deletions.
9 changes: 7 additions & 2 deletions system/stack/eatt/eatt.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@

#pragma once

#include <algorithm>
#include <deque>

#include "stack/gatt/gatt_int.h"
#include "types/raw_address.h"

#define EATT_MIN_MTU_MPS (64)
#define EATT_DEFAULT_MTU (256)
#define EATT_MAX_TX_MTU (1024)
#define EATT_ALL_CIDS (0xFFFF)

namespace bluetooth {
Expand Down Expand Up @@ -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<tGATT_CMD_Q>();
EattChannelSetTxMTU(tx_mtu);
}

~EattChannel() {
Expand Down Expand Up @@ -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<uint16_t>(tx_mtu, EATT_MAX_TX_MTU);
}
};

/* Interface class */
Expand Down
2 changes: 1 addition & 1 deletion system/stack/eatt/eatt_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 585f583

Please sign in to comment.