From: Sangyoon Jang Date: Thu, 3 Sep 2015 11:29:40 +0000 (+0900) Subject: Fix prevent issues X-Git-Tag: submit/tizen/20150924.065914~4 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=05c5a24e6916a2ee411541b64d307454db208555;p=platform%2Fcore%2Fappfw%2Fslp-pkgmgr.git Fix prevent issues fix resource leak fix null pointer dereferences fix using uninitialized value fix control flow issues Change-Id: I0ac1937f5e26ad6929465605830ca918ce578b52 Signed-off-by: Sangyoon Jang --- diff --git a/client/src/pkgmgr.c b/client/src/pkgmgr.c index 8caabcb..d0c625f 100644 --- a/client/src/pkgmgr.c +++ b/client/src/pkgmgr.c @@ -617,7 +617,7 @@ static int __move_pkg_process(pkgmgr_client *pc, const char *pkgid, static int __check_app_process(pkgmgr_request_service_type service_type, pkgmgr_client *pc, const char *pkgid, uid_t uid, void *data) { - GVariant *result; + GVariant *result = NULL; int ret = PKGMGR_R_ECOMM; pkgmgrinfo_pkginfo_h handle; int pid = -1; @@ -737,16 +737,22 @@ static int __get_pkg_size_info_cb(uid_t target_uid, int req_id, const char *req_ char *save_ptr = NULL; char *token = strtok_r((char*)value, ":", &save_ptr); + tryvm_if(token == NULL, ret = -1, "failed to parse sizeinfo"); size_info->data_size = atoll(token); token = strtok_r(NULL, ":", &save_ptr); + tryvm_if(token == NULL, ret = -1, "failed to parse sizeinfo"); size_info->cache_size = atoll(token); token = strtok_r(NULL, ":", &save_ptr); + tryvm_if(token == NULL, ret = -1, "failed to parse sizeinfo"); size_info->app_size = atoll(token); token = strtok_r(NULL, ":", &save_ptr); + tryvm_if(token == NULL, ret = -1, "failed to parse sizeinfo"); size_info->ext_data_size = atoll(token); token = strtok_r(NULL, ":", &save_ptr); + tryvm_if(token == NULL, ret = -1, "failed to parse sizeinfo"); size_info->ext_cache_size = atoll(token); token = strtok_r(NULL, ":", &save_ptr); + tryvm_if(token == NULL, ret = -1, "failed to parse sizeinfo"); size_info->ext_app_size = atoll(token); DBG("data: %lld, cache: %lld, app: %lld, ext_data: %lld, ext_cache: %lld, ext_app: %lld", @@ -781,7 +787,8 @@ API pkgmgr_client *pkgmgr_client_new(client_type ctype) pkgmgr_client_t *pc = NULL; int ret = -1; - retvm_if(ctype != PC_REQUEST && ctype != PC_LISTENING && ctype != PC_BROADCAST, NULL, "ctype is not client_type"); + retvm_if(ctype == PC_BROADCAST, NULL, "broadcast type is not supported"); + retvm_if(ctype != PC_REQUEST && ctype != PC_LISTENING, NULL, "ctype is not client_type"); /* Allocate memory for ADT:pkgmgr_client */ pc = calloc(1, sizeof(pkgmgr_client_t)); @@ -803,9 +810,6 @@ API pkgmgr_client *pkgmgr_client_new(client_type ctype) ret = comm_client_set_status_callback(COMM_STATUS_BROADCAST_ALL, pc->info.listening.cc, __status_callback, pc); trym_if(ret < 0L, "comm_client_set_status_callback() failed - %d", ret); - } else if (pc->ctype == PC_BROADCAST) { - /* client cannot broadcast signal */ - return NULL; } return (pkgmgr_client *) pc; diff --git a/server/src/pkgmgr-server.c b/server/src/pkgmgr-server.c index f5b9c23..350cb0e 100644 --- a/server/src/pkgmgr-server.c +++ b/server/src/pkgmgr-server.c @@ -626,7 +626,7 @@ user_ctx* get_user_context(uid_t uid) context_res = NULL; int i = 0; //env variable ends by NULL element - while (env[i]) { + while (env && env[i]) { free(env[i]); i++; } @@ -1066,12 +1066,15 @@ int main(int argc, char *argv[]) if (fgets(buf, 32, fp_status)) backend_cmd = _get_backend_cmd(buf); if (!backend_cmd) { /* if NULL, */ - DBG("fail to get" - " backend command"); + DBG("fail to get backend command"); goto err; } backend_name = strrchr(backend_cmd, '/'); + if (!backend_name) { + DBG("fail to get backend name"); + goto err; + } execl(backend_cmd, backend_name, "-r", NULL); diff --git a/server/src/pm-queue.c b/server/src/pm-queue.c index 495f82e..0100a9d 100644 --- a/server/src/pm-queue.c +++ b/server/src/pm-queue.c @@ -213,12 +213,12 @@ int _pm_queue_init(void) } /*executable*/ else { - strncpy(buf, abs_filename, MAX_PKG_NAME_LEN - 1); + snprintf(buf, sizeof(buf), "%s", abs_filename); } ret = __entry_exist(buf); if (ret == -1) { - strncpy(ptr->backend, buf, MAX_PKG_NAME_LEN - 1); - strncpy(ptr->pkgtype, namelist[n]->d_name, MAX_PKG_TYPE_LEN - 1); + snprintf(ptr->backend, sizeof(ptr->backend), "%s", buf); + snprintf(ptr->pkgtype, sizeof(ptr->pkgtype), "%s", namelist[n]->d_name); ptr->queue_slot = slot; ptr->head = NULL; entries++; @@ -226,8 +226,8 @@ int _pm_queue_init(void) ptr++; } else { - strncpy(ptr->backend, buf, MAX_PKG_NAME_LEN - 1); - strncpy(ptr->pkgtype, namelist[n]->d_name, MAX_PKG_TYPE_LEN - 1); + snprintf(ptr->backend, sizeof(ptr->backend), "%s", buf); + snprintf(ptr->pkgtype, sizeof(ptr->pkgtype), "%s", namelist[n]->d_name); ptr->queue_slot = ret; ptr->head = NULL; entries++; @@ -278,12 +278,12 @@ int _pm_queue_push(uid_t uid, const char *req_id, int req_type, return -1; } - strncpy(data->msg->req_id, req_id, strlen(req_id)); + snprintf(data->msg->req_id, sizeof(data->msg->req_id), "%s", req_id); data->msg->req_type = req_type; data->msg->uid = uid; - strncpy(data->msg->pkg_type, pkg_type, strlen(pkg_type)); - strncpy(data->msg->pkgid, pkgid, strlen(pkgid)); - strncpy(data->msg->args, args, strlen(args)); + snprintf(data->msg->pkg_type, sizeof(data->msg->pkg_type), "%s", pkg_type); + snprintf(data->msg->pkgid, sizeof(data->msg->pkgid), "%s", pkgid); + snprintf(data->msg->args, sizeof(data->msg->args), "%s", args); data->next = NULL; @@ -331,12 +331,12 @@ pm_dbus_msg *_pm_queue_pop(int position) return ret; } - strncpy(ret->req_id, cur->msg->req_id, strlen(cur->msg->req_id)); + snprintf(ret->req_id, sizeof(ret->req_id), "%s", cur->msg->req_id); ret->req_type = cur->msg->req_type; ret->uid = cur->msg->uid; - strncpy(ret->pkg_type, cur->msg->pkg_type, strlen(cur->msg->pkg_type)); - strncpy(ret->pkgid, cur->msg->pkgid, strlen(cur->msg->pkgid)); - strncpy(ret->args, cur->msg->args, strlen(cur->msg->args)); + snprintf(ret->pkg_type, sizeof(ret->pkg_type), "%s", cur->msg->pkg_type); + snprintf(ret->pkgid, sizeof(ret->pkgid), "%s", cur->msg->pkgid); + snprintf(ret->args, sizeof(ret->args), "%s", cur->msg->args); ptr->head = cur->next; saveptr = ptr->head; diff --git a/server/src/request.c b/server/src/request.c index c8cce3a..f6a79d3 100644 --- a/server/src/request.c +++ b/server/src/request.c @@ -121,6 +121,8 @@ static int __handle_request_install(uid_t uid, } reqkey = __generate_reqkey(pkgpath); + if (reqkey == NULL) + return -1; if (_pm_queue_push(uid, reqkey, PKGMGR_REQUEST_TYPE_INSTALL, pkgtype, pkgpath, "")) { g_dbus_method_invocation_return_value(invocation, @@ -151,6 +153,8 @@ static int __handle_request_reinstall(uid_t uid, } reqkey = __generate_reqkey(pkgid); + if (reqkey == NULL) + return -1; if (_pm_queue_push(uid, reqkey, PKGMGR_REQUEST_TYPE_REINSTALL, pkgtype, pkgid, "")) { g_dbus_method_invocation_return_value(invocation, @@ -181,6 +185,8 @@ static int __handle_request_uninstall(uid_t uid, } reqkey = __generate_reqkey(pkgid); + if (reqkey == NULL) + return -1; if (_pm_queue_push(uid, reqkey, PKGMGR_REQUEST_TYPE_UNINSTALL, pkgtype, pkgid, "")) { g_dbus_method_invocation_return_value(invocation, @@ -211,6 +217,8 @@ static int __handle_request_move(uid_t uid, } reqkey = __generate_reqkey(pkgid); + if (reqkey == NULL) + return -1; if (_pm_queue_push(uid, reqkey, PKGMGR_REQUEST_TYPE_MOVE, pkgtype, pkgid, "")) { g_dbus_method_invocation_return_value(invocation, @@ -292,6 +300,9 @@ static int __handle_request_getsize(uid_t uid, } reqkey = __generate_reqkey(pkgid); + if (reqkey == NULL) + return -1; + snprintf(buf, sizeof(buf), "%d", get_type); if (_pm_queue_push(uid, reqkey, PKGMGR_REQUEST_TYPE_GETSIZE, "getsize", pkgid, buf)) { @@ -485,16 +496,19 @@ static const GDBusInterfaceVTable interface_vtable = static void __on_bus_acquired(GDBusConnection *connection, const gchar *name, gpointer user_data) { + GError *err = NULL; DBG("on bus acquired"); reg_id = g_dbus_connection_register_object(connection, COMM_PKGMGR_DBUS_OBJECT_PATH, instropection_data->interfaces[0], - &interface_vtable, NULL, NULL, NULL); + &interface_vtable, NULL, NULL, &err); - if (reg_id < 0) - ERR("failed to register object"); + if (reg_id == 0) { + ERR("failed to register object: %s", err->message); + g_error_free(err); + } } static void __on_name_acquired(GDBusConnection *connection, const gchar *name, diff --git a/tool/pkg_clearcache.c b/tool/pkg_clearcache.c index db98be2..9a20dd9 100644 --- a/tool/pkg_clearcache.c +++ b/tool/pkg_clearcache.c @@ -121,14 +121,9 @@ static int __clear_cache_dir(const char *pkgid) return -1; } - int internal_prefix_len = sizeof(INTERNAL_CACHE_PATH_PREFIX); - int cache_postfix_len = sizeof(CACHE_PATH_POSTFIX); - int shared_postfix_len = sizeof(SHARED_PATH_POSTFIX); - // cache internal - strcat(dirname, INTERNAL_CACHE_PATH_PREFIX); - strncat(dirname, pkgid, PATH_MAX - internal_prefix_len - cache_postfix_len - 1); - strcat(dirname, CACHE_PATH_POSTFIX); + snprintf(dirname, sizeof(dirname), "%s/%s%s", + INTERNAL_CACHE_PATH_PREFIX, pkgid, CACHE_PATH_POSTFIX); ret = __clear_dir(dirname); if (ret < 0) { @@ -136,10 +131,8 @@ static int __clear_cache_dir(const char *pkgid) } // shared/cache internal - memset(dirname, 0x00, PATH_MAX); - strcat(dirname, INTERNAL_CACHE_PATH_PREFIX); - strncat(dirname, pkgid, PATH_MAX - internal_prefix_len - shared_postfix_len - 1); - strcat(dirname, SHARED_PATH_POSTFIX); + snprintf(dirname, sizeof(dirname), "%s/%s%s", + INTERNAL_CACHE_PATH_PREFIX, pkgid, SHARED_PATH_POSTFIX); ret = __clear_dir(dirname); if (ret < 0) { diff --git a/tool/pkg_cmd.c b/tool/pkg_cmd.c index 6baeeb9..4757e52 100644 --- a/tool/pkg_cmd.c +++ b/tool/pkg_cmd.c @@ -866,7 +866,8 @@ int main(int argc, char *argv[]) case 'S': /* csc packages */ data.request = CSC_REQ; if (optarg) - strncpy(data.des_path, optarg, PKG_NAME_STRING_LEN_MAX); + snprintf(data.des_path, sizeof(data.des_path), + "%s", optarg); printf("csc file is %s\n", data.des_path); break; @@ -881,8 +882,8 @@ int main(int argc, char *argv[]) case 'L': /* activate with Label */ data.request = ACTIVATE_REQ; if (optarg) - strncpy(data.label, optarg, - PKG_NAME_STRING_LEN_MAX); + snprintf(data.pkg_path, sizeof(data.pkg_path), + "%s", optarg); break; case 'a': /* app installation path */ @@ -907,8 +908,8 @@ int main(int argc, char *argv[]) case 'p': /* package path */ if (optarg) - strncpy(data.pkg_path, optarg, - PKG_NAME_STRING_LEN_MAX); + snprintf(data.pkg_path, sizeof(data.pkg_path), + "%s", optarg); ret = __convert_to_absolute_path(data.pkg_path); if (ret == -1) { printf("conversion of relative path to absolute path failed\n"); @@ -919,20 +920,20 @@ int main(int argc, char *argv[]) case 'd': /* descriptor path */ if (optarg) - strncpy(data.des_path, optarg, - PKG_NAME_STRING_LEN_MAX); + snprintf(data.des_path, sizeof(data.des_path), + "%s", optarg); break; case 'n': /* package name */ if (optarg) - strncpy(data.pkgid, optarg, - PKG_NAME_STRING_LEN_MAX); + snprintf(data.pkgid, sizeof(data.pkgid), + "%s", optarg); break; case 't': /* package type */ if (optarg) - strncpy(data.pkg_type, optarg, - PKG_TYPE_STRING_LEN_MAX); + snprintf(data.pkg_type, sizeof(data.pkg_type), + "%s", optarg); break; case 'T': /* move type */ diff --git a/tool/pkg_info.c b/tool/pkg_info.c index 04b699c..1d122c4 100644 --- a/tool/pkg_info.c +++ b/tool/pkg_info.c @@ -1140,9 +1140,9 @@ static int __get_certinfo_from_db(char *pkgid, uid_t uid) ret = pkgmgrinfo_pkginfo_destroy_certinfo(handle); if (ret < 0) { printf("pkgmgrinfo_pkginfo_destroy_certinfo failed\n"); - return -1; + ret = -1; } - return 0; + break; case 1: case 2: case 3: @@ -1155,21 +1155,24 @@ static int __get_certinfo_from_db(char *pkgid, uid_t uid) ret = pkgmgrinfo_pkginfo_get_cert_value(handle, choice - 1, &value); if (value) printf("cert type[%d] value = %s\n", choice - 1, value); + if (ret < 0) + ret = -1; break; case 10: ret = pkgmgrinfo_pkginfo_destroy_certinfo(handle); if (ret < 0) { printf("pkgmgrinfo_pkginfo_destroy_certinfo failed\n"); - return -1; + ret = -1; } - return 0; + break; default: printf("Invalid choice entered\n"); - return -1; + ret = -1; + break; } } - return -1; + return ret; } static int __compare_pkg_certinfo_from_db(char *lhs_pkgid, char *rhs_pkgid, uid_t uid) diff --git a/tool/pkg_initdb.c b/tool/pkg_initdb.c index c897dba..91e32e9 100644 --- a/tool/pkg_initdb.c +++ b/tool/pkg_initdb.c @@ -97,6 +97,7 @@ static int _initdb_load_directory(uid_t uid, const char *directory) if (setresuid(uid, uid, OWNER_ROOT)) { _E("Failed to setresuid: %s", strerror_r(errno, buf, sizeof(buf))); + closedir(dir); return -1; } snprintf(buf2, sizeof(buf2), "%s %s", PKGINFO_CMD, buf); @@ -111,6 +112,7 @@ static int _initdb_load_directory(uid_t uid, const char *directory) if (setresuid(OWNER_ROOT, OWNER_ROOT, OWNER_ROOT)) { _E("Failed to setresuid: %s", strerror_r(errno, buf, sizeof(buf))); + closedir(dir); return -1; } }