[FIX] prevent issues

| Type                       | Function                         |
| String not null terminated | get_process_cmd_line             |
| Resource leak              | initialize_log                   |
| Resource leak              | update_thread_data               |
| Resource leak              | parse_lib_inst_list              |
| Resource leak              | parse_app_inst_list              |
| Resource leak              | get_build_dir                    |
| Resource leak              | process_msg_get_process_add_info |
| Resource leak              | process_msg_get_process_add_info |
| Resource leak              | process_msg_get_process_add_info |
| Out-of-bounds access       | parse_replay_event               |
| Out-of-bounds access       | parse_replay_event               |

Change-Id: I201baf8042a008e41a8f12f699b7a64740fe3e24
Signed-off-by: Vitaliy Cherepanov <v.cherepanov@samsung.com>

diff --git a/daemon/da_data.c b/daemon/da_data.c
index 8d27f15..f0a630d 100644 (file)
--- a/daemon/da_data.c
+++ b/daemon/da_data.c
@@ -262,6 +262,7 @@ struct msg_data_t *gen_message_event(
        p = data->payload;
        pack_int32(p, events_count);
 
+       /* FIXME events[i].type, events[i].code should be uint16_t */
        for (i=0; i<events_count; i++){
                pack_int32(p,id);
                pack_int32(p, (int32_t) events[i].type);

diff --git a/daemon/da_debug.c b/daemon/da_debug.c
index 420ebc2..15a019a 100644 (file)
--- a/daemon/da_debug.c
+++ b/daemon/da_debug.c
@@ -49,7 +49,7 @@ static inline void close_on_exec_dup(int old, int new)
 void initialize_log(void)
 {
        int fd = open(DEBUG_LOGFILE, O_WRONLY | O_CREAT | O_TRUNC, 0777);
-       if (fd > 0) {
+       if (fd != -1) {
                close_on_exec_dup(fd, 1);
                close_on_exec_dup(fd, 2);
 

diff --git a/daemon/da_protocol.c b/daemon/da_protocol.c
index 9b210d4..bb97f4f 100644 (file)
--- a/daemon/da_protocol.c
+++ b/daemon/da_protocol.c
@@ -347,6 +347,7 @@ static int parse_timeval(struct msg_buf_t *msg, struct timeval *tv)
 static int parse_replay_event(struct msg_buf_t *msg,
                                    struct replay_event_t *re)
 {
+       uint32_t dummy;
 
        if (!parse_timeval(msg, &re->ev.time)) {
                LOGE("time parsing error\n");
@@ -358,15 +359,18 @@ static int parse_replay_event(struct msg_buf_t *msg,
                return 0;
        }
 
-       if (!parse_int32(msg, (uint32_t *)&re->ev.type)) {
+       /* FIXME ev.type, ev.code should be uint16_t */
+       if (!parse_int32(msg, &dummy)) {
                LOGE("type parsing error\n");
                return 0;
        }
+       re->ev.type = (uint16_t)dummy;
 
-       if (!parse_int32(msg, (uint32_t *)&re->ev.code)) {
+       if (!parse_int32(msg, &dummy)) {
                LOGE("code parsing error\n");
                return 0;
        }
+       re->ev.code = (uint16_t)dummy;
 
        if (!parse_int32(msg, (uint32_t *)&re->ev.value)) {
                LOGE("value parsing error\n");
@@ -1042,8 +1046,9 @@ static char *get_process_cmd_line(uint32_t pid)
        f = open(buf, O_RDONLY);
        if (f != -1) {
                count = read(f, buf, sizeof(buf));
-               if (count == 0)
-                       buf[0] = '\0';
+               if (count >= sizeof(buf))
+                       count = sizeof(buf) - 1;
+               buf[count] = '\0';
                close(f);
        } else {
                LOGE("file not found <%s>\n", buf);
@@ -1065,7 +1070,7 @@ static int process_msg_get_process_add_info(struct msg_buf_t *msg)
        if (!parse_int32(msg, &count)) {
                LOGE("NMSG_GET_PROCESS_ADD_INFO error: No process count\n");
                err_code = ERR_WRONG_MESSAGE_DATA;
-               goto send_ack;
+               goto send_fail;
        }
 
        /* alloc array for pids */
@@ -1073,18 +1078,18 @@ static int process_msg_get_process_add_info(struct msg_buf_t *msg)
        cmd_line_arr = malloc(count * sizeof(*cmd_line_arr));
        if (pidarr == NULL) {
                LOGE("can not alloc pid array (%u)", count);
-               goto send_ack;
+               goto send_fail;
        }
        if (cmd_line_arr == NULL) {
                LOGE("can not alloc cmd line array (%u)", count);
-               goto send_fail_parse;
+               goto send_fail;
        }
 
        /* parse all pids */
        for (i = 0; i != count; i++) {
                if (!parse_int32(msg, &pidarr[i])) {
                        LOGE("can not parse pid #%u", i);
-                       goto send_fail_parse;
+                       goto send_fail;
                }
        }
 
@@ -1096,7 +1101,7 @@ static int process_msg_get_process_add_info(struct msg_buf_t *msg)
 
        payload = malloc(total_len);
        if (payload == NULL)
-               goto send_fail_payload;
+               goto send_fail;
        /* pack payload data */
        p = payload;
        pack_int32(p, count);
@@ -1109,14 +1114,20 @@ static int process_msg_get_process_add_info(struct msg_buf_t *msg)
        /* success */
        goto send_ack;
 
-send_fail_payload:
+send_fail:
+       /* fail */
+       total_len = 0;
+
+send_ack:
+       /* success */
+       sendACKToHost(NMSG_GET_PROCESS_ADD_INFO, err_code, payload, total_len);
+
+       /* free data */
        if (payload != NULL) {
                free(payload);
                payload = NULL;
        }
 
-send_fail_parse:
-       /* fail */
        if (pidarr != NULL) {
                free(pidarr);
                pidarr = NULL;
@@ -1127,11 +1138,6 @@ send_fail_parse:
                cmd_line_arr = NULL;
        }
 
-       total_len = 0;
-
-send_ack:
-       /* success */
-       sendACKToHost(NMSG_GET_PROCESS_ADD_INFO, err_code, payload, total_len);
        return -(err_code != ERR_NO);
 }
 

diff --git a/daemon/da_protocol_inst.c b/daemon/da_protocol_inst.c
index 13a03f9..26ffb35 100644 (file)
--- a/daemon/da_protocol_inst.c
+++ b/daemon/da_protocol_inst.c
@@ -142,27 +142,37 @@ static int parse_func_inst_list(struct msg_buf_t *msg,
 
 static int parse_inst_lib(struct msg_buf_t *msg, struct lib_list_t **dest)
 {
+       int res = 1;
        *dest = new_lib();
        if (*dest == NULL) {
                LOGE("lib alloc error\n");
-               return 0;
+               res = 0;
+               goto exit;
        };
 
        if (!parse_string(msg, &((*dest)->lib->bin_path)) ||
            !check_exec_path((*dest)->lib->bin_path))
        {
                LOGE("bin path parsing error\n");
-               return 0;
+               goto exit_free_err;
        }
 
        if (!parse_func_inst_list(msg, (struct data_list_t *) *dest)) {
                LOGE("funcs parsing error\n");
-               return 0;
+               goto exit_free_err;
        }
 
        (*dest)->size +=  strlen((*dest)->lib->bin_path) + 1 + sizeof((*dest)->func_num);
        (*dest)->hash = calc_lib_hash((*dest)->lib);
-       return 1;
+
+       goto exit;
+
+exit_free_err:
+       res = 0;
+       free(*dest);
+
+exit:
+       return res;
 
 }
 
@@ -194,13 +204,15 @@ int parse_lib_inst_list(struct msg_buf_t *msg,
 
 int parse_inst_app(struct msg_buf_t *msg, struct app_list_t **dest)
 {
+       int res = 1;
        char *start, *end;
        struct app_info_t *app_info = NULL;
        *dest = new_app();
 
        if (*dest == NULL) {
                LOGE("lib alloc error\n");
-               return 0;
+               res = 0;
+               goto exit;
        };
 
        app_info = (*dest)->app;
@@ -209,15 +221,16 @@ int parse_inst_app(struct msg_buf_t *msg, struct app_list_t **dest)
                !check_app_type(app_info->app_type))
        {
                LOGE("app type parsing error <0x%X>\n", app_info->app_type);
-               return 0;
+               goto exit_free_err;
        }
 
        if (!parse_string(msg, &app_info->app_id) ||
                !check_app_id(app_info->app_type, app_info->app_id))
        {
                LOGE("app id parsing error\n");
-               return 0;
+               goto exit_free_err;
        }
+
        if (!parse_string(msg, &app_info->exe_path) ||
            ((app_info->app_type != APP_TYPE_WEB) &&
             ((app_info->app_type != APP_TYPE_RUNNING) ||
@@ -225,18 +238,24 @@ int parse_inst_app(struct msg_buf_t *msg, struct app_list_t **dest)
             !check_exec_path(app_info->exe_path)))
        {
                LOGE("exec path parsing error\n");
-               return 0;
+               goto exit_free_err;
        }
        end = msg->cur_pos;
 
        if (!parse_func_inst_list(msg, (struct data_list_t *)*dest)) {
                LOGE("funcs parsing error\n");
-               return 0;
+               goto exit_free_err;
        }
 
        (*dest)->size += (end - start) + sizeof((*dest)->func_num);
        (*dest)->hash = calc_app_hash(app_info);
-       return 1;
+       goto exit;
+
+exit_free_err:
+       res = 0;
+       free(*dest);
+exit:
+       return res;
 }
 
 int parse_app_inst_list(struct msg_buf_t *msg,

diff --git a/daemon/elf.c b/daemon/elf.c
index 8104448..ca61c3b 100644 (file)
--- a/daemon/elf.c
+++ b/daemon/elf.c
@@ -146,21 +146,24 @@ void get_build_dir(char builddir[PATH_MAX], const char *filename)
        size_t len;
        void *filemem;
        char adj_filename[PATH_MAX];
+
        suffix_filename(adj_filename, filename);
 
        filemem = mmap_file(adj_filename, &len);
        if (filemem) {
                const Elf_Shdr *debug_header = elf_find_debug_header(filemem);
                if (debug_header) {
-                       const char *debug_section =
-                           filemem + debug_header->sh_offset;
-                       const char *debug_section_end =
-                           debug_section + debug_header->sh_size;
-                       const char *p = debug_section;
+                       const char *debug_section, *debug_section_end, *p;
+
+                       debug_section = filemem + debug_header->sh_offset;
+                       debug_section_end = debug_section + debug_header->sh_size;
+                       p = debug_section;
+
                        /* `is_like_absolute_path' checks three chars forward. */
                        while (p < debug_section_end - 3) {
                                if (is_like_absolute_path(p)) {
                                        snprintf(builddir, PATH_MAX, "%s", p);
+                                       munmap(filemem, len);
                                        return;
                                }
                                p = 1 + memchr(p, '\0', debug_section_end - p);

diff --git a/daemon/sys_stat.c b/daemon/sys_stat.c
index 6eb2aac..6a39a71 100644 (file)
--- a/daemon/sys_stat.c
+++ b/daemon/sys_stat.c
@@ -1137,13 +1137,15 @@ static int update_thread_data(int pid)
        if(!(taskdir = opendir(path)))
        {
                LOGE("task not found '%s'\n", path);
-               return -1;
+               ret = -1;
+               goto exit;
        }
 
        node = find_node(inst_prochead, pid);
        if (node == NULL) {
                LOGE("inst node task not found '%s' pid = %d\n", path, pid);
-               return -1;
+               ret = -1;
+               goto exit_close_dir;
        }
        thread_prochead = (procNode **)&(node->thread_prochead);
 
@@ -1188,7 +1190,9 @@ static int update_thread_data(int pid)
        del_notfound_node(thread_prochead);
        reset_found_node(*thread_prochead);
 
+exit_close_dir:
        closedir(taskdir);
+exit:
        return ret;
 }