From ad30032ee1490d291e272356abfdbec5f62aff9c Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Fri, 19 Nov 2021 09:11:23 +0900 Subject: [PATCH] Fix static analysis issues - Use after free - Resource leak Change-Id: I5483ac575b66fe12eec08a82382127d829c78da1 Signed-off-by: Hwankyu Jhun --- idlc/gen/c_proxy_body_gen_cb.h | 1 + idlc/gen_cion/c_cion_proxy_body_gen_cb.h | 1 + idlc/gen_cion/c_cion_proxy_header_gen_cb.h | 3 +- idlc/gen_cion/c_cion_stub_header_gen_cb.h | 5 +- idlc/gen_cion/cpp_cion_proxy_body_gen_cb.h | 9 ++- idlc/gen_cion/cpp_cion_stub_body_gen_cb.h | 120 +++++++++++++++++++++++------ 6 files changed, 110 insertions(+), 29 deletions(-) diff --git a/idlc/gen/c_proxy_body_gen_cb.h b/idlc/gen/c_proxy_body_gen_cb.h index 3d2bb86..0bb5e63 100644 --- a/idlc/gen/c_proxy_body_gen_cb.h +++ b/idlc/gen/c_proxy_body_gen_cb.h @@ -938,6 +938,7 @@ rpc_port_parcel_read(parcel_, &->parcelable, ); if (get_last_result() != RPC_PORT_ERROR_NONE) { _E("Failed to read data"); __destroy(); + = nullptr; res_ = get_last_result(); break; } diff --git a/idlc/gen_cion/c_cion_proxy_body_gen_cb.h b/idlc/gen_cion/c_cion_proxy_body_gen_cb.h index 05268f9..638d9bd 100644 --- a/idlc/gen_cion/c_cion_proxy_body_gen_cb.h +++ b/idlc/gen_cion/c_cion_proxy_body_gen_cb.h @@ -1001,6 +1001,7 @@ rpc_port_parcel_read(parcel_, &->parcelable, ); if (get_last_result() != CION_ERROR_NONE) { _E("Failed to read data"); __destroy(); + = nullptr; res_ = get_last_result(); goto out; } diff --git a/idlc/gen_cion/c_cion_proxy_header_gen_cb.h b/idlc/gen_cion/c_cion_proxy_header_gen_cb.h index 2a9278e..352ff08 100644 --- a/idlc/gen_cion/c_cion_proxy_header_gen_cb.h +++ b/idlc/gen_cion/c_cion_proxy_header_gen_cb.h @@ -286,7 +286,7 @@ void ___file_received_cb(const cion_peer_info_h peer_info, const c data_path = app_get_data_path(); if (data_path == NULL) { LOGE("failed to get data path."); - return; + goto end; } snprintf(data_path_buff, 512, "%s%s", data_path, file_name); @@ -298,6 +298,7 @@ void ___file_received_cb(const cion_peer_info_h peer_info, const c app_id, file_name, bytes, total); } +end: free(name); free(app_id); diff --git a/idlc/gen_cion/c_cion_stub_header_gen_cb.h b/idlc/gen_cion/c_cion_stub_header_gen_cb.h index 79bd711..80c8607 100644 --- a/idlc/gen_cion/c_cion_stub_header_gen_cb.h +++ b/idlc/gen_cion/c_cion_stub_header_gen_cb.h @@ -224,7 +224,7 @@ void ___file_received_cb(const cion_peer_info_h peer_info, const c data_path = app_get_data_path(); if (data_path == NULL) { LOGE("failed to get data path."); - return; + goto end; } snprintf(data_path_buff, 512, "%s%s", data_path, file_name); @@ -236,10 +236,11 @@ void ___file_received_cb(const cion_peer_info_h peer_info, const c app_id, file_name, bytes, total); } +end: free(name); free(app_id); - } + * @endcode */ typedef void (*__file_received_cb)(const cion_peer_info_h peer_info, const cion_payload_h payload, diff --git a/idlc/gen_cion/cpp_cion_proxy_body_gen_cb.h b/idlc/gen_cion/cpp_cion_proxy_body_gen_cb.h index 1a628ce..294df1f 100644 --- a/idlc/gen_cion/cpp_cion_proxy_body_gen_cb.h +++ b/idlc/gen_cion/cpp_cion_proxy_body_gen_cb.h @@ -262,6 +262,8 @@ void ##::OnDiscoveredCB(const char *service_name, return; } + auto app_id_auto = std::unique_ptr( + app_id, std::free); if (std::string(app_id) == cl->target_appid_) { ret = cion_peer_info_clone(peer_info, &cl->peer_); if (ret != CION_ERROR_NONE) { @@ -274,9 +276,8 @@ void ##::OnDiscoveredCB(const char *service_name, } void ##::OnPayloadAsyncResultCB(const cion_payload_async_result_h result, - void *user_data) { - - } + void* user_data) { +} void ##::OnPayloadReceivedCB(const char *service_name, const cion_peer_info_h peer_info, const cion_payload_h payload, @@ -290,6 +291,8 @@ void ##::OnPayloadReceivedCB(const char *service_name, return; } + auto app_id_auto = std::unique_ptr( + app_id, std::free); if (std::string(app_id) != cl->target_appid_) { _E("peer is wrong"); return; diff --git a/idlc/gen_cion/cpp_cion_stub_body_gen_cb.h b/idlc/gen_cion/cpp_cion_stub_body_gen_cb.h index 929564c..f6a886e 100644 --- a/idlc/gen_cion/cpp_cion_stub_body_gen_cb.h +++ b/idlc/gen_cion/cpp_cion_stub_body_gen_cb.h @@ -145,15 +145,39 @@ void ##::OnConnectionResultCB(const char *service_name, void ##::OnDisconnectedCB(const char *service_name, const cion_peer_info_h peer_info, void *user_data) { ##* stub = static_cast<##*>(user_data); - char *peer_app_id, *peer_uuid; - cion_peer_info_get_app_id(peer_info, &peer_app_id); - cion_peer_info_get_app_id(peer_info, &peer_uuid); - - for (auto s = stub->services_.begin(); - s != stub->services_.end(); s++) { - char *service_app_id, *service_uuid; - cion_peer_info_get_app_id(s->get()->GetPeer(), &service_app_id); - cion_peer_info_get_app_id(s->get()->GetPeer(), &service_uuid); + char* peer_app_id = nullptr; + int ret = cion_peer_info_get_app_id(peer_info, &peer_app_id); + if (ret != CION_ERROR_NONE) { + _E("cion_peer_info_get_app_id() is failed. error(%d)", ret); + return; + } + auto perr_app_id_auto = std::unique_ptr(peer_app_id, std::free); + + char* peer_uuid = nullptr; + ret = cion_peer_info_get_uuid(peer_info, &peer_uuid); + if (ret != CION_ERROR_NONE) { + _E("cion_peer_info_get_uuid() is failed. error(%d)", ret); + return; + } + auto peer_uuid_auto = std::unique_ptr(peer_uuid, std::free); + + for (auto s = stub->services_.begin(); s != stub->services_.end(); s++) { + char* service_app_id = nullptr; + ret = cion_peer_info_get_app_id(s->get()->GetPeer(), &service_app_id); + if (ret != CION_ERROR_NONE) { + _E("cion_peer_info_get_app_id() is failed. error(%d)", ret); + return; + } + auto service_app_id_auto = std::unique_ptr(service_app_id, std::free); + + char* service_uuid = nullptr;; + ret = cion_peer_info_get_uuid(s->get()->GetPeer(), &service_uuid); + if (ret != CION_ERROR_NONE) { + _E("cion_peeer_info_get_uuid() is failed. error(%d)", ret); + return; + } + auto service_uuid_auto = std::unique_ptr(service_uuid, std::free); + if (strcmp(peer_app_id, service_app_id) == 0 && strcmp(peer_uuid, service_uuid) == 0) { stub->services_.erase(s); @@ -311,14 +335,39 @@ void $$::OnPayloadReceivedCB(const char *service_name, $$* stub = static_cast<$$*>(user_data); std::shared_ptr b; rpc_port_parcel_h p; - char *peer_app_id, *peer_uuid; - cion_peer_info_get_app_id(peer_info, &peer_app_id); - cion_peer_info_get_app_id(peer_info, &peer_uuid); + char* peer_app_id = nullptr; + int ret = cion_peer_info_get_app_id(peer_info, &peer_app_id); + if (ret != CION_ERROR_NONE) { + _E("cion_peer_info_get_app_id() is failed. error(%d)", ret); + return; + } + auto peer_app_id_auto = std::unique_ptr(peer_app_id, std::free); + + char* peer_uuid = nullptr;; + ret = cion_peer_info_get_uuid(peer_info, &peer_uuid); + if (ret != CION_ERROR_NONE) { + _E("cion_peer_info_get_uuid() is failed. error(%d)", ret); + return; + } + auto peer_uuid_auto = std::unique_ptr(peer_uuid, std::free); for (auto& s : stub->services_) { - char *service_app_id, *service_uuid; - cion_peer_info_get_app_id(s->GetPeer(), &service_app_id); - cion_peer_info_get_app_id(s->GetPeer(), &service_uuid); + char* service_app_id = nullptr; + ret = cion_peer_info_get_app_id(s->GetPeer(), &service_app_id); + if (ret != CION_ERROR_NONE) { + _E("cion_peer_info_get_app_id() is failed. error(%d)", ret); + return; + } + auto service_app_id_auto = std::unique_ptr(service_app_id, std::free); + + char* service_uuid = nullptr; + ret = cion_peer_info_get_uuid(s->GetPeer(), &service_uuid); + if (ret != CION_ERROR_NONE) { + _E("cion_peer_info_get_uuid() is failed. error(%d)", ret); + return; + } + auto service_uuid_auto = std::unique_ptr(service_uuid, std::free); + if (strcmp(peer_app_id, service_app_id) == 0 && strcmp(peer_uuid, service_uuid) == 0) { b = s; @@ -332,7 +381,7 @@ void $$::OnPayloadReceivedCB(const char *service_name, } cion_payload_type_e type; - int ret = cion_payload_get_type(payload, &type); + ret = cion_payload_get_type(payload, &type); if (ret != CION_ERROR_NONE) { _E("Failed to cion_payload_get_type. error(%d)", ret); return; @@ -383,14 +432,39 @@ void $$::OnDataReceivedCB(const char *service_name, $$* stub = static_cast<$$*>(user_data); std::shared_ptr b; rpc_port_parcel_h p; - char *peer_app_id, *peer_uuid; - cion_peer_info_get_app_id(peer_info, &peer_app_id); - cion_peer_info_get_app_id(peer_info, &peer_uuid); + char* peer_app_id = nullptr; + int ret = cion_peer_info_get_app_id(peer_info, &peer_app_id); + if (ret != CION_ERROR_NONE) { + _E("cion_peer_info_get_app_id() is failed. error(%d)", ret); + return; + } + auto peer_app_id_auto = std::unique_ptr(peer_app_id, std::free); + + char* peer_uuid = nullptr; + ret = cion_peer_info_get_uuid(peer_info, &peer_uuid); + if (ret != CION_ERROR_NONE) { + _E("cion_peer_info_get_uuid() is failed. error(%d)", ret); + return; + } + auto peer_uuid_auto = std::unique_ptr(peer_uuid, std::free); for (auto& s : stub->services_) { - char *service_app_id, *service_uuid; - cion_peer_info_get_app_id(s->GetPeer(), &service_app_id); - cion_peer_info_get_app_id(s->GetPeer(), &service_uuid); + char* service_app_id = nullptr; + ret = cion_peer_info_get_app_id(s->GetPeer(), &service_app_id); + if (ret != CION_ERROR_NONE) { + _E("cion_peer_info_get_app_id() is failed. error(%d)", ret); + return; + } + auto service_app_id_auto = std::unique_ptr(service_app_id, std::free); + + char* service_uuid = nullptr; + ret = cion_peer_info_get_uuid(s->GetPeer(), &service_uuid); + if (ret != CION_ERROR_NONE) { + _E("cion_peer_info_get_uuid() is failed. error(%d)", ret); + return; + } + auto service_uuid_auto = std::unique_ptr(service_uuid, std::free); + if (strcmp(peer_app_id, service_app_id) == 0 && strcmp(peer_uuid, service_uuid) == 0) { b = s; @@ -403,7 +477,7 @@ void $$::OnDataReceivedCB(const char *service_name, return; } - int ret = rpc_port_parcel_create_from_raw(&p, data, data_size); + ret = rpc_port_parcel_create_from_raw(&p, data, data_size); if (ret != 0) { _E("Failed to create parcel from port"); return; -- 2.7.4