diff options
author | Hui Peng <phui@google.com> | 2023-11-29 00:33:02 +0000 |
---|---|---|
committer | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2023-12-11 21:38:32 +0000 |
commit | 13ccfe816fe12f5235ec7a61f1b92d8c3bfbea9b (patch) | |
tree | b6d5180220ecbb68d748f0bed7c090222470dd6b | |
parent | 0b7ea93a0772030530a005134f03f684a49e7e78 (diff) | |
download | bt-13ccfe816fe12f5235ec7a61f1b92d8c3bfbea9b.tar.gz |
Fix an OOB write bug in attp_build_value_cmd
This is a backport of Ie16251c3a2b7c0f807ecb53bbf125d1e8c276e48
Bug: 295887535
Test: m com.android.btservices
Ignore-AOSP-First: security
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:ddca760763a434dc3067fa47be4a4f1d7200cb7d)
Merged-In: Ibc28bc537eaf4c7369fdd35d8235667ddf2f4cdd
Change-Id: Ibc28bc537eaf4c7369fdd35d8235667ddf2f4cdd
-rw-r--r-- | stack/gatt/att_protocol.cc | 54 |
1 files changed, 43 insertions, 11 deletions
diff --git a/stack/gatt/att_protocol.cc b/stack/gatt/att_protocol.cc index 5d3d4a818..c6e24b8f1 100644 --- a/stack/gatt/att_protocol.cc +++ b/stack/gatt/att_protocol.cc @@ -272,46 +272,78 @@ BT_HDR* attp_build_opcode_cmd(uint8_t op_code) { BT_HDR* attp_build_value_cmd(uint16_t payload_size, uint8_t op_code, uint16_t handle, uint16_t offset, uint16_t len, uint8_t* p_data) { - uint8_t *p, *pp, pair_len, *p_pair_len; + uint8_t *p, *pp, *p_pair_len; + size_t pair_len; + size_t size_now = 1; + + #define CHECK_SIZE() do { \ + if (size_now > payload_size) { \ + LOG(ERROR) << "payload size too small"; \ + osi_free(p_buf); \ + return nullptr; \ + } \ + } while (false) + BT_HDR* p_buf = (BT_HDR*)osi_malloc(sizeof(BT_HDR) + payload_size + L2CAP_MIN_OFFSET); p = pp = (uint8_t*)(p_buf + 1) + L2CAP_MIN_OFFSET; + + CHECK_SIZE(); UINT8_TO_STREAM(p, op_code); p_buf->offset = L2CAP_MIN_OFFSET; - p_buf->len = 1; if (op_code == GATT_RSP_READ_BY_TYPE) { p_pair_len = p; pair_len = len + 2; - UINT8_TO_STREAM(p, pair_len); - p_buf->len += 1; + size_now += 1; + CHECK_SIZE(); + // this field will be backfilled in the end of this function } if (op_code != GATT_RSP_READ_BLOB && op_code != GATT_RSP_READ) { + size_now += 2; + CHECK_SIZE(); UINT16_TO_STREAM(p, handle); - p_buf->len += 2; } if (op_code == GATT_REQ_PREPARE_WRITE || op_code == GATT_RSP_PREPARE_WRITE) { + size_now += 2; + CHECK_SIZE(); UINT16_TO_STREAM(p, offset); - p_buf->len += 2; } - if (len > 0 && p_data != NULL) { + if (len > 0 && p_data != NULL && payload_size > size_now) { /* ensure data not exceed MTU size */ - if (payload_size - p_buf->len < len) { - len = payload_size - p_buf->len; + if (payload_size - size_now < len) { + len = payload_size - size_now; /* update handle value pair length */ - if (op_code == GATT_RSP_READ_BY_TYPE) *p_pair_len = (len + 2); + if (op_code == GATT_RSP_READ_BY_TYPE) { + pair_len = (len + 2); + } LOG(WARNING) << StringPrintf( "attribute value too long, to be truncated to %d", len); } + size_now += len; + CHECK_SIZE(); ARRAY_TO_STREAM(p, p_data, len); - p_buf->len += len; } + // backfill pair len field + if (op_code == GATT_RSP_READ_BY_TYPE) { + if (pair_len > UINT8_MAX) { + LOG(ERROR) << "pair_len greater than" << UINT8_MAX; + osi_free(p_buf); + return nullptr; + } + + *p_pair_len = (uint8_t) pair_len; + } + + #undef CHECK_SIZE + + p_buf->len = (uint16_t) size_now; return p_buf; } |