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

Commit c8b83c7c authored by Ian Coolidge's avatar Ian Coolidge
Browse files

Fix use-after-free while setting advertisement data. 

Producer context was freeing data buffers without
performing a deep copy on the passed structure.

Also added some const correctness so we can reason
about the state of btif_adv_data_t as its data
gets associated with the advertising instance.

Bug: 21343750

Change-Id: Ifea0209e8426b61d8456c299c836ba640bd30f31
parent ef9f725f

system/btif/include/btif_gatt_multi_adv_util.h

+2 −1
Original line number Diff line number Diff line
@@ -83,13 +83,14 @@ extern void btif_gattc_cleanup_multi_inst_cb(btgatt_multi_adv_inst_cb *p_inst_cb 
                                                    BOOLEAN stop_timer);

// Free a buffer and reset *buf to NULL.

extern void btif_gattc_cleanup(void** buf);

extern BOOLEAN btif_gattc_copy_datacb(int arrindex, btif_adv_data_t *p_adv_data,

extern BOOLEAN btif_gattc_copy_datacb(int arrindex, const btif_adv_data_t *p_adv_data,

                                            BOOLEAN bInstData);

extern void btif_gattc_adv_data_packager(int client_if, bool set_scan_rsp,

                bool include_name, bool include_txpower, int min_interval, int max_interval,

                int appearance, int manufacturer_len, char* manufacturer_data,

                int service_data_len, char* service_data, int service_uuid_len,

                char* service_uuid, btif_adv_data_t *p_multi_adv_inst);

extern void btif_gattc_adv_data_cleanup(const btif_adv_data_t* adv);

void btif_multi_adv_timer_ctrl(int client_if, TIMER_CBACK cb);

#endif

system/btif/src/btif_gatt_client.c

+93 −48
Original line number Diff line number Diff line
@@ -1058,6 +1058,23 @@ static void bta_scan_filt_status_cb(UINT8 action, tBTA_STATUS status, 
                          (char*) &btif_cb, sizeof(btgatt_adv_filter_cb_t), NULL);

}



static void btgattc_free_event_data(UINT16 event, char *event_data)

{

    switch (event)

    {

        case BTIF_GATTC_ADV_INSTANCE_SET_DATA:

        case BTIF_GATTC_SET_ADV_DATA:

        {

            const btif_adv_data_t *adv_data = (btif_adv_data_t*) event_data;

            btif_gattc_adv_data_cleanup(adv_data);

            break;

        }



        default:

            break;

    }

}



static void btgattc_handle_event(uint16_t event, char* p_param)

{

    tBTA_GATT_STATUS           status;
@@ -1502,14 +1519,11 @@ static void btgattc_handle_event(uint16_t event, char* p_param) 


        case BTIF_GATTC_SET_ADV_DATA:

        {

            btif_adv_data_t *p_adv_data = (btif_adv_data_t*) p_param;

            int cbindex = CLNT_IF_IDX;

            if (cbindex >= 0 && NULL != p_adv_data)

            const btif_adv_data_t *p_adv_data = (btif_adv_data_t*) p_param;

            const int cbindex = CLNT_IF_IDX;

            if (cbindex >= 0 && btif_gattc_copy_datacb(cbindex, p_adv_data, false))

            {

                btgatt_multi_adv_common_data *p_multi_adv_data_cb = btif_obtain_multi_adv_data_cb();

                if (!btif_gattc_copy_datacb(cbindex, p_adv_data, false))

                    return;



                if (!p_adv_data->set_scan_rsp)

                {

                    BTA_DmBleSetAdvConfig(p_multi_adv_data_cb->inst_cb[cbindex].mask,
@@ -1520,8 +1534,13 @@ static void btgattc_handle_event(uint16_t event, char* p_param) 
                    BTA_DmBleSetScanRsp(p_multi_adv_data_cb->inst_cb[cbindex].mask,

                        &p_multi_adv_data_cb->inst_cb[cbindex].data, bta_gattc_set_adv_data_cback);

                }

                break;

            }

            else

            {

                BTIF_TRACE_ERROR("%s:%s: failed to get instance data cbindex: %d",

                                 __func__, "BTIF_GATTC_SET_ADV_DATA", cbindex);

            }

            break;

        }



        case BTIF_GATTC_ADV_INSTANCE_ENABLE:
@@ -1578,19 +1597,23 @@ static void btgattc_handle_event(uint16_t event, char* p_param) 
            btif_adv_data_t *p_adv_data = (btif_adv_data_t*) p_param;

            int cbindex = btif_gattc_obtain_idx_for_datacb(p_adv_data->client_if, CLNT_IF_IDX);

            int inst_id = btif_multi_adv_instid_for_clientif(p_adv_data->client_if);

            if (inst_id < 0 || cbindex < 0)

            if (inst_id >= 0 && cbindex >= 0 && btif_gattc_copy_datacb(cbindex, p_adv_data, true))

            {

               BTIF_TRACE_ERROR("%s invalid index in BTIF_GATTC_SETADV_INST_DATA", __FUNCTION__);

               return;

            }



            if (!btif_gattc_copy_datacb(cbindex, p_adv_data, true))

                return;



            btgatt_multi_adv_common_data *p_multi_adv_data_cb = btif_obtain_multi_adv_data_cb();

            BTA_BleCfgAdvInstData((UINT8)inst_id, p_multi_adv_data_cb->inst_cb[cbindex].is_scan_rsp,

                btgatt_multi_adv_common_data *p_multi_adv_data_cb =

                    btif_obtain_multi_adv_data_cb();

                BTA_BleCfgAdvInstData(

                    (UINT8)inst_id,

                    p_multi_adv_data_cb->inst_cb[cbindex].is_scan_rsp,

                    p_multi_adv_data_cb->inst_cb[cbindex].mask,

                    &p_multi_adv_data_cb->inst_cb[cbindex].data);

            }

            else

            {

                BTIF_TRACE_ERROR(

                    "%s:%s: failed to get invalid instance data: inst_id:%d "

                    "cbindex:%d",

                    __func__, "BTIF_GATTC_ADV_INSTANCE_SET_DATA", inst_id, cbindex);

            }

            break;

        }
@@ -1666,6 +1689,8 @@ static void btgattc_handle_event(uint16_t event, char* p_param) 
            LOG_ERROR("%s: Unknown event (%d)!", __FUNCTION__, event);

            break;

    }



    btgattc_free_event_data(event, p_param);

}



