From 1ea2bfca7a0a18c35fffdeeb7b1d0d6d45f98cbf Mon Sep 17 00:00:00 2001 From: Ravinder Date: Wed, 23 Feb 2022 16:41:00 +0530 Subject: [PATCH] Fix svace issues This patch handles the static analysis issues related to memory leak, deref after NULL, tainted int loop, vulnerable function uses etc. Change-Id: I9c1b6e6e6504433342544c406333e66abc98b7ac Signed-off-by: Abhay Agarwal --- src/thread-br.c | 26 +++++++++++++------------- src/thread-commissioner.c | 7 ++++--- src/thread-joiner.c | 2 -- src/thread-network.c | 8 +++++++- src/thread-socket-handler.c | 6 ++++-- tests/thread-core.c | 6 +++--- tests/thread-main.c | 2 +- tests/thread-network.c | 13 ++++++++++++- tests/thread-srp.c | 3 ++- 9 files changed, 46 insertions(+), 27 deletions(-) diff --git a/src/thread-br.c b/src/thread-br.c index 68010af..fa837c6 100644 --- a/src/thread-br.c +++ b/src/thread-br.c @@ -305,14 +305,14 @@ int thread_br_add_external_route(thread_instance_h instance, ipv6_prefix_len); if (is_stable) - strcat(msg, " s"); + strncat(msg, " s", 2); if (preference == 0) - strcat(msg, " low"); + strncat(msg, " low", 4); else if (preference == 1) - strcat(msg, " med"); + strncat(msg, " med", 4); else if (preference == 2) - strcat(msg, " high"); + strncat(msg, " high", 5); THREAD_DBG("DEBUG: BORDER ROUTER MESSAGE -> [%s]", msg); int ret = _thread_socket_client_execute(_thread_get_socket_fd(), @@ -418,24 +418,24 @@ int thread_br_add_onmesh_prefix(thread_instance_h instance, ipv6_prefix_len); if (preference == 0) - strcat(msg, " low "); + strncat(msg, " low ", 5); else if (preference == 1) - strcat(msg, " med "); + strncat(msg, " med ", 5); else if (preference == 2) - strcat(msg, " high "); + strncat(msg, " high ", 6); if (preferred) - strcat(msg, "p"); + strncat(msg, "p", 1); if (slaac) - strcat(msg, "a"); + strncat(msg, "a", 1); if (configure) - strcat(msg, "c"); + strncat(msg, "c", 1); if (default_route) - strcat(msg, "r"); + strncat(msg, "r", 1); if (on_mesh) - strcat(msg, "o"); + strncat(msg, "o", 1); if (stable) - strcat(msg, "s"); + strncat(msg, "s", 1); THREAD_DBG("DEBUG: BORDER ROUTER MESSAGE -> [%s]", msg); int ret = _thread_socket_client_execute(_thread_get_socket_fd(), diff --git a/src/thread-commissioner.c b/src/thread-commissioner.c index 4ee4788..922d9e0 100644 --- a/src/thread-commissioner.c +++ b/src/thread-commissioner.c @@ -68,6 +68,10 @@ int thread_commissioner_set_commisioning_data(thread_instance_h instance, THREAD_VALIDATE_INPUT_PARAMETER(joiner_passphrase); int joiner_passphrase_len = strlen(joiner_passphrase); + + if (!joiner_uuid || !strcmp(joiner_uuid, "")) + joiner_uuid = "*"; + int joiner_uuid_len = strlen(joiner_uuid); retv_if(joiner_passphrase_len < 6 || joiner_passphrase_len > 32, THREAD_ERROR_INVALID_PARAMETER); @@ -75,9 +79,6 @@ int thread_commissioner_set_commisioning_data(thread_instance_h instance, int ret = THREAD_ERROR_NONE; - if (!joiner_uuid || !strcmp(joiner_uuid, "")) - joiner_uuid = "*"; - char msg[THREAD_COMMISSIONER_JOINER_BUFFER_MAX]; snprintf(msg, THREAD_COMMISSIONER_JOINER_BUFFER_MAX, "commissioner joiner add %s %s", joiner_uuid, joiner_passphrase); diff --git a/src/thread-joiner.c b/src/thread-joiner.c index 5ff4bc0..24519bc 100644 --- a/src/thread-joiner.c +++ b/src/thread-joiner.c @@ -50,7 +50,6 @@ int thread_joiner_join_by_network_key(thread_instance_h instance, char buf[THREAD_MAX_BUFFER_SIZE]; snprintf(buf, sizeof(buf), "%s %s", set_network_key, network_key); ret = _thread_socket_client_execute(_thread_get_socket_fd(), buf, strlen(buf)); - retv_if(ret != THREAD_ERROR_NONE, ret); if (ret != THREAD_ERROR_NONE) { THREAD_DBG("socket dataset networkkey execute failed"); return ret; @@ -61,7 +60,6 @@ int thread_joiner_join_by_network_key(thread_instance_h instance, buf[0] = '\0'; snprintf(buf, sizeof(buf), "%s %s", set_panid, panid); ret = _thread_socket_client_execute(_thread_get_socket_fd(), buf, strlen(buf)); - retv_if(ret != THREAD_ERROR_NONE, ret); if (ret != THREAD_ERROR_NONE) { THREAD_DBG("socket dataset panid execute failed"); return ret; diff --git a/src/thread-network.c b/src/thread-network.c index 5cec98c..2e25d2c 100644 --- a/src/thread-network.c +++ b/src/thread-network.c @@ -145,7 +145,7 @@ int thread_network_set_active_dataset_tlvs(thread_instance_h instance, /* Print input data */ char buf[THREAD_MAX_BUFFER_SIZE]; for (int i = 0; i < buf_length; i++) - sprintf(buf + i*2, "%2.2x", tlvs_buffer[i]); + snprintf(buf + i*2, 3, "%2.2x", tlvs_buffer[i]); THREAD_DBG("Active dataset tlvs size: %d :: %s", buf_length, buf); bytes = g_bytes_new(tlvs_buffer, buf_length); @@ -216,6 +216,12 @@ int thread_network_get_active_dataset_tlvs(thread_instance_h instance, } *tlvs_buffer = g_malloc0(THREAD_MAX_BUFFER_SIZE*sizeof(uint8_t)); + if (!(*tlvs_buffer)) { + /* LCOV_EXCL_START */ + THREAD_ERR("g_malloc0 failed"); + return THREAD_ERROR_OUT_OF_MEMORY; + /* LCOV_EXCL_STOP */ + } index = 0; while (index < THREAD_MAX_BUFFER_SIZE) { if (buffer[index] == 'D') diff --git a/src/thread-socket-handler.c b/src/thread-socket-handler.c index f198622..553d9db 100644 --- a/src/thread-socket-handler.c +++ b/src/thread-socket-handler.c @@ -60,8 +60,10 @@ int _thread_socket_client_init(int *socket_fd, const char *if_name) ret = snprintf(sockname.sun_path, sizeof(sockname.sun_path), OPENTHREAD_POSIX_DAEMON_SOCKET_NAME, if_name); - retv_if(!(ret >= 0 && (size_t)ret < sizeof(sockname.sun_path)), - THREAD_ERROR_OPERATION_FAILED); + if (!(ret >= 0 && (size_t)ret < sizeof(sockname.sun_path))) { + close(session_fd); + return THREAD_ERROR_OPERATION_FAILED; + } THREAD_DBG("Connect on socket fd %d path %s", session_fd, sockname.sun_path); diff --git a/tests/thread-core.c b/tests/thread-core.c index 79af951..57c9749 100755 --- a/tests/thread-core.c +++ b/tests/thread-core.c @@ -78,9 +78,9 @@ static void __thread_scan_callback(int result, thread_network_scanning_state_e s void thread_device_role_callback(thread_device_role_e device_role, void *user_data) { - FUNC_ENTRY; - THREAD_DBG("Device Role is [%d]", device_role); - FUNC_EXIT; + FUNC_ENTRY; + THREAD_DBG("Device Role is [%d]", device_role); + FUNC_EXIT; } diff --git a/tests/thread-main.c b/tests/thread-main.c index 049d514..6f92c9f 100755 --- a/tests/thread-main.c +++ b/tests/thread-main.c @@ -115,9 +115,9 @@ int main(int arg, char **argv) g_io_add_watch(channel, (G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL), on_menu_manager_keyboard, manager); g_main_loop_run(mainloop); + thread_disable(manager->t_instance); OUT: - thread_disable(manager->t_instance); __deinit_func(); menu_manager_free(manager); diff --git a/tests/thread-network.c b/tests/thread-network.c index 2ad3fb3..54c8ed9 100644 --- a/tests/thread-network.c +++ b/tests/thread-network.c @@ -101,6 +101,10 @@ static int run_thread_network_set_active_dataset_tlvs(MManager *mm, struct menu_ msg("Set active dataset tlvs size: %d :: %s", buf_length, g_str_tlvs_buffer); + if (!buf_length || buf_length > (MENU_DATA_SIZE + 1)) { + msg("Invalid Input"); + goto OUT; + } for (int i = 0; i < buf_length; i++) { char subbuff[3]; memcpy(subbuff, &g_str_tlvs_buffer[2*i], 2); @@ -128,10 +132,17 @@ static int run_thread_network_get_active_dataset_tlvs(MManager *mm, struct menu_ int buf_length; uint8_t *tlvs_buffer; + char buf[1024]; + int ret = thread_network_get_active_dataset_tlvs(g_instance, &tlvs_buffer, &buf_length); if (ret == THREAD_ERROR_NONE) { msg("thread_network_get_active_dataset_tlvs success"); - msg("Active dataset tlvs size: %d :: %s", buf_length, tlvs_buffer); + + for (int i = 0; i < buf_length; i++) + snprintf(buf + i*2, 3, "%2.2x", tlvs_buffer[i]); + + msg("Active dataset tlvs size: %d :: %s", buf_length, buf); + free(tlvs_buffer); } else { msg("thread_network_get_active_dataset_tlvs failed"); } diff --git a/tests/thread-srp.c b/tests/thread-srp.c index 460edc9..70fb291 100644 --- a/tests/thread-srp.c +++ b/tests/thread-srp.c @@ -136,7 +136,8 @@ static int __run_thread_srp_client_register_service(MManager *mm, struct menu_da if (g_instance == NULL) goto OUT; - g_port = (uint64_t)atoi(g_str_port); + g_port = (uint64_t)strtoll(g_str_port, (char **)NULL, 10); + int ret = thread_srp_client_register_service(g_instance, g_service_name, g_service_type, g_port); if (ret == THREAD_ERROR_NONE) -- 2.7.4