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

Commit 0a951647 authored by Brian Delwiche's avatar Brian Delwiche
Browse files

Fix UAF in sdp_discovery.cc 

It is possible with modifications to a client to open two connections
against the same SDP discovery database.  If this happens, it becomes
possible to reference a freed instance of the discovery database in the
second connection once the first one is closed.

To guard against this, check during discovery if a database has already
been allocated, and abort iff it has.

Also, add a null check to process_service_search_attr_rsp to guard
against unchecked calls to the SDP discovery database.

Bug: 291281168
Bug: 356201480
Flag: com.android.bluetooth.flags.btsec_check_valid_discovery_database
Test: atest bluetooth_test_gd_unit, net_test_stack_sdp
Tag: #security
Ignore-AOSP-First: Security
Change-Id: I65495091936a81d95cd1fd73c7456805a715d5bf
parent 2d1deafb

system/bta/hf_client/bta_hf_client_sdp.cc

+14 −0
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ 
 ******************************************************************************/



#include <bluetooth/log.h>

#include <com_android_bluetooth_flags.h>



#include <cstdint>
@@ -346,6 +347,19 @@ void bta_hf_client_do_disc(tBTA_HF_CLIENT_CB* client_cb) { 
    uuid_list[0] = Uuid::From16Bit(UUID_SERVCLASS_AG_HANDSFREE);

  }



  /* If we already have a non-null discovery database at this point, we can get

   * into a race condition leading to UAF once this connection is closed.

   * This should only happen with malicious modifications to a client. */

  if (com::android::bluetooth::flags::btsec_check_valid_discovery_database() &&

      client_cb->p_disc_db != NULL) {

    log::error("Tried to set up a HF client with a preexisting discovery database.");

    client_cb->p_disc_db = NULL;

    // We manually set the state here because it's possible to call this from an

    // OPEN state, in which case the discovery fail event will be ignored.

    client_cb->state = 0;  // BTA_HF_CLIENT_INIT_ST

    return;

  }



  /* allocate buffer for sdp database */

  client_cb->p_disc_db = (tSDP_DISCOVERY_DB*)osi_malloc(BT_DEFAULT_BUFFER_SIZE);

system/stack/sdp/sdp_discovery.cc

+11 −0
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ 
#define LOG_TAG "stack::sdp"



#include <bluetooth/log.h>

#include <com_android_bluetooth_flags.h>



#include <cstdint>
@@ -635,6 +636,16 @@ static void process_service_search_attr_rsp(tCONN_CB* p_ccb, uint8_t* p_reply, 
    uint8_t* p;

    uint16_t bytes_left = SDP_DATA_BUF_SIZE;



    /* If we don't have a valid discovery database, we can't do anything. */

    if (com::android::bluetooth::flags::btsec_check_valid_discovery_database() &&

        p_ccb->p_db == NULL) {

      log::warn(

              "Attempted continuation or first time request with invalid discovery "

              "database");

      sdp_disconnect(p_ccb, tSDP_STATUS::SDP_INVALID_CONT_STATE);

      return;

    }



    p_msg->offset = L2CAP_MIN_OFFSET;

    p = p_start = (uint8_t*)(p_msg + 1) + L2CAP_MIN_OFFSET;