Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
kydahe opened this issue Aug 16, 2021 · 5 comments
Open

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

kydahe opened this issue Aug 16, 2021 · 5 comments

Comments

@kydahe
Copy link

kydahe commented Aug 16, 2021

  1. 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;
}
  1. 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安全问题。

  1. 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安全问题。

  1. 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;
}
  1. 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
Copy link
Collaborator

xupenghu commented Sep 1, 2021

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

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

所以double free应该没关系,但是在代码中确实会形成double free的现象,后面会想办法优化,再次感谢!

@kydahe
Copy link
Author

kydahe commented Sep 1, 2021

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

  1. samples/asr/asr_data_template_sample.c
    int IOT_Template_Destroy(void *pClient);函数会对入参进行判断
    POINTER_SANITY_CHECK(pClient, QCLOUD_ERR_INVAL);
    所以double free应该没关系。 2. 3. 同上
  2. 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
Copy link
Collaborator

xupenghu commented Sep 1, 2021

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

  1. samples/asr/asr_data_template_sample.c
    int IOT_Template_Destroy(void *pClient);函数会对入参进行判断
    POINTER_SANITY_CHECK(pClient, QCLOUD_ERR_INVAL);
    所以double free应该没关系。 2. 3. 同上
  2. 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 kydahe closed this as completed Nov 30, 2021
@kydahe kydahe reopened this Nov 30, 2021
@kydahe
Copy link
Author

kydahe commented Nov 30, 2021

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

  1. samples/asr/asr_data_template_sample.c
    int IOT_Template_Destroy(void *pClient);函数会对入参进行判断
    POINTER_SANITY_CHECK(pClient, QCLOUD_ERR_INVAL);
    所以double free应该没关系。 2. 3. 同上
  2. 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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants