Secure get_test_id against buffer overflows

So far it's always called safely so this is mostly
for future-proofing and silencing static analysis.

Change-Id: I837aba9024954bdaee6e0adab695d514c753cc19
Signed-off-by: Michal Bloch <m.bloch@samsung.com>

diff --git a/src/test_runner.c b/src/test_runner.c
index 5c9cc42..9e9dbe0 100755 (executable)
--- a/src/test_runner.c
+++ b/src/test_runner.c
@@ -338,7 +338,7 @@ void parse_test_state(const struct binary* b, const char* test_name, char* buffe
                {
                        int k = 0;
                        buffer[state_option] = 0;
-                       get_test_id(test_id, b, test_name);
+                       get_test_id(test_id, sizeof test_id, b, test_name);
                        fprintf(stderr, "[stdout][%s]%s\n", test_id, buffer);
                        while ((k = sm_update(buffer, k))) {
                                const char* t_name = sm_get_result(0);
@@ -346,7 +346,7 @@ void parse_test_state(const struct binary* b, const char* test_name, char* buffe
                                bool is_successful = result_code[0] == '1' && result_code[1] == 0;
                                const char* result_text = is_successful ? "PASS" : "FAIL";
 
-                               get_test_id(test_id, b, t_name);
+                               get_test_id(test_id, sizeof test_id, b, t_name);
                                add_test_result(test_id, result_text, "", is_successful);
                                sm_reset();
                        }
@@ -354,18 +354,18 @@ void parse_test_state(const struct binary* b, const char* test_name, char* buffe
                break;
        case NEW_STDERR:
                buffer[state_option] = 0;
-               get_test_id(test_id, b, test_name);
+               get_test_id(test_id, sizeof test_id, b, test_name);
                fprintf(stderr, "[stderr][%s]%s\n", test_id, buffer);
                break;
        case RESULT_CODE:
                if (state_option != 0)
-                       add_test_result(get_test_id(test_id, b, test_name), "FAIL", "Test exited with error code", 0);
+                       add_test_result(get_test_id(test_id, sizeof test_id, b, test_name), "FAIL", "Test exited with error code", 0);
                break;
        case RESULT_SIGNAL:
-               add_test_result(get_test_id(test_id, b, test_name), "FAIL", "Finished by SIGNAL", 0);
+               add_test_result(get_test_id(test_id, sizeof test_id, b, test_name), "FAIL", "Finished by SIGNAL", 0);
                break;
        case RESULT_TIMEOUT:
-               add_test_result(get_test_id(test_id, b, test_name), "FAIL", "Test TIMEOUT", 0);
+               add_test_result(get_test_id(test_id, sizeof test_id, b, test_name), "FAIL", "Test TIMEOUT", 0);
                break;
        }
 }
@@ -388,11 +388,15 @@ static int test_results_i;
 static char buffer[MAX_BUFFER];
 static const char* requested_tc[MAX_TC_NUM];
 
-char* get_test_id(char* dest, const struct binary* b, const char* test_name)
+char* get_test_id(char *dest, size_t dest_len, const struct binary* b, const char* test_name)
 {
-       int len = strlen(b->name);
-       memcpy(dest, b->name, len);
-       memcpy(dest + len, test_name, strlen(test_name)+1);
+       const size_t name_len = strlen(b->name);
+       const size_t test_len = strlen(test_name);
+
+       assert(name_len + test_len < dest_len);
+
+       memcpy(dest, b->name, name_len);
+       memcpy(dest + name_len, test_name, test_len + 1);
        return dest;
 }
 
@@ -592,7 +596,7 @@ static void run_test(const struct binary* b, const char* test_name)
 
        if (b->init)
                if (!b->init()) {
-                       add_test_result(get_test_id(test_id, b, test_name), "ERROR", "Internal error: Cannot init test", 0);
+                       add_test_result(get_test_id(test_id, sizeof test_id, b, test_name), "ERROR", "Internal error: Cannot init test", 0);
                        return;
                }
 
@@ -600,7 +604,7 @@ static void run_test(const struct binary* b, const char* test_name)
        if (res > 0)
                parse_output_with_timeout(b, res, test_name);
        else
-               add_test_result(get_test_id(test_id, b, test_name), "ERROR", "Internal error: Cannot start test", 0);
+               add_test_result(get_test_id(test_id, sizeof test_id, b, test_name), "ERROR", "Internal error: Cannot start test", 0);
 
        if (b->clean)
                b->clean();

diff --git a/src/test_runner.h b/src/test_runner.h
index d297238..14f8ec0 100644 (file)
--- a/src/test_runner.h
+++ b/src/test_runner.h
@@ -62,7 +62,7 @@ struct binary {
        int (*clean)(void);
 };
 
-char* get_test_id(char* dest, const struct binary* b, const char* test_name);
+char* get_test_id(char *dest, size_t dest_len, const struct binary* b, const char* test_name);
 void add_test_result(const char* test_id, const char* result, const char* comment, int res);