Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 7f5e6613 authored by Martin Brabham's avatar Martin Brabham Committed by Automerger Merge Worker
Browse files

Security Fix: Crafted GATT request causes BT stack crash am: 61570b9d

Original change: https://googleplex-android-review.googlesource.com/c/platform/system/bt/+/15717436

Change-Id: Ifacad9537f67f1d6525203d8799479bf868db6de
parents 5d53922d 61570b9d
Loading
Loading
Loading
Loading
+82 −40
Original line number Diff line number Diff line
@@ -609,6 +609,7 @@ void gatt_process_prep_write_rsp(tGATT_TCB& tcb, tGATT_CLCB* p_clcb,
    gatt_end_operation(p_clcb, p_clcb->status, &value);
  }
}

/*******************************************************************************
 *
 * Function         gatt_process_notification
@@ -620,10 +621,10 @@ void gatt_process_prep_write_rsp(tGATT_TCB& tcb, tGATT_CLCB* p_clcb,
 ******************************************************************************/
void gatt_process_notification(tGATT_TCB& tcb, uint16_t cid, uint8_t op_code,
                               uint16_t len, uint8_t* p_data) {
  tGATT_VALUE value;
  tGATT_VALUE value = {};
  tGATT_REG* p_reg;
  uint16_t conn_id;
  tGATT_STATUS encrypt_status;
  tGATT_STATUS encrypt_status = {};
  uint8_t* p = p_data;
  uint8_t i;
  tGATTC_OPTYPE event = (op_code == GATT_HANDLE_VALUE_IND)
@@ -632,34 +633,52 @@ void gatt_process_notification(tGATT_TCB& tcb, uint16_t cid, uint8_t op_code,

  VLOG(1) << __func__;

  // Ensure our packet has enough data (2 bytes)
  if (len < GATT_NOTIFICATION_MIN_LEN) {
    LOG(ERROR) << "illegal notification PDU length, discard";
    return;
  }

  memset(&value, 0, sizeof(value));
  // Get 2 byte handle
  STREAM_TO_UINT16(value.handle, p);

  // Fail early if the GATT handle is not valid
  if (!GATT_HANDLE_IS_VALID(value.handle)) {
    /* illegal handle, send ack now */
    if (op_code == GATT_HANDLE_VALUE_IND)
      attp_send_cl_confirmation_msg(tcb, cid);
    return;
  }

  // Calculate value length based on opcode
  if (op_code == GATT_HANDLE_MULTI_VALUE_NOTIF) {
    // Ensure our packet has enough data; MIN + 2 more bytes for len value
    if (len < GATT_NOTIFICATION_MIN_LEN + 2) {
      LOG(ERROR) << "illegal notification PDU length, discard";
      return;
    }

    // Allow multi value opcode to set value len from the packet
    STREAM_TO_UINT16(value.len, p);

    if (value.len > len - 4) {
      LOG(ERROR) << "value.len (" << value.len << ") greater than length ("
                 << (len - 4);
      return;
    }

  } else {
    // For single value, just use the passed in len minus opcode length (2)
    value.len = len - 2;
  }

  // Verify the new calculated length
  if (value.len > GATT_MAX_ATTR_LEN) {
    LOG(ERROR) << "value.len larger than GATT_MAX_ATTR_LEN, discard";
    return;
  }

  STREAM_TO_ARRAY(value.value, p, value.len);

  if (!GATT_HANDLE_IS_VALID(value.handle)) {
    /* illegal handle, send ack now */
    if (op_code == GATT_HANDLE_VALUE_IND)
      attp_send_cl_confirmation_msg(tcb, cid);
    return;
  }

  // Handle indications differently
  if (event == GATTC_OPTYPE_INDICATION) {
    if (tcb.ind_count) {
      /* this is an error case that receiving an indication but we
@@ -670,33 +689,30 @@ void gatt_process_notification(tGATT_TCB& tcb, uint16_t cid, uint8_t op_code,
      LOG(ERROR) << __func__ << " rcv Ind. but ind_count=" << tcb.ind_count
                 << " (will reset ind_count)";
    }
    tcb.ind_count = 0;
  }

  /* should notify all registered client with the handle value
     notificaion/indication
     Note: need to do the indication count and start timer first then do
     callback
   */
    // Zero out the ind_count
    tcb.ind_count = 0;

    // Notify all registered clients with the handle value
    // notification/indication
    // Note: need to do the indication count and start timer first then do
    // callback
    for (i = 0, p_reg = gatt_cb.cl_rcb; i < GATT_MAX_APPS; i++, p_reg++) {
    if (p_reg->in_use && p_reg->app_cb.p_cmpl_cb &&
        (event == GATTC_OPTYPE_INDICATION))
      tcb.ind_count++;
      if (p_reg->in_use && p_reg->app_cb.p_cmpl_cb) tcb.ind_count++;
    }

  if (event == GATTC_OPTYPE_INDICATION) {
    /* start a timer for app confirmation */
    if (tcb.ind_count > 0)
    if (tcb.ind_count > 0) {
      gatt_start_ind_ack_timer(tcb, cid);
    else /* no app to indicate, or invalid handle */
    } else { /* no app to indicate, or invalid handle */
      attp_send_cl_confirmation_msg(tcb, cid);
    }
  }

  encrypt_status = gatt_get_link_encrypt_status(tcb);

  uint16_t rem_len = len;
  while (rem_len) {
  STREAM_TO_ARRAY(value.value, p, value.len);

  tGATT_CL_COMPLETE gatt_cl_complete;
  gatt_cl_complete.att_value = value;
  gatt_cl_complete.cid = cid;
@@ -709,14 +725,40 @@ void gatt_process_notification(tGATT_TCB& tcb, uint16_t cid, uint8_t op_code,
    }
  }

  // If this is single value, then nothing is left to do
  if (op_code != GATT_HANDLE_MULTI_VALUE_NOTIF) return;

    /* 4 stands for 2 octects for handle and 2 octecs for len */
    rem_len -= (4 + value.len);
    if (rem_len) {
  // Need a signed type to check if the value is below 0
  // as uint16_t doesn't have negatives so the negatives register as a number
  // thus anything less than zero won't trigger the conditional and it is not
  // always 0
  // when done looping as value.len is arbitrary.
  int16_t rem_len = (int16_t)len - (4 /* octets */ + value.len);

  // Already streamed the first value and sent it, lets send the rest
  while (rem_len > 4 /* octets */) {
    // 2
    STREAM_TO_UINT16(value.handle, p);
    // + 2 = 4
    STREAM_TO_UINT16(value.len, p);
    // Accounting
    rem_len -= 4;
    // Make sure we don't read past the remaining data even if the length says
    // we can Also need to watch comparing the int16_t with the uint16_t
    value.len = std::min(rem_len, (int16_t)value.len);
    STREAM_TO_ARRAY(value.value, p, value.len);
    // Accounting
    rem_len -= value.len;

    gatt_cl_complete.att_value = value;
    gatt_cl_complete.cid = cid;

    for (i = 0, p_reg = gatt_cb.cl_rcb; i < GATT_MAX_APPS; i++, p_reg++) {
      if (p_reg->in_use && p_reg->app_cb.p_cmpl_cb) {
        conn_id = GATT_CREATE_CONN_ID(tcb.tcb_idx, p_reg->gatt_if);
        (*p_reg->app_cb.p_cmpl_cb)(conn_id, event, encrypt_status,
                                   &gatt_cl_complete);
      }
    }
  }
}