From 3d536b75e732ef7af7aa969c5ebe4859cea9b343 Mon Sep 17 00:00:00 2001 From: Michal Bloch Date: Mon, 22 Oct 2018 16:22:52 +0200 Subject: [PATCH] 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 --- src/test_runner.c | 28 ++++++++++++++++------------ src/test_runner.h | 2 +- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/test_runner.c b/src/test_runner.c index 5c9cc42..9e9dbe0 100755 --- 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 --- 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); -- 2.7.4