Fix svace issues in arastorage and testcases for that

utc_arastorage_main.c
- Use of vulnerable function 'sprintf' at utc_arastorage_main.c:165. This function is unsafe, use snprintf instead.
- After having been assigned to NULL value at utc_arastorage_main.c:851, pointer '0' is passed as 1st parameter in call to function 'cursor_move_last' at utc_arastorage_main.c:851, where it is dereferenced at cursor.c:80.
- After having been assigned to NULL value at utc_arastorage_main.c:773, pointer '0' is passed as 1st parameter in call to function 'cursor_move_next' at utc_arastorage_main.c:773, where it is dereferenced at cursor.c:86.
- After having been assigned to NULL value at utc_arastorage_main.c:812, pointer '0' is passed as 1st parameter in call to function 'cursor_move_prev' at utc_arastorage_main.c:812, where it is dereferenced at cursor.c:92.
- Expression '(count)' has type 'tuple_id_t' ('unsigned int'), so it's never less than zero

storage_interface.c
- Copying from string 'filename' to '&g_storage_write_buffer.file_name[0]' without null termination at storage_interface.c:557 by calling function 'memcpy'.
- Copying from parameter string 'name' to '&rel->name[0]' at storage_interface.c:184 may form a non-terminated C string of size 17.

aql_exec.c
- This statement in the source code might be unreachable during program execution.

index_bplustree.c
- Dynamic memory referenced by 'path' was allocated at index_bplustree.c:1376 by calling function 'malloc' and lost at index_bplustree.c:1391.
- The handle 'fd' was created at storage_abstraction.c:51 by calling function 'storage_open' at index_bplustree.c:522 and lost at index_bplustree.c:527.

relation.c
- Dynamic memory referenced by 'cursor' was allocated at relation.c:1064 by calling function 'malloc' and lost at relation.c:1102.

diff --git a/apps/examples/testcase/ta_tc/arastorage/utc/utc_arastorage_main.c b/apps/examples/testcase/ta_tc/arastorage/utc/utc_arastorage_main.c
index c2c130b..450b1a5 100644 (file)
--- a/apps/examples/testcase/ta_tc/arastorage/utc/utc_arastorage_main.c
+++ b/apps/examples/testcase/ta_tc/arastorage/utc/utc_arastorage_main.c
@@ -162,7 +162,7 @@ void utc_arastorage_db_exec_tc_p(void)
        TC_ASSERT("db_exec", DB_SUCCESS(res));
 
        memset(query, 0, QUERY_LENGTH);
-       sprintf(query, "CREATE INDEX %s.%s TYPE %s;", RELATION_NAME2, g_attribute_set[1], INDEX_BPLUS);
+       snprintf(query, QUERY_LENGTH, "CREATE INDEX %s.%s TYPE %s;", RELATION_NAME2, g_attribute_set[1], INDEX_BPLUS);
        res = db_exec(query);
        TC_ASSERT("db_exec", DB_SUCCESS(res));
 
@@ -674,7 +674,7 @@ void utc_arastorage_cursor_get_count_tc_p(void)
        tuple_id_t count;
 
        count = cursor_get_count(g_cursor);
-       TC_ASSERT_GEQ("cursor_get_count", count, 0);
+       TC_ASSERT_GT("cursor_get_count", count, 0);
        printf("cursor count : %d\n", count);
 
        TC_SUCCESS_RESULT();

diff --git a/framework/src/arastorage/aql_exec.c b/framework/src/arastorage/aql_exec.c
index 80aaf8f..4278374 100644 (file)
--- a/framework/src/arastorage/aql_exec.c
+++ b/framework/src/arastorage/aql_exec.c
@@ -313,10 +313,6 @@ errout:
                relation_release(rel);
        }
 
-       if (cursor != NULL) {
-               cursor_deinit(cursor);
-       }
-
        aql_deinit_handle(&handler);
 
        return NULL;

