代码中存在Memory Double Free问题

kydahe commented 3 years ago

samples/asr/asr_data_template_sample.c https://github.com/tencentyun/qcloud-iot-explorer-sdk-embedded-c/blob/master/samples/asr/asr_data_template_sample.c

DF问题：在main函数的第407行，调用了_register_data_template_property函数；在_register_data_template_property函数的214行中，如果rc!=QCLOUD_RET_SUCCESS，则调用IOT_Template_Destroy对参数1（即main函数中的data_template_client）进行释放；之后，main函数进入210的else分支，并跳转到exit；exit中在514行，调用IOT_Template_Destroy对data_template_client进行了而此释放，造成了double free安全问题。

static int _register_data_template_property(void *pTemplate_client)
{
    int i, rc;

    ...
        if (rc != QCLOUD_RET_SUCCESS) {
            rc = IOT_Template_Destroy(pTemplate_client);
            Log_e("register device data template property failed, err: %d", rc);
            return rc;
        } else {
            Log_i("data template property=%s registered.", sg_DataTemplate[i].data_property.key);
        }
    }

    return QCLOUD_RET_SUCCESS;
}

int main(int argc, char **argv)
{
    DeviceProperty *pReportDataList[TOTAL_PROPERTY_COUNT];
    sReplyPara      replyPara;
    int             ReportCont;
    int             rc;
    void *          data_template_client = NULL;
    void *          asr_client           = NULL;

    ...
    // usr init
    _usr_init();

    // init data template
    _init_data_template();

    // register data template propertys here
    rc = _register_data_template_property(data_template_client);
    if (rc == QCLOUD_RET_SUCCESS) {
        Log_i("Register data template propertys Success");
    } else {
        Log_e("Register data template propertys Failed: %d", rc);
        goto exit;
    }

    // report device info, then you can manager your product by these info, like position
    ...

exit:

#ifdef MULTITHREAD_ENABLED
    IOT_Template_Stop_Yield_Thread(data_template_client);
#endif
    HAL_SleepMs(1000);
    rc = IOT_Template_Destroy(data_template_client);
    rc |= IOT_Asr_Destroy(asr_client);

#ifdef LOG_UPLOAD
    IOT_Log_Upload(true);
    IOT_Log_Fini_Uploader();
#endif

    return rc;
}

samples/data_template/data_template_sample.c https://github.com/tencentyun/qcloud-iot-explorer-sdk-embedded-c/blob/master/samples/data_template/data_template_sample.c

DF问题：同上，client在_register_data_template_property中被释放（273行IOT_Template_Destroy），之后在main函数中的exit跳转中又再次被释放（521行IOT_Template_Destroy），造成Memory Double Free安全问题。

samples/scenarized/light_data_template_sample.c https://github.com/tencentyun/qcloud-iot-explorer-sdk-embedded-c/blob/master/samples/scenarized/light_data_template_sample.c

DF问题：同上，client在_register_data_template_property中被释放482行IOT_Template_Destroy），之后在main函数中的exit跳转中又再次被释放837行IOT_Template_Destroy），造成Memory Double Free安全问题。

samples/ota/ota_mqtt_sample.c https://github.com/tencentyun/qcloud-iot-explorer-sdk-embedded-c/blob/master/samples/ota/ota_mqtt_sample.c

DF问题：在第295行已经释放了version内存，但在第300行又再次释放version，造成double free安全问题。

static int _get_local_fw_info(char *file_name, char *local_version)
{
    ...
    char *version = LITE_json_value_of(KEY_VER, json_doc);
    char *size    = LITE_json_value_of(KEY_SIZE, json_doc);

    if ((NULL == version) || (NULL == size)) {
        if (version)
            HAL_Free(version);
        if (size)
            HAL_Free(size);
        HAL_FileClose(fp);
        return 0;
    }

    int local_size = atoi(size);
    HAL_Free(size);

    if (local_size <= 0) {
        Log_w("local info offset invalid: %d", local_size);
        HAL_Free(version);    //!!! version memory is freed
        local_size = 0;
    }

    strncpy(local_version, version, FW_VERSION_MAX_LEN);
    HAL_Free(version);    //!!! Free memory version that has been freed before --- DF
    HAL_FileClose(fp);
    return local_size;
}

sdk_src/services/data_template/data_template_client.c https://github.com/tencentyun/qcloud-iot-explorer-sdk-embedded-c/blob/master/sdk_src/services/data_template/data_template_client.c

DF问题：在869行，pTemplate在IOT_Template_Destroy中通过HAL_Free进行释放，但在870行中pTemplate被HAL_Free进行二次释放，造成double free安全问题。

void *IOT_Template_Construct(TemplateInitParams *pParams, void *pMqttClient)
{
    ...
    rc = qcloud_iot_template_init(pTemplate);
    if (rc != QCLOUD_RET_SUCCESS) {
        IOT_MQTT_Destroy(&(pTemplate->mqtt));
        IOT_Template_Destroy(pTemplate);    //!!! Memory pTemplate is released via HAL_Free in IOT_Template_Destroy
        HAL_Free(pTemplate);    //!!! Free memory pTemplate which is already freed
        goto End;
    }
    ...
}