/*******************************************************************************
@@ -1732,6 +1757,44 @@ static bt_status_t btif_gattc_listen(int client_if, bool start) 
                                 (char*) &btif_cb, sizeof(btif_gattc_cb_t), NULL);

}



static void btif_gattc_deep_copy(UINT16 event, char *p_dest, char *p_src)

{

    switch (event)

    {

        case BTIF_GATTC_ADV_INSTANCE_SET_DATA:

        case BTIF_GATTC_SET_ADV_DATA:

        {

            const btif_adv_data_t *src = (btif_adv_data_t*) p_src;

            btif_adv_data_t *dst = (btif_adv_data_t*) p_dest;

            memcpy(dst, src, sizeof(*src));



            if (src->p_manufacturer_data)

            {

                dst->p_manufacturer_data = GKI_getbuf(src->manufacturer_len);

                memcpy(dst->p_manufacturer_data, src->p_manufacturer_data,

                       src->manufacturer_len);

            }



            if (src->p_service_data)

            {

                dst->p_service_data = GKI_getbuf(src->service_data_len);

                memcpy(dst->p_service_data, src->p_service_data, src->service_data_len);

            }



            if (src->p_service_uuid)

            {

                dst->p_service_uuid = GKI_getbuf(src->service_uuid_len);

                memcpy(dst->p_service_uuid, src->p_service_uuid, src->service_uuid_len);

            }

            break;

        }



        default:

            ASSERTC(false, "Unhandled deep copy", event);

            break;

    }

}



static bt_status_t btif_gattc_set_adv_data(int client_if, bool set_scan_rsp, bool include_name,

                bool include_txpower, int min_interval, int max_interval, int appearance,

                uint16_t manufacturer_len, char* manufacturer_data,
@@ -1739,8 +1802,6 @@ static bt_status_t btif_gattc_set_adv_data(int client_if, bool set_scan_rsp, boo 
                uint16_t service_uuid_len, char* service_uuid)

{

    CHECK_BTGATT_INIT();

    bt_status_t status =0;



    btif_adv_data_t adv_data;



    btif_gattc_adv_data_packager(client_if, set_scan_rsp, include_name,
@@ -1748,18 +1809,9 @@ static bt_status_t btif_gattc_set_adv_data(int client_if, bool set_scan_rsp, boo 
        manufacturer_data, service_data_len, service_data, service_uuid_len, service_uuid,

        &adv_data);



    status = btif_transfer_context(btgattc_handle_event, BTIF_GATTC_SET_ADV_DATA,

                       (char*) &adv_data, sizeof(btif_adv_data_t), NULL);



    if (NULL != adv_data.p_service_data)

        GKI_freebuf(adv_data.p_service_data);



    if (NULL != adv_data.p_service_uuid)

        GKI_freebuf(adv_data.p_service_uuid);



    if (NULL != adv_data.p_manufacturer_data)

        GKI_freebuf(adv_data.p_manufacturer_data);



    bt_status_t status = btif_transfer_context(btgattc_handle_event, BTIF_GATTC_SET_ADV_DATA,

                       (char*) &adv_data, sizeof(adv_data), btif_gattc_deep_copy);

    btif_gattc_adv_data_cleanup(&adv_data);

    return status;

}
@@ -2141,28 +2193,21 @@ static bt_status_t btif_gattc_multi_adv_setdata(int client_if, bool set_scan_rsp 
{

    CHECK_BTGATT_INIT();



    int min_interval = 0, max_interval = 0;

    bt_status_t status =0;



    btif_adv_data_t multi_adv_data_inst;

    memset(&multi_adv_data_inst, 0, sizeof(btif_adv_data_t));

    memset(&multi_adv_data_inst, 0, sizeof(multi_adv_data_inst));



    const int min_interval = 0;

    const int max_interval = 0;



    btif_gattc_adv_data_packager(client_if, set_scan_rsp, include_name, incl_txpower,

        min_interval, max_interval, appearance, manufacturer_len, manufacturer_data,

        service_data_len, service_data, service_uuid_len, service_uuid, &multi_adv_data_inst);



    status = btif_transfer_context(btgattc_handle_event, BTIF_GATTC_ADV_INSTANCE_SET_DATA,

                       (char*) &multi_adv_data_inst, sizeof(btif_adv_data_t), NULL);



    if (NULL != multi_adv_data_inst.p_service_data)

        GKI_freebuf(multi_adv_data_inst.p_service_data);



    if (NULL != multi_adv_data_inst.p_service_uuid)

        GKI_freebuf(multi_adv_data_inst.p_service_uuid);



    if (NULL != multi_adv_data_inst.p_manufacturer_data)

        GKI_freebuf(multi_adv_data_inst.p_manufacturer_data);



    bt_status_t status = btif_transfer_context(

        btgattc_handle_event, BTIF_GATTC_ADV_INSTANCE_SET_DATA,

        (char *)&multi_adv_data_inst, sizeof(multi_adv_data_inst),

        btif_gattc_deep_copy);

    btif_gattc_adv_data_cleanup(&multi_adv_data_inst);

    return status;

}

system/btif/src/btif_gatt_multi_adv_util.c

+19 −9
Original line number Diff line number Diff line
@@ -252,8 +252,21 @@ void btif_gattc_adv_data_packager(int client_if, bool set_scan_rsp, 
    }

}



BOOLEAN btif_gattc_copy_datacb(int cbindex, btif_adv_data_t *p_adv_data, BOOLEAN bInstData)

void btif_gattc_adv_data_cleanup(const btif_adv_data_t* adv)

{

    if (adv->p_service_data)

        GKI_freebuf(adv->p_service_data);



    if (adv->p_service_uuid)

        GKI_freebuf(adv->p_service_uuid);



    if (adv->p_manufacturer_data)

        GKI_freebuf(adv->p_manufacturer_data);

}





BOOLEAN btif_gattc_copy_datacb(int cbindex, const btif_adv_data_t *p_adv_data,

                               BOOLEAN bInstData) {

    btgatt_multi_adv_common_data *p_multi_adv_data_cb = btif_obtain_multi_adv_data_cb();

    if (NULL == p_multi_adv_data_cb || cbindex < 0)

       return false;
@@ -377,18 +390,18 @@ BOOLEAN btif_gattc_copy_datacb(int cbindex, btif_adv_data_t *p_adv_data, BOOLEAN 
        }

    }



    if (p_adv_data->service_uuid_len > 0 && NULL != p_adv_data->p_service_uuid)

    if (p_adv_data->service_uuid_len && p_adv_data->p_service_uuid)

    {

        UINT16 *p_uuid_out16 = NULL;

        UINT32 *p_uuid_out32 = NULL;

        while (p_adv_data->service_uuid_len >= LEN_UUID_128)

        for (int position = 0; position < p_adv_data->service_uuid_len; position += LEN_UUID_128)

        {

             bt_uuid_t uuid;

             memset(&uuid, 0, sizeof(bt_uuid_t));

             memcpy(&uuid.uu, p_adv_data->p_service_uuid, LEN_UUID_128);

             memset(&uuid, 0, sizeof(uuid));

             memcpy(&uuid.uu, p_adv_data->p_service_uuid + position, LEN_UUID_128);



             tBT_UUID bt_uuid;

             memset(&bt_uuid, 0, sizeof(tBT_UUID));

             memset(&bt_uuid, 0, sizeof(bt_uuid));

             btif_to_bta_uuid(&bt_uuid, &uuid);



             switch(bt_uuid.len)
@@ -467,9 +480,6 @@ BOOLEAN btif_gattc_copy_datacb(int cbindex, btif_adv_data_t *p_adv_data, BOOLEAN 
                default:

                     break;

             }



             p_adv_data->p_service_uuid += LEN_UUID_128;

             p_adv_data->service_uuid_len -= LEN_UUID_128;

        }

    }