Fix defect

- Unreachable code
- Unchecked function result
- Negative return value handle
- Memory leak

Change-Id: I21ecb692a244e9c41861471836880ed6d4537297
Signed-off-by: Hyunho Kang <hhstark.kang@samsung.com>

diff --git a/src/data-control-internal.c b/src/data-control-internal.c
index ca035f95629e90960e35b76d2febd328ff7c14d1..e682a458df0460c1ae0c696d0a072e28ad491bfb 100755 (executable)
--- a/src/data-control-internal.c
+++ b/src/data-control-internal.c
@@ -117,13 +117,12 @@ char *_datacontrol_create_select_statement(char *data_id, const char **column_li
 
        while (i < column_count - 1) {
                LOGI("column i = %d, %s", i, column_list[i]);
-               strcat(column, column_list[i]);
-               strcat(column, ", ");
+               strncat(column, column_list[i], MAX_COLUMN_SIZE - (strlen(column) + 1));
+               strncat(column, ", ", MAX_COLUMN_SIZE - (strlen(column) + 1));
                i++;
        }
 
-       LOGI("column i = %d, %s", i, column_list[i]);
-       strcat(column, column_list[i]);
+       strncat(column, column_list[i], MAX_COLUMN_SIZE - (strlen(column) + 1));
 
        char *statement = calloc(MAX_STATEMENT_SIZE, sizeof(char));
        snprintf(statement, MAX_STATEMENT_SIZE, "SELECT %s * FROM %s WHERE %s ORDER BY %s", column,

diff --git a/src/data-control-provider.c b/src/data-control-provider.c
index e51edf75b36312157d8de67147e3fafbea839061..2f34eaaf73cbd83e07648dc1c17a1b1cc99efeb7 100755 (executable)
--- a/src/data-control-provider.c
+++ b/src/data-control-provider.c
@@ -434,9 +434,19 @@ static bundle *__get_data_sql(int fd)
                char *buf = (char *)calloc(len, sizeof(char));
                if (buf == NULL) {
                        LOGE("calloc fail");
+                       if (b)
+                               bundle_free(b);
                        return NULL;
                }
                ret = read(fd, buf, len);
+               if (ret < len) {
+                       LOGE("read error :%d", ret);
+                       if (b)
+                               bundle_free(b);
+                       if (buf)
+                               free(buf);
+                       return NULL;
+               }
                b = bundle_decode_raw((bundle_raw *)buf, len);
 
                if (buf)
@@ -573,11 +583,16 @@ static int __send_result(bundle *b, datacontrol_request_type type, void *data)
 
        datacontrol_socket_info *socket_info;
        char *caller_app_id = (char *)bundle_get_val(b, AUL_K_CALLER_APPID);
+       int ret = DATACONTROL_ERROR_NONE;
 
        LOGI("__datacontrol_send_async caller_app_id : %s ", caller_app_id);
 
        socket_info = g_hash_table_lookup(__socket_pair_hash, caller_app_id);
-       int ret = __datacontrol_send_async(socket_info->socket_fd, b, type, data);
+       if (socket_info == NULL) {
+               LOGE("__socket_pair_hash lookup fail");
+               return DATACONTROL_ERROR_IO_ERROR;
+       }
+       ret = __datacontrol_send_async(socket_info->socket_fd, b, type, data);
 
        LOGI("__datacontrol_send_async result : %d ", ret);
 
@@ -591,6 +606,14 @@ static int __send_result(bundle *b, datacontrol_request_type type, void *data)
 
 int __provider_process(bundle *b, int fd)
 {
+       int len = 0;
+       const char **arg_list = NULL;
+       const char **column_list = NULL;
+       datacontrol_h provider = NULL;
+       int provider_req_id = 0;
+       int *key = NULL;
+       bundle *value = NULL;
+
        const char *request_type = bundle_get_val(b, OSP_K_DATACONTROL_REQUEST_TYPE);
        if (request_type == NULL) {
                LOGE("Invalid data control request");
@@ -616,10 +639,9 @@ int __provider_process(bundle *b, int fd)
                return DATACONTROL_ERROR_INVALID_PARAMETER;
        }
 
-       int len = 0;
-       const char **arg_list = bundle_get_str_array(b, OSP_K_ARG, &len);
+       arg_list = bundle_get_str_array(b, OSP_K_ARG, &len);
 
-       datacontrol_h provider = malloc(sizeof(struct datacontrol_s));
+       provider = malloc(sizeof(struct datacontrol_s));
        if (provider == NULL) {
                LOGE("Out of memory. fail to alloc provider.");
                return DATACONTROL_ERROR_IO_ERROR;
@@ -632,22 +654,22 @@ int __provider_process(bundle *b, int fd)
        provider->data_id = (char *)arg_list[PACKET_INDEX_DATAID];
 
        /* Set the request ID */
-       int provider_req_id = __provider_new_request_id();
+       provider_req_id = __provider_new_request_id();
 
        LOGI("Provider ID: %s, data ID: %s, request type: %s", provider->provider_id, provider->data_id, request_type);
 
        /* Add the data to the table */
-       int *key = malloc(sizeof(int));
+       key = malloc(sizeof(int));
        if (key == NULL) {
                LOGE("Out of memory. fail to malloc key");
-               return DATACONTROL_ERROR_IO_ERROR;
+               goto err;
        }
        *key = provider_req_id;
 
-       bundle *value = bundle_dup(b);
+       value = bundle_dup(b);
        if (value == NULL) {
                LOGE("Fail to dup value");
-               return DATACONTROL_ERROR_IO_ERROR;
+               goto err;
        }
        g_hash_table_insert(__request_table, key, value);
 
@@ -659,10 +681,10 @@ int __provider_process(bundle *b, int fd)
                int column_count = atoi(arg_list[i++]); /* Column count */
 
                LOGI("SELECT column count: %d", column_count);
-               const char **column_list = (const char **)malloc(column_count * (sizeof(char *)));
+               column_list = (const char **)malloc(column_count * (sizeof(char *)));
                if (column_list == NULL) {
                        LOGE("Out of memory. Fail to malloc column_list.");
-                       return DATACONTROL_ERROR_IO_ERROR;
+                       goto err;
                }
 
                while (current < column_count) {
@@ -695,11 +717,13 @@ int __provider_process(bundle *b, int fd)
        {
                LOGI("INSERT / UPDATE handler");
                bundle *sql = __get_data_sql(fd);
-               if (sql == NULL)
-                       return DATACONTROL_ERROR_IO_ERROR;
-               if (type == DATACONTROL_TYPE_SQL_INSERT)
+               if (sql == NULL) {
+                       LOGE("__get_data_sql fail");
+                       goto err;
+               }
+               if (type == DATACONTROL_TYPE_SQL_INSERT) {
                        provider_sql_cb->insert(provider_req_id, provider, sql, provider_sql_user_data);
-               else {
+               } else {
                        const char *where = arg_list[PACKET_INDEX_UPDATEWHERE];
                        LOGI("UPDATE from where: %s", where);
                        if (strncmp(where, DATACONTROL_EMPTY, strlen(DATACONTROL_EMPTY)) == 0)
@@ -761,12 +785,26 @@ int __provider_process(bundle *b, int fd)
        free(provider);
 
        return DATACONTROL_ERROR_NONE;
+err:
+
+       if (provider)
+               free(provider);
+       if (key)
+               free(key);
+       if (value)
+               free(value);
+
+       return DATACONTROL_ERROR_IO_ERROR;
 }
 
 gboolean __provider_recv_message(GIOChannel *channel,
                GIOCondition cond,
                gpointer data) {
 
+       char *buf = NULL;
+       int data_len;
+       guint nb;
+
        gint fd = g_io_channel_unix_get_fd(channel);
        gboolean retval = TRUE;
 
@@ -780,9 +818,6 @@ gboolean __provider_recv_message(GIOChannel *channel,
                goto error;
 
        if (cond & G_IO_IN) {
-               char *buf;
-               int data_len;
-               guint nb;
 
                if (_read_socket(fd, (char *)&data_len, sizeof(data_len), &nb) != DATACONTROL_ERROR_NONE) {
                        LOGE("read socket fail : data_len");
@@ -800,7 +835,7 @@ gboolean __provider_recv_message(GIOChannel *channel,
                if (data_len > 0) {
                        bundle *kb = NULL;
 
-                       buf = (char *) calloc(data_len + 1, sizeof(char));
+                       buf = (char *)calloc(data_len + 1, sizeof(char));
                        if (buf == NULL) {
                                LOGE("calloc failed");
                                goto error;
@@ -813,12 +848,10 @@ gboolean __provider_recv_message(GIOChannel *channel,
 
                        if (nb == 0) {
                                LOGI("__provider_recv_message: nb 0 : EOF\n");
-                               free(buf);
                                goto error;
                        }
 
                        kb = bundle_decode_raw((bundle_raw *)buf, data_len);
-                       free(buf);
                        if (__provider_process(kb, fd) != DATACONTROL_ERROR_NONE) {
                                bundle_free(kb);
                                goto error;
@@ -826,10 +859,15 @@ gboolean __provider_recv_message(GIOChannel *channel,
                        bundle_free(kb);
                }
        }
+       if (buf)
+               free(buf);
+
        return retval;
 error:
        if (((char *)data) != NULL)
                g_hash_table_remove(__socket_pair_hash, (char *)data);
+       if (buf)
+               free(buf);
 
        return FALSE;
 }
@@ -962,7 +1000,10 @@ int datacontrol_provider_send_select_result(int request_id, void *db_handle)
        }
 
        bundle *res = __set_result(b, DATACONTROL_TYPE_SQL_SELECT, db_handle);
-       return __send_result(res, DATACONTROL_TYPE_SQL_SELECT, db_handle);
+       int ret =  __send_result(res, DATACONTROL_TYPE_SQL_SELECT, db_handle);
+       g_hash_table_remove(__request_table, &request_id);
+
+       return ret;
 }
 
 int datacontrol_provider_send_insert_result(int request_id, long long row_id)
@@ -980,7 +1021,11 @@ int datacontrol_provider_send_insert_result(int request_id, long long row_id)
 
        bundle *res = __set_result(b, DATACONTROL_TYPE_SQL_INSERT, (void *)&row_id);
 
-       return __send_result(res, DATACONTROL_TYPE_SQL_INSERT, NULL);
+       int ret = __send_result(res, DATACONTROL_TYPE_SQL_INSERT, NULL);
+       g_hash_table_remove(__request_table, &request_id);
+
+       return ret;
+
 }
 
 int datacontrol_provider_send_update_result(int request_id)
@@ -998,7 +1043,11 @@ int datacontrol_provider_send_update_result(int request_id)
 
        bundle *res = __set_result(b, DATACONTROL_TYPE_SQL_UPDATE, NULL);
 
-       return __send_result(res, DATACONTROL_TYPE_SQL_UPDATE, NULL);
+       int ret = __send_result(res, DATACONTROL_TYPE_SQL_UPDATE, NULL);
+       g_hash_table_remove(__request_table, &request_id);
+
+       return ret;
+
 }
 
 int datacontrol_provider_send_delete_result(int request_id)
@@ -1016,7 +1065,10 @@ int datacontrol_provider_send_delete_result(int request_id)
 
        bundle *res = __set_result(b, DATACONTROL_TYPE_SQL_DELETE, NULL);
 
-       return __send_result(res, DATACONTROL_TYPE_SQL_DELETE, NULL);
+       int ret = __send_result(res, DATACONTROL_TYPE_SQL_DELETE, NULL);
+       g_hash_table_remove(__request_table, &request_id);
+
+       return ret;
 }
 
 int datacontrol_provider_send_error(int request_id, const char *error)
@@ -1052,7 +1104,11 @@ int datacontrol_provider_send_map_result(int request_id)
 
        bundle *res = __set_result(b, DATACONTROL_TYPE_UNDEFINED, NULL);
 
-       return __send_result(res, DATACONTROL_TYPE_UNDEFINED, NULL);
+       int ret = __send_result(res, DATACONTROL_TYPE_UNDEFINED, NULL);
+       g_hash_table_remove(__request_table, &request_id);
+
+       return ret;
+
 }
 
 int datacontrol_provider_send_map_get_value_result(int request_id, char **value_list, int value_count)
@@ -1074,5 +1130,9 @@ int datacontrol_provider_send_map_get_value_result(int request_id, char **value_
 
        bundle *res = __set_result(b, DATACONTROL_TYPE_MAP_GET, value_list);
 
-       return __send_result(res, DATACONTROL_TYPE_MAP_GET, value_list);
+       int ret = __send_result(res, DATACONTROL_TYPE_MAP_GET, value_list);
+       g_hash_table_remove(__request_table, &request_id);
+
+       return ret;
+
 }

diff --git a/src/data-control-sql-cursor.c b/src/data-control-sql-cursor.c
index 43507829f94f32ae2bc96b3a9afdf6c7220605cd..b1d50aea28cdef195acda2c271ccdb327891fb90 100755 (executable)
--- a/src/data-control-sql-cursor.c
+++ b/src/data-control-sql-cursor.c
@@ -116,7 +116,14 @@ int datacontrol_sql_get_column_name(resultset_cursor *cursor, int column_index,
        char col_name[4096] = {0, };
        int i = 0;
        int ret = 0;
-       FILE *fp = fdopen(dup(cursor->resultset_fd), "r");
+       FILE *fp = NULL;
+       int resultset_fd = dup(cursor->resultset_fd);
+       if (resultset_fd < 0) {
+               LOGE("dup failed errno %d : %s \n", errno, strerror(errno));
+               return DATACONTROL_ERROR_IO_ERROR;
+       }
+
+       fp = fdopen(resultset_fd, "r");
        if (fp == NULL) {
                LOGE("unable to open resultset file: %s", strerror(errno));
                return DATACONTROL_ERROR_IO_ERROR;
@@ -321,6 +328,11 @@ int datacontrol_sql_get_blob_data(resultset_cursor *cursor, int column_index, vo
        }
 
        ret = read(fd, &size, sizeof(int));
+       if (ret == 0) {
+               LOGE("unable to read size in the resultset file: %s", strerror(errno));
+               return DATACONTROL_ERROR_IO_ERROR;
+       }
+
        if (size > data_size) {
                LOGE("size is more than the size requested");
                return DATACONTROL_ERROR_MAX_EXCEEDED; /* overflow */

diff --git a/src/data-control-sql.c b/src/data-control-sql.c
index ad70bbba44d77dfc9ab47a515d571a7f7127793a..279577fd5bb9ba9818213858ebe9f917fcba0a17 100755 (executable)
--- a/src/data-control-sql.c
+++ b/src/data-control-sql.c
@@ -309,7 +309,8 @@ static int __recv_sql_select_process(bundle *kb, int fd, resultset_cursor *curso
        cursor->resultset_path = strdup(select_map_file);
        if (cursor->resultset_path == NULL) {
                LOGE("Out of memory. can not dup select map file.");
-               return DATACONTROL_ERROR_IO_ERROR;
+               retval = DATACONTROL_ERROR_IO_ERROR;
+               goto out;
        }
        cursor->resultset_fd = result_fd;
        if (_read_socket(fd, (char *)&column_count, sizeof(column_count), &nb) != DATACONTROL_ERROR_NONE) {
@@ -323,6 +324,7 @@ static int __recv_sql_select_process(bundle *kb, int fd, resultset_cursor *curso
        /* no data check. */
        if (column_count == DATACONTROL_RESULT_NO_DATA) {
                LOGE("No result");
+               close(result_fd);
                return DATACONTROL_ERROR_NONE;
        }
 
@@ -490,9 +492,12 @@ static int __recv_sql_select_process(bundle *kb, int fd, resultset_cursor *curso
                if (i + 1 < row_count)
                        cursor->row_offset_list[i + 1] = cursor->row_offset_list[i] + row_offset;
        }
+       close(result_fd);
        return retval;
 
 out:
+       if (result_fd != -1)
+               close(result_fd);
        if (column_name)
                free(column_name);
        if (value)
@@ -1195,8 +1200,7 @@ int datacontrol_sql_select_with_page(datacontrol_h provider, char **column_list,
                while (select_col < column_count)
                        arg_list[i++] = column_list[select_col++];
 
-       } else
-               arg_list[i++] = DATACONTROL_EMPTY;
+       }
 
        if (where)      /* arg: where clause */
                arg_list[i++] = where;

diff --git a/src/data_control_provider.c b/src/data_control_provider.c
index b222c5e7f7337030bcacd07fa5d786da9b7d8195..f5acca4cd78fdb2ad82a2bc4d17afc33be007673 100755 (executable)
--- a/src/data_control_provider.c
+++ b/src/data_control_provider.c
@@ -251,7 +251,7 @@ EXPORT_API char *data_control_provider_create_insert_statement(data_control_h pr
 
        _LOGI("SQL statement length: %d", sql_len);
 
-       char* sql = (char *) calloc(sql_len, sizeof(char));
+       char* sql = (char *)calloc(sql_len, sizeof(char));
        if (sql == NULL) {
                free(data_id);
                free(cols->keys);
@@ -262,24 +262,24 @@ EXPORT_API char *data_control_provider_create_insert_statement(data_control_h pr
        }
        memset(sql, 0, sql_len);
 
-       sprintf(sql, "INSERT INTO %s (", data_id);
+       snprintf(sql, sql_len, "INSERT INTO %s (", data_id);
        free(data_id);
 
        for (index = 0; index < row_count - 1; index++) {
-               strcat(sql, cols->keys[index]);
-               strcat(sql, ", ");
+               strncat(sql, cols->keys[index], sql_len - (strlen(sql) + 1));
+               strncat(sql, ", ", sql_len - (strlen(sql) + 1));
        }
 
-       strcat(sql, cols->keys[index]);
-       strcat(sql, ") VALUES (");
+       strncat(sql, cols->keys[index], sql_len - (strlen(sql) + 1));
+       strncat(sql, ") VALUES (", sql_len - (strlen(sql) + 1));
 
        for (index = 0; index < row_count - 1; index++) {
-               strcat(sql, cols->vals[index]);
-               strcat(sql, ", ");
+               strncat(sql, cols->vals[index], sql_len - (strlen(sql) + 1));
+               strncat(sql, ", ", sql_len - (strlen(sql) + 1));
        }
 
-       strcat(sql, cols->vals[index]);
-       strcat(sql, ")");
+       strncat(sql, cols->vals[index], sql_len - (strlen(sql) + 1));
+       strncat(sql, ")", sql_len - (strlen(sql) + 1));
 
        _LOGI("SQL statement is: %s", sql);
 
@@ -310,7 +310,7 @@ EXPORT_API char *data_control_provider_create_delete_statement(data_control_h pr
 
        _LOGI("SQL statement length: %d", sql_len);
 
-       char *sql = (char *) calloc(sql_len, sizeof(char));
+       char *sql = (char *)calloc(sql_len, sizeof(char));
        if (sql == NULL) {
                free(data_id);
                set_last_result(DATA_CONTROL_ERROR_OUT_OF_MEMORY);
@@ -318,10 +318,10 @@ EXPORT_API char *data_control_provider_create_delete_statement(data_control_h pr
        }
        memset(sql, 0, sql_len);
 
-       sprintf(sql, "DELETE FROM %s", data_id);
+       snprintf(sql, sql_len, "DELETE FROM %s", data_id);
        if (where) {
-               strcat(sql, " WHERE ");
-               strcat(sql, where);
+               strncat(sql, " WHERE ", sql_len - (strlen(sql) + 1));
+               strncat(sql, where, sql_len - (strlen(sql) + 1));
        }
 
        _LOGI("SQL statement is: %s", sql);
@@ -382,23 +382,23 @@ EXPORT_API char *data_control_provider_create_update_statement(data_control_h pr
        }
        memset(sql, 0, sql_len);
 
-       sprintf(sql, "UPDATE %s SET ", data_id);
+       snprintf(sql, sql_len, "UPDATE %s SET ", data_id);
        free(data_id);
 
        for (index = 0; index < row_count - 1; index++) {
-               strcat(sql, cols->keys[index]);
-               strcat(sql, " = ");
-               strcat(sql, cols->vals[index]);
-               strcat(sql, ", ");
+               strncat(sql, cols->keys[index], sql_len - (strlen(sql) + 1));
+               strncat(sql, " = ", sql_len - (strlen(sql) + 1));
+               strncat(sql, cols->vals[index], sql_len - (strlen(sql) + 1));
+               strncat(sql, ", ", sql_len - (strlen(sql) + 1));
        }
 
-       strcat(sql, cols->keys[index]);
-       strcat(sql, " = ");
-       strcat(sql, cols->vals[index]);
+       strncat(sql, cols->keys[index], sql_len - (strlen(sql) + 1));
+       strncat(sql, " = ", sql_len - (strlen(sql) + 1));
+       strncat(sql, cols->vals[index], sql_len - (strlen(sql) + 1));
 
        if (where) {
-               strcat(sql, " WHERE ");
-               strcat(sql, where);
+               strncat(sql, " WHERE ", sql_len - (strlen(sql) + 1));
+               strncat(sql, where, sql_len - (strlen(sql) + 1));
        }
 
        _LOGI("SQL statement is: %s", sql);
@@ -451,27 +451,27 @@ EXPORT_API char *data_control_provider_create_select_statement(data_control_h pr
        }
        memset(sql, 0, sql_len);
 
-       strcpy(sql, "SELECT ");
+       strncpy(sql, "SELECT ", sql_len);
        if (!column_list) {
-               strcat(sql, "*");
+               strncat(sql, "*", sql_len - (strlen(sql) + 1));
        } else {
                for (index = 0; index < column_count - 1; index++) {
-                       strcat(sql, column_list[index]);
-                       strcat(sql, ", ");
+                       strncat(sql, column_list[index], sql_len - (strlen(sql) + 1));
+                       strncat(sql, ", ", sql_len - (strlen(sql) + 1));
                }
-               strcat(sql, column_list[index]);
+               strncat(sql, column_list[index], sql_len - (strlen(sql) + 1));
        }
 
-       strcat(sql, " FROM ");
-       strcat(sql, data_id);
+       strncat(sql, " FROM ", sql_len - (strlen(sql) + 1));
+       strncat(sql, data_id, sql_len - (strlen(sql) + 1));
 
        if (where) {
-               strcat(sql, " WHERE ");
-               strcat(sql, where);
+               strncat(sql, " WHERE ", sql_len - (strlen(sql) + 1));
+               strncat(sql, where, sql_len - (strlen(sql) + 1));
        }
        if (order) {
-               strcat(sql, " ORDER BY ");
-               strcat(sql, order);
+               strncat(sql, " ORDER BY ", sql_len - (strlen(sql) + 1));
+               strncat(sql, order, sql_len - (strlen(sql) + 1));
        }
 
        _LOGI("SQL statement is: %s", sql);