From 80e29dd7e6facb2d92114b7eac32908b783e57a1 Mon Sep 17 00:00:00 2001 From: Ronan Abhamon Date: Tue, 24 Apr 2018 14:52:35 +0200 Subject: [PATCH] fix(c-content): avoid many memory leaks --- coreapi/lime.c | 62 +++++++++---------- src/c-wrapper/api/c-content.cpp | 37 ++++++----- .../file-transfer-chat-message-modifier.cpp | 6 +- src/content/content-manager.cpp | 4 -- tester/message_tester.c | 24 ++++--- tester/multipart-tester.cpp | 1 - 6 files changed, 58 insertions(+), 76 deletions(-) diff --git a/coreapi/lime.c b/coreapi/lime.c index 861830d3d..d0c07cd89 100644 --- a/coreapi/lime.c +++ b/coreapi/lime.c @@ -317,7 +317,7 @@ static int lime_deriveKey(limeKey_t *key) { return LIME_UNABLE_TO_DERIVE_KEY; } - /* Derivation is made derived Key = HMAC_SHA256(Key, 0x0000001||"MessageKey"||0x00||SessionId||SessionIndex||0x00000100)*/ + /* Derivation is made derived Key = HMAC_SHA256(Key, 0x0000001||"MessageKey"||0x00||SessionId||SessionIndex||0x00000100)*/ /* total data to be hashed is 55 bytes : 4 + 10 + 1 + 32 + 4 + 4 */ inputData[0] = 0x00; inputData[1] = 0x00; @@ -811,7 +811,7 @@ int lime_im_encryption_engine_process_incoming_message_cb(LinphoneImEncryptionEn peerUri = linphone_address_as_string_uri_only(linphone_chat_message_get_from_address(msg)); selfUri = linphone_address_as_string_uri_only(linphone_chat_message_get_to_address(msg)); retval = lime_decryptMultipartMessage(zrtp_cache_db, (uint8_t *)linphone_chat_message_get_text(msg), selfUri, peerUri, &decrypted_body, &decrypted_content_type, - bctbx_time_string_to_sec(lp_config_get_string(lc->config, "sip", "lime_key_validity", "0"))); + bctbx_time_string_to_sec(lp_config_get_string(lc->config, "sip", "lime_key_validity", "0"))); ms_free(peerUri); ms_free(selfUri); if (retval != 0) { @@ -848,9 +848,9 @@ int lime_im_encryption_engine_process_outgoing_message_cb(LinphoneImEncryptionEn if (linphone_chat_message_get_content_type(msg)) { if (strcmp(linphone_chat_message_get_content_type(msg), "application/vnd.gsma.rcs-ft-http+xml") == 0) { /* It's a file transfer, content type shall be set to application/cipher.vnd.gsma.rcs-ft-http+xml - TODO: As of january 2017, the content type is now included in the encrypted body, this - application/cipher.vnd.gsma.rcs-ft-http+xml is kept for compatibility with previous versions, - but may be dropped in the future to use xml/cipher instead. */ + TODO: As of january 2017, the content type is now included in the encrypted body, this + application/cipher.vnd.gsma.rcs-ft-http+xml is kept for compatibility with previous versions, + but may be dropped in the future to use xml/cipher instead. */ new_content_type = "application/cipher.vnd.gsma.rcs-ft-http+xml"; } else if (strcmp(linphone_chat_message_get_content_type(msg), "application/im-iscomposing+xml") == 0) { /* We don't encrypt composing messages */ @@ -897,22 +897,20 @@ int lime_im_encryption_engine_process_downloading_file_cb(LinphoneImEncryptionEn LinphoneContent *content = linphone_chat_message_get_file_transfer_information(msg); if (!content) return -1; - - if (!linphone_content_get_key(content)) { - linphone_content_unref(content); + + if (!linphone_content_get_key(content)) return -1; - } - if (!buffer || (size == 0)) { - int result = lime_decryptFile(linphone_content_get_cryptoContext_address(content), NULL, 0, NULL, NULL); - linphone_content_unref(content); - return result; - } + if (!buffer || size == 0) + return lime_decryptFile(linphone_content_get_cryptoContext_address(content), NULL, 0, NULL, NULL); - int result = lime_decryptFile(linphone_content_get_cryptoContext_address(content), - (unsigned char *)linphone_content_get_key(content), size, (char *)decrypted_buffer, (char *)buffer); - linphone_content_unref(content); - return result; + return lime_decryptFile( + linphone_content_get_cryptoContext_address(content), + (unsigned char *)linphone_content_get_key(content), + size, + (char *)decrypted_buffer, + (char *)buffer + ); } int lime_im_encryption_engine_process_uploading_file_cb(LinphoneImEncryptionEngine *engine, LinphoneChatMessage *msg, size_t offset, const uint8_t *buffer, size_t *size, uint8_t *encrypted_buffer) { @@ -921,16 +919,11 @@ int lime_im_encryption_engine_process_uploading_file_cb(LinphoneImEncryptionEngi if (!content) return -1; - if (!linphone_content_get_key(content)) { - linphone_content_unref(content); + if (!linphone_content_get_key(content)) return -1; - } - if (!buffer || (*size == 0)) { - int result = lime_encryptFile(linphone_content_get_cryptoContext_address(content), NULL, 0, NULL, NULL); - linphone_content_unref(content); - return result; - } + if (!buffer || *size == 0) + return lime_encryptFile(linphone_content_get_cryptoContext_address(content), NULL, 0, NULL, NULL); size_t file_size = linphone_content_get_size(content); if (file_size == 0) { @@ -939,10 +932,13 @@ int lime_im_encryption_engine_process_uploading_file_cb(LinphoneImEncryptionEngi *size -= (*size % 16); } - int result = lime_encryptFile(linphone_content_get_cryptoContext_address(content), - (unsigned char *)linphone_content_get_key(content), *size, (char *)buffer, (char *)encrypted_buffer); - linphone_content_unref(content); - return result; + return lime_encryptFile( + linphone_content_get_cryptoContext_address(content), + (unsigned char *)linphone_content_get_key(content), + *size, + (char *)buffer, + (char *)encrypted_buffer + ); } bool_t lime_im_encryption_engine_is_file_encryption_enabled_cb(LinphoneImEncryptionEngine *engine, LinphoneChatRoom *room) { @@ -956,10 +952,8 @@ void lime_im_encryption_engine_generate_file_transfer_key_cb(LinphoneImEncryptio * file_transfer_information->key field of the msg */ sal_get_random_bytes((unsigned char *)keyBuffer, FILE_TRANSFER_KEY_SIZE); LinphoneContent *content = linphone_chat_message_get_file_transfer_information(msg); - if (!content) - return; - linphone_content_set_key(content, keyBuffer, FILE_TRANSFER_KEY_SIZE); /* key is duplicated in the content private structure */ - linphone_content_unref(content); + if (content) + linphone_content_set_key(content, keyBuffer, FILE_TRANSFER_KEY_SIZE); /* key is duplicated in the content private structure */ } #else /* HAVE_LIME */ diff --git a/src/c-wrapper/api/c-content.cpp b/src/c-wrapper/api/c-content.cpp index b4af8dd30..0d92a6243 100644 --- a/src/c-wrapper/api/c-content.cpp +++ b/src/c-wrapper/api/c-content.cpp @@ -55,8 +55,7 @@ LinphoneContent * linphone_content_ref(LinphoneContent *content) { } void linphone_content_unref(LinphoneContent *content) { - // FIXME: Avoid leaks. - // belle_sip_object_unref(content); + belle_sip_object_unref(content); } void *linphone_content_get_user_data(const LinphoneContent *content) { @@ -70,8 +69,8 @@ void linphone_content_set_user_data(LinphoneContent *content, void *ud) { // ============================================================================= const char * linphone_content_get_type(const LinphoneContent *content) { - if (content->type) ms_free(content->type); - content->type = ms_strdup(L_STRING_TO_C(L_GET_CPP_PTR_FROM_C_OBJECT(content)->getContentType().getType())); + if (content->type) bctbx_free(content->type); + content->type = bctbx_strdup(L_STRING_TO_C(L_GET_CPP_PTR_FROM_C_OBJECT(content)->getContentType().getType())); return content->type; } @@ -82,8 +81,8 @@ void linphone_content_set_type(LinphoneContent *content, const char *type) { } const char * linphone_content_get_subtype(const LinphoneContent *content) { - if (content->subtype) ms_free(content->subtype); - content->subtype = ms_strdup(L_STRING_TO_C(L_GET_CPP_PTR_FROM_C_OBJECT(content)->getContentType().getSubType())); + if (content->subtype) bctbx_free(content->subtype); + content->subtype = bctbx_strdup(L_STRING_TO_C(L_GET_CPP_PTR_FROM_C_OBJECT(content)->getContentType().getSubType())); return content->subtype; } @@ -108,8 +107,8 @@ void linphone_content_set_buffer(LinphoneContent *content, const uint8_t *buffer } const char * linphone_content_get_string_buffer(const LinphoneContent *content) { - if (content->body) ms_free(content->body); - content->body = ms_strdup(L_GET_CPP_PTR_FROM_C_OBJECT(content)->getBodyAsUtf8String().c_str()); + if (content->body) bctbx_free(content->body); + content->body = bctbx_strdup(L_GET_CPP_PTR_FROM_C_OBJECT(content)->getBodyAsUtf8String().c_str()); return content->body; } @@ -145,26 +144,26 @@ const char * linphone_content_get_encoding(const LinphoneContent *content) { } void linphone_content_set_encoding(LinphoneContent *content, const char *encoding) { - if (content->encoding) ms_free(content->encoding); - content->encoding = ms_strdup(encoding); + if (content->encoding) bctbx_free(content->encoding); + content->encoding = bctbx_strdup(encoding); } const char * linphone_content_get_name(const LinphoneContent *content) { const LinphonePrivate::Content *c = L_GET_CPP_PTR_FROM_C_OBJECT(content); if (c->isFile()) { const LinphonePrivate::FileContent *fc = static_cast(c); - if (content->name) ms_free(content->name); - content->name = ms_strdup(L_STRING_TO_C(fc->getFileName())); + if (content->name) bctbx_free(content->name); + content->name = bctbx_strdup(L_STRING_TO_C(fc->getFileName())); } else if (c->isFileTransfer()) { const LinphonePrivate::FileTransferContent *ftc = static_cast(c); - if (content->name) ms_free(content->name); - content->name = ms_strdup(L_STRING_TO_C(ftc->getFileName())); + if (content->name) bctbx_free(content->name); + content->name = bctbx_strdup(L_STRING_TO_C(ftc->getFileName())); } return content->name; } void linphone_content_set_name(LinphoneContent *content, const char *name) { - if (content->name) ms_free(content->name); + if (content->name) bctbx_free(content->name); LinphonePrivate::Content *c = L_GET_CPP_PTR_FROM_C_OBJECT(content); if (c->isFile()) { @@ -175,7 +174,7 @@ void linphone_content_set_name(LinphoneContent *content, const char *name) { ftc->setFileName(L_C_TO_STRING(name)); } - content->name = ms_strdup(name); + content->name = bctbx_strdup(name); } bool_t linphone_content_is_multipart(const LinphoneContent *content) { @@ -216,12 +215,12 @@ const char * linphone_content_get_custom_header(const LinphoneContent *content, } const char *linphone_content_get_key(const LinphoneContent *content) { - if (content->key) ms_free(content->key); + if (content->key) bctbx_free(content->key); const LinphonePrivate::Content *c = L_GET_CPP_PTR_FROM_C_OBJECT(content); if (c->isFileTransfer()) { const LinphonePrivate::FileTransferContent *ftc = static_cast(c); - content->key = ms_strdup(ftc->getFileKeyAsString()); + content->key = bctbx_strdup(ftc->getFileKeyAsString()); } return content->key; @@ -317,7 +316,7 @@ SalBodyHandler * sal_body_handler_from_content(const LinphoneContent *content) { if (contentType.isMultipart()) { size_t size = linphone_content_get_size(content); - char *buffer = ms_strdup(L_GET_CPP_PTR_FROM_C_OBJECT(content)->getBodyAsUtf8String().c_str()); + char *buffer = bctbx_strdup(L_GET_CPP_PTR_FROM_C_OBJECT(content)->getBodyAsUtf8String().c_str()); const char *boundary = L_STRING_TO_C(contentType.getParameter("boundary").getValue()); belle_sip_multipart_body_handler_t *bh = belle_sip_multipart_body_handler_new_from_buffer(buffer, size, boundary); body_handler = (SalBodyHandler *)BELLE_SIP_BODY_HANDLER(bh); diff --git a/src/chat/modifier/file-transfer-chat-message-modifier.cpp b/src/chat/modifier/file-transfer-chat-message-modifier.cpp index 86f67d02c..ad17cfff7 100644 --- a/src/chat/modifier/file-transfer-chat-message-modifier.cpp +++ b/src/chat/modifier/file-transfer-chat-message-modifier.cpp @@ -116,7 +116,6 @@ void FileTransferChatMessageModifier::fileTransferOnProgress ( // Legacy: call back given by application level. linphone_core_notify_file_transfer_progress_indication(message->getCore()->getCCore(), msg, content, offset, total); } - linphone_content_unref(content); } static int _chat_message_on_send_body ( @@ -173,7 +172,6 @@ int FileTransferChatMessageModifier::onSendBody ( // Legacy linphone_core_notify_file_transfer_send(message->getCore()->getCCore(), msg, content, (char *)buffer, size); } - linphone_content_unref(content); } LinphoneImEncryptionEngine *imee = linphone_core_get_im_encryption_engine(message->getCore()->getCCore()); @@ -265,7 +263,7 @@ void FileTransferChatMessageModifier::processResponseFromPostFile (const belle_h // Actual filename stored in msg->file_transfer_information->name will be set in encrypted msg // sended to the first_part_header = "form-data; name=\"File\"; filename=\"filename.txt\""; - + LinphoneImEncryptionEngineCbs *imee_cbs = linphone_im_encryption_engine_get_callbacks(imee); LinphoneImEncryptionEngineCbsGenerateFileTransferKeyCb generate_file_transfer_key_cb = linphone_im_encryption_engine_cbs_get_generate_file_transfer_key(imee_cbs); @@ -763,7 +761,6 @@ void FileTransferChatMessageModifier::onRecvBody (belle_sip_user_body_handler_t // Legacy: call back given by application level linphone_core_notify_file_transfer_recv(message->getCore()->getCCore(), msg, content, (const char *)buffer, size); } - linphone_content_unref(content); } } else { lWarning() << "File transfer decrypt failed with code " << (int)retval; @@ -804,7 +801,6 @@ void FileTransferChatMessageModifier::onRecvEnd (belle_sip_user_body_handler_t * // Legacy: call back given by application level linphone_core_notify_file_transfer_recv(message->getCore()->getCCore(), msg, content, nullptr, 0); } - linphone_content_unref(content); } } diff --git a/src/content/content-manager.cpp b/src/content/content-manager.cpp index 6040140b8..0f10cfaa4 100644 --- a/src/content/content-manager.cpp +++ b/src/content/content-manager.cpp @@ -49,11 +49,9 @@ list ContentManager::multipartToContentList (const Content &content) { LinphoneContent *cContent = linphone_content_from_sal_body_handler(part); Content *cppContent = L_GET_CPP_PTR_FROM_C_OBJECT(cContent); contents.push_back(*cppContent); - linphone_content_unref(cContent); } sal_body_handler_unref(sbh); - linphone_content_unref(cContent); return contents; } @@ -67,7 +65,6 @@ Content ContentManager::contentListToMultipart (const list &contents) LinphoneContent *cContent = L_GET_C_BACK_PTR(content); SalBodyHandler *sbh = sal_body_handler_from_content(cContent); belle_sip_multipart_body_handler_add_part(mpbh, BELLE_SIP_BODY_HANDLER(sbh)); - linphone_content_unref(cContent); } SalBodyHandler *sbh = (SalBodyHandler *)mpbh; @@ -77,7 +74,6 @@ Content ContentManager::contentListToMultipart (const list &contents) LinphoneContent *cContent = linphone_content_from_sal_body_handler(sbh); Content *content = L_GET_CPP_PTR_FROM_C_OBJECT(cContent); Content returnContent = *content; - linphone_content_unref(cContent); belle_sip_object_unref(mpbh); return returnContent; diff --git a/tester/message_tester.c b/tester/message_tester.c index df264123a..17dc41e43 100644 --- a/tester/message_tester.c +++ b/tester/message_tester.c @@ -66,10 +66,9 @@ void message_received(LinphoneCore *lc, LinphoneChatRoom *room, LinphoneChatMess } counters->last_received_chat_message=linphone_chat_message_ref(msg); LinphoneContent * content = linphone_chat_message_get_file_transfer_information(msg); - if (content) { + if (content) counters->number_of_LinphoneMessageReceivedWithFile++; - linphone_content_unref(content); - } else if (linphone_chat_message_get_external_body_url(msg)) { + else if (linphone_chat_message_get_external_body_url(msg)) { counters->number_of_LinphoneMessageExtBodyReceived++; if (message_external_body_url) { BC_ASSERT_STRING_EQUAL(linphone_chat_message_get_external_body_url(msg),message_external_body_url); @@ -834,12 +833,12 @@ void info_message_base(bool_t with_content) { info=linphone_core_create_info_message(marie->lc); linphone_info_message_add_header(info,"Weather","still bad"); if (with_content) { - LinphoneContent* ct=linphone_core_create_content(marie->lc); - linphone_content_set_type(ct,"application"); - linphone_content_set_subtype(ct,"somexml"); - linphone_content_set_buffer(ct,(const uint8_t *)info_content,strlen(info_content)); - linphone_info_message_set_content(info,ct); - linphone_content_unref(ct); + LinphoneContent* content = linphone_core_create_content(marie->lc); + linphone_content_set_type(content, "application"); + linphone_content_set_subtype(content, "somexml"); + linphone_content_set_buffer(content, (const uint8_t *)info_content, strlen(info_content)); + linphone_info_message_set_content(info, content); + linphone_content_unref(content); } linphone_call_send_info_message(linphone_core_get_current_call(marie->lc),info); linphone_info_message_unref(info); @@ -1442,7 +1441,6 @@ void lime_transfer_message_base(bool_t encrypt_file,bool_t download_file_from_st BC_ASSERT_PTR_NOT_NULL(linphone_content_get_key(content)); else BC_ASSERT_PTR_NULL(linphone_content_get_key(content)); - linphone_content_unref(content); if (use_file_body_handler_in_download) { linphone_chat_message_set_file_transfer_filepath(recv_msg, receive_filepath); @@ -1647,7 +1645,7 @@ void crash_during_file_transfer(void) { /* Create a new core and check that the message stored in the saved database is in the not delivered state */ linphone_core_manager_restart(pauline, TRUE); linphone_core_set_file_transfer_server(pauline->lc, "https://www.linphone.org:444/lft.php"); - + //BC_ASSERT_TRUE(wait_for(pauline->lc, pauline->lc, &pauline->stat.number_of_LinphoneRegistrationOk, 1)); chat_room = linphone_core_get_chat_room(pauline->lc, marie->identity); @@ -1684,9 +1682,9 @@ void crash_during_file_transfer(void) { } } - + bctbx_list_free_with_data(msg_list, (bctbx_list_free_func)linphone_chat_message_unref); - + linphone_core_manager_destroy(pauline); linphone_core_manager_destroy(marie); diff --git a/tester/multipart-tester.cpp b/tester/multipart-tester.cpp index c94dc9a2e..1c54389f2 100644 --- a/tester/multipart-tester.cpp +++ b/tester/multipart-tester.cpp @@ -86,7 +86,6 @@ static void chat_message_multipart_modifier_base(bool first_file_transfer, bool if (first_file_transfer || second_file_transfer) { LinphoneContent *content = linphone_chat_message_get_file_transfer_information(pauline->stat.last_received_chat_message); BC_ASSERT_PTR_NOT_NULL(content); - linphone_content_unref(content); } if (!first_file_transfer || !second_file_transfer) { const char *content = linphone_chat_message_get_text_content(pauline->stat.last_received_chat_message);