[security]write data without checking malloc result.

jmp0x7c00 commented 2 years ago

Dear sir,

This code didn't check whether malloc() result is NULL and writed sensitive data to it, as we know, the attacker can mmap the address 0, if the attacker do that, the sensitve data will be writed outside enclave.

in file PrivacyGuard/DataBroker/Enclave/enclave.cpp line ,vulnerability code is here:

sgx_status_t ECALL_enclave_DO_config(int num_DOs)
{
    int i;
    sgx_status_t ret = SGX_SUCCESS;

    sk_key_DO = (sgx_ec_key_128bit_t *) malloc(num_DOs * sizeof(sgx_ec_key_128bit_t));
    // here.
    // same issue to varaible sk_key_DO and DO_data_key_assigned 
    DO_data_key = (sgx_aes_gcm_128bit_key_t *) malloc(num_DOs * sizeof(sgx_aes_gcm_128bit_key_t));
    DO_data_key_assigned = (bool *) malloc(num_DOs * sizeof(bool));

    for(i = 0; i < num_DOs; i++)
    {
        DO_data_key_assigned[i] = false;
    }

    return ret;
}

here sensitive data is writen:

if(!DO_data_key_assigned[DO_ID-1])
    {
        /* Generate a 16-Byte data encryption key for DO's data */
        sgx_read_rand(DO_data_key[DO_ID-1], sizeof(sgx_aes_gcm_128bit_key_t));  //   the data encryption key will be leaked if malloc fails

yang-sec commented 2 years ago

Thanks and that's a legit concern. I am thinking of ditching malloc and instead, we assign fixed-size arrays (i.e., a fixed maximum number of DOs) to these keys. I guess we should have this fixed number publicized as DataBroker parameter.

jmp0x7c00 commented 2 years ago

same issues: https://github.com/yang-sec/PrivacyGuard/blob/1ef665fca9dadf00bc0bb363842ab471a747ab0a/CEE/isv_enclave/enclave_svm.cpp#L2006

https://github.com/yang-sec/PrivacyGuard/blob/1ef665fca9dadf00bc0bb363842ab471a747ab0a/CEE/isv_enclave/enclave_svm.cpp#L2205

https://github.com/yang-sec/PrivacyGuard/blob/1ef665fca9dadf00bc0bb363842ab471a747ab0a/CEE/isv_enclave/enclave_svm.cpp#L2283

https://github.com/yang-sec/PrivacyGuard/blob/1ef665fca9dadf00bc0bb363842ab471a747ab0a/CEE/isv_enclave/enclave_svm.cpp#L2312

https://github.com/yang-sec/PrivacyGuard/blob/1ef665fca9dadf00bc0bb363842ab471a747ab0a/CEE/isv_enclave/enclave_svm.cpp#L2574

//sk_key_DO, DO_data_key , it is sensitive https://github.com/yang-sec/PrivacyGuard/blob/1ef665fca9dadf00bc0bb363842ab471a747ab0a/CEE/isv_enclave/isv_enclave.cpp#L404

https://github.com/yang-sec/PrivacyGuard/blob/1ef665fca9dadf00bc0bb363842ab471a747ab0a/DataBroker/Enclave/enclave.cpp#L298

https://github.com/yang-sec/PrivacyGuard/blob/1ef665fca9dadf00bc0bb363842ab471a747ab0a/DataBroker/Enclave/enclave.cpp#L299

https://github.com/yang-sec/PrivacyGuard/blob/1ef665fca9dadf00bc0bb363842ab471a747ab0a/DataBroker/Enclave/enclave.cpp#L300

https://github.com/yang-sec/PrivacyGuard/blob/1ef665fca9dadf00bc0bb363842ab471a747ab0a/Enclave_testML/isv_enclave/enclave_svm.cpp#L2006

jmp0x7c00 commented 1 year ago