From e5b7ca4bcea8c404d95e00f5db4c40d47b6e185b Mon Sep 17 00:00:00 2001 From: Andrew Yourtchenko Date: Fri, 29 Jan 2021 14:18:12 +0000 Subject: libmemif: fix insecure uses of strncpy A calling patterm of "strncpy(dst, src, strlen(src))" invites a lot of troubles. However, even using the target size may result in a problem if the string is longer, since then the termination is not done. Use strlcpy(dst, src, sizeof(dst)), which will always null-terminate the string. Change-Id: I8ddaf3dc8380a78af08914e81849279dae7ab24a Type: fix Signed-off-by: Andrew Yourtchenko Signed-off-by: Jakub Grajciar --- extras/libmemif/src/CMakeLists.txt | 7 +++++++ extras/libmemif/src/main.c | 42 +++++++++++++++---------------------- extras/libmemif/src/memif_private.h | 27 ++++++++++++++++++++++++ extras/libmemif/src/socket.c | 36 ++++++++++++++----------------- 4 files changed, 67 insertions(+), 45 deletions(-) (limited to 'extras/libmemif') diff --git a/extras/libmemif/src/CMakeLists.txt b/extras/libmemif/src/CMakeLists.txt index aced550ff5d..ddb8a52f82b 100644 --- a/extras/libmemif/src/CMakeLists.txt +++ b/extras/libmemif/src/CMakeLists.txt @@ -34,6 +34,13 @@ include_directories(${HEADERS_DIR}) add_library(memif SHARED ${MEMIF_SOURCES}) target_link_libraries(memif ${CMAKE_THREAD_LIBS_INIT}) + +find_library(LIB_BSD bsd) +if(LIB_BSD) + add_compile_definitions(HAS_LIB_BSD) + target_link_libraries(memif ${LIB_BSD}) +endif() + foreach(file ${MEMIF_HEADERS}) get_filename_component(dir ${file} DIRECTORY) install( diff --git a/extras/libmemif/src/main.c b/extras/libmemif/src/main.c index d7345d6cf8b..1eb6929137e 100644 --- a/extras/libmemif/src/main.c +++ b/extras/libmemif/src/main.c @@ -158,14 +158,11 @@ memif_strerror (int err_code) { if (err_code >= ERRLIST_LEN) { - strncpy (memif_buf, MEMIF_ERR_UNDEFINED, strlen (MEMIF_ERR_UNDEFINED)); - memif_buf[strlen (MEMIF_ERR_UNDEFINED)] = '\0'; + strlcpy (memif_buf, MEMIF_ERR_UNDEFINED, sizeof (memif_buf)); } else { - strncpy (memif_buf, memif_errlist[err_code], - strlen (memif_errlist[err_code])); - memif_buf[strlen (memif_errlist[err_code])] = '\0'; + strlcpy (memif_buf, memif_errlist[err_code], sizeof (memif_buf)); } return memif_buf; } @@ -532,14 +529,12 @@ memif_init (memif_control_fd_update_t * on_control_fd_update, char *app_name, if (app_name != NULL) { - uint8_t len = (strlen (app_name) > MEMIF_NAME_LEN) - ? strlen (app_name) : MEMIF_NAME_LEN; - strncpy ((char *) lm->app_name, app_name, len); + strlcpy ((char *) lm->app_name, app_name, sizeof (lm->app_name)); } else { - strncpy ((char *) lm->app_name, MEMIF_DEFAULT_APP_NAME, - strlen (MEMIF_DEFAULT_APP_NAME)); + strlcpy ((char *) lm->app_name, MEMIF_DEFAULT_APP_NAME, + sizeof (lm->app_name)); } lm->poll_cancel_fd = -1; @@ -699,14 +694,12 @@ memif_per_thread_init (memif_per_thread_main_handle_t * pt_main, /* set app name */ if (app_name != NULL) { - uint8_t len = (strlen (app_name) > MEMIF_NAME_LEN) - ? strlen (app_name) : MEMIF_NAME_LEN; - strncpy ((char *) lm->app_name, app_name, len); + strlcpy ((char *) lm->app_name, app_name, MEMIF_NAME_LEN); } else { - strncpy ((char *) lm->app_name, MEMIF_DEFAULT_APP_NAME, - strlen (MEMIF_DEFAULT_APP_NAME)); + strlcpy ((char *) lm->app_name, MEMIF_DEFAULT_APP_NAME, + sizeof (lm->app_name)); } lm->poll_cancel_fd = -1; @@ -885,8 +878,7 @@ memif_socket_start_listening (memif_socket_t * ms) DBG ("socket %d created", ms->fd); un.sun_family = AF_UNIX; - strncpy ((char *) un.sun_path, (char *) ms->filename, - sizeof (un.sun_path) - 1); + strlcpy ((char *) un.sun_path, (char *) ms->filename, sizeof (un.sun_path)); if (setsockopt (ms->fd, SOL_SOCKET, SO_PASSCRED, &on, sizeof (on)) < 0) { err = memif_syscall_error_handler (errno); @@ -963,7 +955,7 @@ memif_create_socket (memif_socket_handle_t * sock, const char *filename, goto error; } memset (ms->filename, 0, strlen (filename) + sizeof (char)); - strncpy ((char *) ms->filename, filename, strlen (filename)); + strlcpy ((char *) ms->filename, filename, sizeof (ms->filename)); ms->type = MEMIF_SOCKET_TYPE_NONE; @@ -1047,7 +1039,7 @@ memif_per_thread_create_socket (memif_per_thread_main_handle_t pt_main, goto error; } memset (ms->filename, 0, strlen (filename) + sizeof (char)); - strncpy ((char *) ms->filename, filename, strlen (filename)); + strlcpy ((char *) ms->filename, filename, sizeof (ms->filename)); ms->type = MEMIF_SOCKET_TYPE_NONE; @@ -1150,12 +1142,12 @@ memif_create (memif_conn_handle_t * c, memif_conn_args_t * args, conn->private_ctx = private_ctx; memset (&conn->run_args, 0, sizeof (memif_conn_run_args_t)); - uint8_t l = strlen ((char *) args->interface_name); - strncpy ((char *) conn->args.interface_name, (char *) args->interface_name, - l); + strlcpy ((char *) conn->args.interface_name, (char *) args->interface_name, + sizeof (conn->args.interface_name)); - if ((l = strlen ((char *) args->secret)) > 0) - strncpy ((char *) conn->args.secret, (char *) args->secret, l); + if ((strlen ((char *) args->secret)) > 0) + strlcpy ((char *) conn->args.secret, (char *) args->secret, + sizeof (conn->args.secret)); if (args->socket != NULL) conn->args.socket = args->socket; @@ -1260,7 +1252,7 @@ memif_request_connection (memif_conn_handle_t c) sun.sun_family = AF_UNIX; - strncpy (sun.sun_path, (char *) ms->filename, sizeof (sun.sun_path) - 1); + strlcpy (sun.sun_path, (char *) ms->filename, sizeof (sun.sun_path)); if (connect (sockfd, (struct sockaddr *) &sun, sizeof (struct sockaddr_un)) == 0) diff --git a/extras/libmemif/src/memif_private.h b/extras/libmemif/src/memif_private.h index dd58d6256f3..59899fd6285 100644 --- a/extras/libmemif/src/memif_private.h +++ b/extras/libmemif/src/memif_private.h @@ -66,6 +66,33 @@ _Static_assert (strlen (MEMIF_DEFAULT_APP_NAME) <= MEMIF_NAME_LEN, #define DBG(...) #endif /* MEMIF_DBG */ +#ifndef HAS_LIB_BSD +static inline size_t +strlcpy (char *dest, const char *src, size_t len) +{ + const char *s = src; + size_t n = len; + + while (--n > 0) + { + if ((*dest++ = *s++) == '\0') + break; + } + + if (n == 0) + { + if (len != 0) + *dest = '\0'; + while (*s++) + ; + } + + return (s - src - 1); +} +#else +#include +#endif + typedef enum { MEMIF_SOCKET_TYPE_NONE = 0, /* unassigned, not used by any interface */ diff --git a/extras/libmemif/src/socket.c b/extras/libmemif/src/socket.c index 24546162ee9..b801cac75ba 100644 --- a/extras/libmemif/src/socket.c +++ b/extras/libmemif/src/socket.c @@ -111,8 +111,7 @@ memif_msg_send_hello (libmemif_main_t * lm, int fd) h->max_region = MEMIF_MAX_REGION; h->max_log2_ring_size = MEMIF_MAX_LOG2_RING_SIZE; - strncpy ((char *) h->name, (char *) lm->app_name, - strlen ((char *) lm->app_name)); + strlcpy ((char *) h->name, (char *) lm->app_name, sizeof (h->name)); /* msg hello is not enqueued but sent directly, because it is the first msg to be sent */ @@ -139,8 +138,7 @@ memif_msg_enq_init (memif_connection_t * c) i->id = c->args.interface_id; i->mode = c->args.mode; - strncpy ((char *) i->name, (char *) lm->app_name, - strlen ((char *) lm->app_name)); + strlcpy ((char *) i->name, (char *) lm->app_name, sizeof (i->name)); if (strlen ((char *) c->args.secret) > 0) strncpy ((char *) i->secret, (char *) c->args.secret, sizeof (i->secret)); @@ -260,8 +258,8 @@ memif_msg_enq_connect (memif_connection_t * c) e->msg.type = MEMIF_MSG_TYPE_CONNECT; e->fd = -1; - strncpy ((char *) cm->if_name, (char *) c->args.interface_name, - strlen ((char *) c->args.interface_name)); + strlcpy ((char *) cm->if_name, (char *) c->args.interface_name, + sizeof (cm->if_name)); e->next = NULL; if (c->msg_queue == NULL) @@ -295,8 +293,8 @@ memif_msg_enq_connected (memif_connection_t * c) e->msg.type = MEMIF_MSG_TYPE_CONNECTED; e->fd = -1; - strncpy ((char *) cm->if_name, (char *) c->args.interface_name, - strlen ((char *) c->args.interface_name)); + strlcpy ((char *) cm->if_name, (char *) c->args.interface_name, + sizeof (cm->if_name)); e->next = NULL; if (c->msg_queue == NULL) @@ -327,12 +325,12 @@ memif_msg_send_disconnect (int fd, uint8_t * err_string, uint32_t err_code) msg.type = MEMIF_MSG_TYPE_DISCONNECT; d->code = err_code; uint16_t l = strlen ((char *) err_string); - if (l > 96) + if (l > sizeof (d->string) - 1) { - DBG ("Disconnect string too long. Sending first 96 characters."); - l = 96; + DBG ("Disconnect string too long. Sending the first %d characters.", + sizeof (d->string) - 1); } - strncpy ((char *) d->string, (char *) err_string, l); + strlcpy ((char *) d->string, (char *) err_string, sizeof (d->string)); return memif_msg_send (fd, &msg, -1); } @@ -356,8 +354,7 @@ memif_msg_receive_hello (memif_connection_t * c, memif_msg_t * msg) c->run_args.log2_ring_size = memif_min (h->max_log2_ring_size, c->args.log2_ring_size); c->run_args.buffer_size = c->args.buffer_size; - strncpy ((char *) c->remote_name, (char *) h->name, - strlen ((char *) h->name)); + strlcpy ((char *) c->remote_name, (char *) h->name, sizeof (c->remote_name)); return MEMIF_ERR_SUCCESS; /* 0 */ } @@ -420,8 +417,7 @@ memif_msg_receive_init (memif_socket_t * ms, int fd, memif_msg_t * msg) goto error; } - strncpy ((char *) c->remote_name, (char *) i->name, - strlen ((char *) i->name)); + strlcpy ((char *) c->remote_name, (char *) i->name, sizeof (c->remote_name)); if (strlen ((char *) c->args.secret) > 0) { @@ -588,8 +584,8 @@ memif_msg_receive_connect (memif_connection_t * c, memif_msg_t * msg) if (err != MEMIF_ERR_SUCCESS) return err; - strncpy ((char *) c->remote_if_name, (char *) cm->if_name, - strlen ((char *) cm->if_name)); + strlcpy ((char *) c->remote_if_name, (char *) cm->if_name, + sizeof (c->remote_if_name)); int i; if (c->on_interrupt != NULL) @@ -625,7 +621,7 @@ memif_msg_receive_connected (memif_connection_t * c, memif_msg_t * msg) return err; strncpy ((char *) c->remote_if_name, (char *) cm->if_name, - strlen ((char *) cm->if_name)); + sizeof (c->remote_if_name)); int i; if (c->on_interrupt != NULL) @@ -650,7 +646,7 @@ memif_msg_receive_disconnect (memif_connection_t * c, memif_msg_t * msg) memset (c->remote_disconnect_string, 0, sizeof (c->remote_disconnect_string)); strncpy ((char *) c->remote_disconnect_string, (char *) d->string, - strlen ((char *) d->string)); + sizeof (c->remote_disconnect_string)); /* on returning error, handle function will call memif_disconnect () */ DBG ("disconnect received: %s, mode: %d", -- cgit 1.2.3-korg