xupenghu commented 2 years ago

感谢指出问题。回复如下:

samples/asr/asr_data_template_sample.c int IOT_Template_Destroy(void *pClient);函数会对入参进行判断 POINTER_SANITY_CHECK(pClient, QCLOUD_ERR_INVAL); 所以double free应该没关系。 2. 3. 同上
HAL_Free函数也对入参进行了判断
```
void HAL_Free(void *ptr)
{
if (ptr)
    free(ptr);
}
```
所以double free应该没关系，但是在代码中确实会形成double free的现象，后面会想办法优化，再次感谢！

kydahe commented 2 years ago

感谢指出问题。回复如下:

samples/asr/asr_data_template_sample.c int IOT_Template_Destroy(void *pClient);函数会对入参进行判断 POINTER_SANITY_CHECK(pClient, QCLOUD_ERR_INVAL); 所以double free应该没关系。 2. 3. 同上

HAL_Free函数也对入参进行了判断
void HAL_Free(void *ptr)
{
    if (ptr)
        free(ptr);
}
所以double free应该没关系，但是在代码中确实会形成double free的现象，后面会想办法优化，再次感谢！

感谢回复！！但是想请问一下这个HAL_Free的实现是在哪里？我找到的HAL_Free实现是没有入参判断的....如下所示

void HAL_Free(_IN_ void *ptr)
{
    free(ptr);
}

所以想请教一下，感谢！！

xupenghu commented 2 years ago

感谢指出问题。回复如下:

samples/asr/asr_data_template_sample.c int IOT_Template_Destroy(void *pClient);函数会对入参进行判断 POINTER_SANITY_CHECK(pClient, QCLOUD_ERR_INVAL); 所以double free应该没关系。 2. 3. 同上

HAL_Free函数也对入参进行了判断
void HAL_Free(void *ptr)
{
    if (ptr)
        free(ptr);
}
所以double free应该没关系，但是在代码中确实会形成double free的现象，后面会想办法优化，再次感谢！
感谢回复！！但是想请问一下这个HAL_Free的实现是在哪里？我找到的HAL_Free实现是没有入参判断的....如下所示
void HAL_Free(_IN_ void *ptr)
{
    free(ptr);
}
所以想请教一下，感谢！！

请参考 https://github.com/tencentyun/qcloud-iot-explorer-sdk-embedded-c/blob/master/platform/os/linux/HAL_OS_linux.c 第259行

kydahe commented 2 years ago

感谢指出问题。回复如下:

samples/asr/asr_data_template_sample.c int IOT_Template_Destroy(void *pClient);函数会对入参进行判断 POINTER_SANITY_CHECK(pClient, QCLOUD_ERR_INVAL); 所以double free应该没关系。 2. 3. 同上

HAL_Free函数也对入参进行了判断
void HAL_Free(void *ptr)
{
    if (ptr)
        free(ptr);
}
所以double free应该没关系，但是在代码中确实会形成double free的现象，后面会想办法优化，再次感谢！
感谢回复！！但是想请问一下这个HAL_Free的实现是在哪里？我找到的HAL_Free实现是没有入参判断的....如下所示
void HAL_Free(_IN_ void *ptr)
{
    free(ptr);
}
所以想请教一下，感谢！！
请参考 https://github.com/tencentyun/qcloud-iot-explorer-sdk-embedded-c/blob/master/platform/os/linux/HAL_OS_linux.c 第259行

HAL_Free和IOT_Template_Destroy函数在free指针内存之后，并没有对指针置空，所以在连续调用这两个函数时，尽管进行了空指针检查，但是并没有对已释放的指针置空，所以还是会存在double free问题。

void HAL_Free(_IN_ void *ptr)
{
    if (ptr)
        free(ptr);
}

int IOT_Template_Destroy(void *pClient)
{
    IOT_FUNC_ENTRY;

    POINTER_SANITY_CHECK(pClient, QCLOUD_ERR_INVAL);

    Qcloud_IoT_Template *pTemplate = (Qcloud_IoT_Template *)pClient;
    qcloud_iot_template_reset(pTemplate);
    IOT_MQTT_Destroy(&pTemplate->mqtt);

    if (NULL != pTemplate->mutex) {
        HAL_MutexDestroy(pTemplate->mutex);
    }

    if (NULL != pTemplate->inner_data.downstream_topic) {
        HAL_Free(pTemplate->inner_data.downstream_topic);
        pTemplate->inner_data.downstream_topic = NULL;
    }

    HAL_Free(pClient);

    IOT_FUNC_EXIT_RC(QCLOUD_RET_SUCCESS)
}

Genie-bj commented 2 years ago

感谢指出，下一版本修复。

TencentCloud / tencentcloud-iot-explorer-sdk-embedded-c

代码中存在Memory Double Free问题 #10