Skip to content
Snippets Groups Projects
Commit c4d8043e authored by Brian Delwiche's avatar Brian Delwiche Committed by Android Build Coastguard Worker
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
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:c7bc0115a7cbad342628c8428af1b8181ee1ae94)
Merged-In: I754bf8292e1e0d8e90e78fa87889284e26aa5818
Change-Id: I754bf8292e1e0d8e90e78fa87889284e26aa5818
parent 6893799a
No related branches found
No related tags found
No related merge requests found
......@@ -319,6 +319,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 (client_cb->p_disc_db != NULL) {
APPL_TRACE_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);
......
......@@ -596,6 +596,15 @@ 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 (p_ccb->p_db == NULL) {
SDP_TRACE_WARNING(
"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;
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment