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 <s89.jang@samsung.com>

diff --git a/client/src/pkgmgr.c b/client/src/pkgmgr.c
index 8caabcb..d0c625f 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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;
                }
        }