From 2895bce686826d04a35fe9d38bf99c5ecb3b8f4a Mon Sep 17 00:00:00 2001 From: Ilho Kim Date: Thu, 10 Jun 2021 10:26:22 +0900 Subject: [PATCH] Fix static analysis issues - Fix memory leak - Fix buffer not null terminated - Fix double free - The out parameter is not modified in the error case Change-Id: I24266271e42c249c2d96b3ec764bf1a61d87aa43 Signed-off-by: Ilho Kim --- parser/src/pkgmgr_parser_deprecated.c | 4 ++++ src/common/socket/server_socket.cc | 2 +- src/manager/pkginfo_manager.cc | 40 ++++++++++++++++++++++++++--------- src/pkgmgrinfo_updateinfo.c | 1 - 4 files changed, 35 insertions(+), 12 deletions(-) diff --git a/parser/src/pkgmgr_parser_deprecated.c b/parser/src/pkgmgr_parser_deprecated.c index 442d329..0dd7367 100644 --- a/parser/src/pkgmgr_parser_deprecated.c +++ b/parser/src/pkgmgr_parser_deprecated.c @@ -2011,6 +2011,7 @@ DEPRECATED API int pkgmgr_parser_parse_manifest_for_upgrade(const char *manifest if (ret != PMINFO_R_OK) { _LOGD("Cert Info DB Delete Failed\n"); pkgmgr_parser_free_manifest_xml(mfx); + pkgmgrinfo_pkginfo_destroy_pkginfo(handle); return PMINFO_R_ERROR; } @@ -2018,6 +2019,7 @@ DEPRECATED API int pkgmgr_parser_parse_manifest_for_upgrade(const char *manifest if (ret != PMINFO_R_OK) { _LOGD("DB Insert failed\n"); pkgmgr_parser_free_manifest_xml(mfx); + pkgmgrinfo_pkginfo_destroy_pkginfo(handle); return PMINFO_R_ERROR; } @@ -2091,6 +2093,7 @@ DEPRECATED API int pkgmgr_parser_parse_usr_manifest_for_upgrade(const char *mani if (ret != PMINFO_R_OK) { _LOGD("Cert Info DB Delete Failed\n"); pkgmgr_parser_free_manifest_xml(mfx); + pkgmgrinfo_pkginfo_destroy_pkginfo(handle); return PMINFO_R_ERROR; } @@ -2098,6 +2101,7 @@ DEPRECATED API int pkgmgr_parser_parse_usr_manifest_for_upgrade(const char *mani if (ret != PMINFO_R_OK) { _LOGD("DB Insert failed\n"); pkgmgr_parser_free_manifest_xml(mfx); + pkgmgrinfo_pkginfo_destroy_pkginfo(handle); return PMINFO_R_ERROR; } diff --git a/src/common/socket/server_socket.cc b/src/common/socket/server_socket.cc index 2a85a9e..3b7a5df 100644 --- a/src/common/socket/server_socket.cc +++ b/src/common/socket/server_socket.cc @@ -45,7 +45,7 @@ ServerSocket::ServerSocket(std::string path) : AbstractSocket(std::move(path)) { } addr_.sun_family = AF_UNIX; - strncpy(addr_.sun_path, path_.c_str(), sizeof(addr_.sun_path)); + snprintf(addr_.sun_path, sizeof(addr_.sun_path), "%s", path_.c_str()); if (access(path_.c_str(), F_OK) == 0) { LOGW("socket(%s) is already used", path_.c_str()); unlink(path_.c_str()); diff --git a/src/manager/pkginfo_manager.cc b/src/manager/pkginfo_manager.cc index 218f29c..ee1d5eb 100644 --- a/src/manager/pkginfo_manager.cc +++ b/src/manager/pkginfo_manager.cc @@ -270,12 +270,21 @@ extern "C" EXPORT_API int _appinfo_get_datacontrol_info( for (auto& result : result_list) { if (result.size() != 2 || result.front().empty() || result.back().empty()) return PMINFO_R_ERROR; - *appid = strdup(result.front().c_str()); - *access = strdup(result.back().c_str()); - if (*appid == nullptr || *access == nullptr) { + + char* tmp_appid = strdup(result.front().c_str()); + if (tmp_appid == nullptr) { + LOG(ERROR) << "Out of memory"; + return PMINFO_R_ERROR; + } + char* tmp_access = strdup(result.back().c_str()); + if (tmp_access == nullptr) { LOG(ERROR) << "Out of memory"; + free(tmp_appid); return PMINFO_R_ERROR; } + *appid = tmp_appid; + *access = tmp_access; + break; } return PMINFO_R_OK; @@ -371,12 +380,21 @@ extern "C" EXPORT_API int _appinfo_get_datacontrol_trusted_info( for (auto& result : result_list) { if (result.size() != 2 || result.front().empty() || result.back().empty()) return PMINFO_R_ERROR; - *appid = strdup(result.front().c_str()); - *trusted = strdup(result.back().c_str()); - if (*appid == nullptr || *trusted == nullptr) { + + char* tmp_appid = strdup(result.front().c_str()); + if (tmp_appid == nullptr) { + LOG(ERROR) << "Out of memory"; + return PMINFO_R_ERROR; + } + char* tmp_trusted = strdup(result.back().c_str()); + if (tmp_trusted == nullptr) { LOG(ERROR) << "Out of memory"; + free(tmp_appid); return PMINFO_R_ERROR; } + *appid = tmp_appid; + *trusted = tmp_trusted; + break; } return PMINFO_R_OK; @@ -630,16 +648,18 @@ extern "C" EXPORT_API int _get_pkg_updateinfo(const char* pkgid, if (result_list.size() == 0) return PMINFO_R_ENOENT; + GSList* tmp_list = nullptr; for (auto& result : result_list) { if (result.size() != 3) { LOG(ERROR) << "Invalid result"; - g_slist_free_full(*update_info_list, __free_update_info); + g_slist_free_full(tmp_list, __free_update_info); return PMINFO_R_ERROR; } updateinfo_x* update_info = reinterpret_cast( calloc(1, sizeof(updateinfo_x))); if (update_info == nullptr) { LOG(ERROR) << "Out of memory"; + g_slist_free_full(tmp_list, __free_update_info); return PMINFO_R_ERROR; } update_info->pkgid = strdup(result[0].c_str()); @@ -648,14 +668,14 @@ extern "C" EXPORT_API int _get_pkg_updateinfo(const char* pkgid, ret = __convert_update_type(result[2].c_str(), &convert_type); if (ret != 0) { __free_update_info(update_info); - g_slist_free_full(*update_info_list, __free_update_info); + g_slist_free_full(tmp_list, __free_update_info); return PMINFO_R_ERROR; } update_info->type = static_cast(convert_type); - *update_info_list = g_slist_prepend(*update_info_list, - update_info); + tmp_list = g_slist_prepend(tmp_list, update_info); } + *update_info_list = tmp_list; return PMINFO_R_OK; } diff --git a/src/pkgmgrinfo_updateinfo.c b/src/pkgmgrinfo_updateinfo.c index 2eba0b5..068a1f6 100644 --- a/src/pkgmgrinfo_updateinfo.c +++ b/src/pkgmgrinfo_updateinfo.c @@ -239,7 +239,6 @@ API int pkgmgrinfo_updateinfo_usr_foreach_updateinfo(uid_t uid, ret = _get_pkg_updateinfo(NULL, &info_list, uid); if (ret != 0) { _LOGE("Failed to get pkg update info for user[%d]", (int)uid); - g_slist_free_full(info_list, __free_update_info); return PMINFO_R_ERROR; } -- 2.7.4