diff --git a/framework/src/arastorage/cursor.c b/framework/src/arastorage/cursor.c
index 6a3b9af..d7f07f1 100644 (file)
--- a/framework/src/arastorage/cursor.c
+++ b/framework/src/arastorage/cursor.c
@@ -77,18 +77,30 @@ db_result_t cursor_move_first(db_cursor_t *cursor)
 /* Search the last set tuple id and update storage id corresponding it. */
 db_result_t cursor_move_last(db_cursor_t *cursor)
 {
+       if (!cursor) {
+               return DB_CURSOR_ERROR;
+       }
+
        return cursor_move_to(cursor, cursor->cursor_rows - 1);
 }
 
 /* Search next set tuple id and update storage id corresponding it. */
 db_result_t cursor_move_next(db_cursor_t *cursor)
 {
+       if (!cursor) {
+               return DB_CURSOR_ERROR;
+       }
+
        return cursor_move_to(cursor, cursor->current_cursor_row + 1);
 }
 
 /* Search previous set tuple id and update storage id corresponding it. */
 db_result_t cursor_move_prev(db_cursor_t *cursor)
 {
+       if (!cursor) {
+               return DB_CURSOR_ERROR;
+       }
+
        return cursor_move_to(cursor, cursor->current_cursor_row - 1);
 }
 

diff --git a/framework/src/arastorage/index_bplustree.c b/framework/src/arastorage/index_bplustree.c
index fdad369..313b4bb 100644 (file)
--- a/framework/src/arastorage/index_bplustree.c
+++ b/framework/src/arastorage/index_bplustree.c
@@ -524,6 +524,7 @@ static db_result_t destroy(index_t *index)
                return DB_STORAGE_ERROR;
        }
        if (DB_ERROR(storage_read_from(fd, bucket_file, sizeof(tree_t), sizeof(bucket_file)))) {
+               storage_close(fd);
                return DB_STORAGE_ERROR;
        }
        storage_close(fd);
@@ -1388,6 +1389,7 @@ static pair_t *tree_find(tree_t *tree, int key)
                 */
                node = tree_read(tree, id);
                if (node == NULL) {
+                       free(path);
                        return NULL;
                }
                index = id;
@@ -1411,6 +1413,7 @@ static pair_t *tree_find(tree_t *tree, int key)
                        if (tree->lock_buckets[node->id[index]]) {
                                pthread_mutex_unlock(&(tree->bucket_lock));
                                modify_cache(tree, id, NODE, UNLOCK);
+                               free(path);
                                return NULL;
                        } else {
                                tree->lock_buckets[node->id[index]] = 1;
@@ -1435,6 +1438,7 @@ static pair_t *tree_find(tree_t *tree, int key)
                        modify_cache(tree, index, NODE, UNLOCK);
                }
        }
+       free(path);
        return NULL;
 }
 

diff --git a/framework/src/arastorage/relation.c b/framework/src/arastorage/relation.c
index 6591c25..fcaf805 100644 (file)
--- a/framework/src/arastorage/relation.c
+++ b/framework/src/arastorage/relation.c
@@ -463,6 +463,7 @@ db_result_t relation_remove(relation_t *rel, int remove_tuples)
        }
        snprintf(filename, len, "%s%s\0", rel->name, INDEX_NAME_SUFFIX);
        result = storage_remove(filename);
+       free(filename);
        if (DB_ERROR(result)) {
                DB_LOG_E("DB: Index file unlinking failed\n");
        }
@@ -1099,6 +1100,7 @@ db_cursor_t *relation_process_result(db_handle_t *handler)
                        break;
                }
        }
+       cursor_deinit(cursor);
        return NULL;
 }
 

diff --git a/framework/src/arastorage/storage_interface.c b/framework/src/arastorage/storage_interface.c
index c4f29f2..48fdb6b 100644 (file)
--- a/framework/src/arastorage/storage_interface.c
+++ b/framework/src/arastorage/storage_interface.c
@@ -181,7 +181,7 @@ db_result_t storage_get_relation(relation_t *rel, char *name)
                return DB_STORAGE_ERROR;
        }
 
-       strncpy(rel->name, name, sizeof(rel->name));
+       strncpy(rel->name, name, sizeof(rel->name) - 1);
 
        r = storage_read(fd, rel->tuple_filename, sizeof(rel->tuple_filename));
        if (r != sizeof(rel->tuple_filename)) {
@@ -554,7 +554,7 @@ db_result_t storage_write_row(db_storage_id_t fd, storage_row_t row, unsigned le
        }
        memcpy(g_storage_write_buffer.buffer + g_storage_write_buffer.data_size, row, length);
        g_storage_write_buffer.data_size += length;
-       memcpy(g_storage_write_buffer.file_name, filename, strlen(filename));
+       memcpy(g_storage_write_buffer.file_name, filename, strlen(filename) + 1);
 #else
        if (storage_write(fd, row, length) < 0) {
                DB_LOG_D("DB: Failed to store %u bytes\n", length);