From b9771c5f5edcbeb965fa291a281943d866c3ddb6 Mon Sep 17 00:00:00 2001 From: lurchi Date: Thu, 27 Jun 2019 10:49:40 +0200 Subject: [PATCH] use GNUNET_strlcpy instead of strncpy where possible --- src/arm/gnunet-service-arm.c | 2 +- src/cadet/gnunet-service-cadet_peer.c | 7 ++-- src/exit/gnunet-helper-exit-windows.c | 51 +++++++++++++++++++++--- src/regex/regex_test_lib.c | 5 ++- src/testbed/gnunet-helper-testbed.c | 1 + src/testbed/gnunet-service-testbed.c | 2 + src/testbed/gnunet-service-testbed_oc.c | 6 +-- src/testbed/testbed_api.c | 1 + src/testbed/testbed_api_hosts.c | 6 +++ src/transport/tcp_connection_legacy.c | 2 +- src/transport/tcp_service_legacy.c | 2 +- src/util/client.c | 6 +-- src/util/common_logging.c | 10 ++--- src/util/gnunet-ecc.c | 2 +- src/util/service.c | 6 +-- src/util/socks.c | 9 +++-- src/vpn/gnunet-helper-vpn-windows.c | 52 +++++++++++++++++++++---- 17 files changed, 130 insertions(+), 40 deletions(-) diff --git a/src/arm/gnunet-service-arm.c b/src/arm/gnunet-service-arm.c index 17304d3b3..4e3474cb6 100644 --- a/src/arm/gnunet-service-arm.c +++ b/src/arm/gnunet-service-arm.c @@ -290,7 +290,7 @@ add_unixpath (struct sockaddr **saddrs, un = GNUNET_new (struct sockaddr_un); un->sun_family = AF_UNIX; - strncpy (un->sun_path, unixpath, sizeof (un->sun_path) - 1); + GNUNET_strlcpy (un->sun_path, unixpath, sizeof (un->sun_path)); #ifdef LINUX if (GNUNET_YES == abstract) un->sun_path[0] = '\0'; diff --git a/src/cadet/gnunet-service-cadet_peer.c b/src/cadet/gnunet-service-cadet_peer.c index 8d55e6386..c25f46de5 100644 --- a/src/cadet/gnunet-service-cadet_peer.c +++ b/src/cadet/gnunet-service-cadet_peer.c @@ -256,11 +256,10 @@ GCP_2s (const struct CadetPeer *cp) return "NULL"; - strncpy (buf, - ret, - sizeof (buf) - 1); + GNUNET_strlcpy (buf, + ret, + sizeof (buf)); GNUNET_free (ret); - buf[4] = '\0'; return buf; } diff --git a/src/exit/gnunet-helper-exit-windows.c b/src/exit/gnunet-helper-exit-windows.c index 6633fbc31..1e17ceaac 100644 --- a/src/exit/gnunet-helper-exit-windows.c +++ b/src/exit/gnunet-helper-exit-windows.c @@ -250,6 +250,37 @@ WINBASEAPI HANDLE WINAPI ReOpenFile (HANDLE, DWORD, DWORD, DWORD); */ typedef BOOL (WINAPI *LPFN_ISWOW64PROCESS) (HANDLE, PBOOL); + +/** + * Like strlcpy but portable. The given string @a src is copied in full length + * (until its null byte). The destination buffer is guaranteed to be + * null-terminated. + * + * to a destination buffer + * and ensures that the destination string is null-terminated. + * + * @param dst destination of the copy + * @param src source of the copy, must be null-terminated + * @param n the length of the string to copy, including its terminating null + * byte + * @return the length of the string that was copied, excluding the terminating + * null byte + */ +size_t +GNUNET_strlcpy(char *dst, const char *src, size_t n) +{ + size_t ret; + size_t slen; + + GNUNET_assert (0 != n); + ret = strlen (src); + slen = GNUNET_MIN (ret, n - 1); + memcpy (dst, src, slen); + dst[slen] = '\0'; + return ret; +} + + /** * Determines if the host OS is win32 or win64 * @@ -473,7 +504,9 @@ setup_interface () * Set the device's hardware ID and add it to a list. * This information will later on identify this device in registry. */ - strncpy (hwidlist, HARDWARE_ID, LINE_LEN); + str_length = GNUNET_strlcpy (hwidlist, + HARDWARE_ID, + sizeof (hwidlist)) + 1; /** * this is kind of over-complicated, but allows keeps things independent of * how the openvpn-hwid is actually stored. @@ -481,8 +514,12 @@ setup_interface () * A HWID list is double-\0 terminated and \0 separated */ str_length = strlen (hwidlist) + 1; - strncpy (&hwidlist[str_length], secondary_hwid, LINE_LEN); - str_length += strlen (&hwidlist[str_length]) + 1; + str_length += GNUNET_strlcpy (&hwidlist[str_length], + secondary_hwid, + sizeof (hwidlist) - str_length) + 1; + GNUNET_assert (str_length < sizeof (hwidlist)); + hwidlist[str_length] = '\0'; + ++str_length; /** * Locate the inf-file, we need to store it somewhere where the system can @@ -716,9 +753,11 @@ resolve_interface_name () /* * we have successfully found OUR instance, * save the device GUID before exiting + * + * We can use GNUNET_strlcpy here because instance key is null- + * terminated by RegEnumKeyExA. */ - - strncpy (device_guid, instance_key, 256); + GNUNET_strlcpy (device_guid, instance_key, sizeof (device_guid)); retval = TRUE; fprintf (stderr, "DEBUG: Interface Name lookup succeeded on retry %d, got \"%s\" %s\n", retrys, device_visible_name, device_guid); @@ -1496,7 +1535,7 @@ main (int argc, char **argv) return 1; } - strncpy (hwid, argv[1], LINE_LEN); + GNUNET_strlcpy (hwid, argv[1], sizeof (hwid)); hwid[LINE_LEN - 1] = '\0'; /* diff --git a/src/regex/regex_test_lib.c b/src/regex/regex_test_lib.c index bd1a06a53..7becd567c 100644 --- a/src/regex/regex_test_lib.c +++ b/src/regex/regex_test_lib.c @@ -392,7 +392,10 @@ regex_split (struct RegexCombineCtx *ctx, char *suffix; suffix = GNUNET_malloc (len - prefix_l + 1); - strncpy (suffix, &ctx->s[prefix_l], len - prefix_l + 1); + /* + * We can use GNUNET_strlcpy because ctx->s is null-terminated + */ + GNUNET_strlcpy (suffix, &ctx->s[prefix_l], len - prefix_l + 1); /* Suffix saved, truncate current node so it only contains the prefix, * copy any children nodes to put as grandchildren and initialize new empty diff --git a/src/testbed/gnunet-helper-testbed.c b/src/testbed/gnunet-helper-testbed.c index 25d9724fa..c56a795a6 100644 --- a/src/testbed/gnunet-helper-testbed.c +++ b/src/testbed/gnunet-helper-testbed.c @@ -374,6 +374,7 @@ tokenizer_cb (void *cls, if (0 != hostname_size) { hostname = GNUNET_malloc (hostname_size + 1); + /* intentionally use strncpy (hostname not null terminated) */ (void) strncpy (hostname, ((char *) &msg[1]) + trusted_ip_size + 1, hostname_size); hostname[hostname_size] = '\0'; diff --git a/src/testbed/gnunet-service-testbed.c b/src/testbed/gnunet-service-testbed.c index 51460f65b..d740d31bc 100644 --- a/src/testbed/gnunet-service-testbed.c +++ b/src/testbed/gnunet-service-testbed.c @@ -541,10 +541,12 @@ handle_add_host (void *cls, if (0 != username_length) { username = GNUNET_malloc (username_length + 1); + /* intentionally use strncpy (message payload is not null terminated) */ strncpy (username, ptr, username_length); ptr += username_length; } hostname = GNUNET_malloc (hostname_length + 1); + /* intentionally use strncpy (message payload is not null terminated) */ strncpy (hostname, ptr, hostname_length); diff --git a/src/testbed/gnunet-service-testbed_oc.c b/src/testbed/gnunet-service-testbed_oc.c index 98a4282df..4d6a4d446 100644 --- a/src/testbed/gnunet-service-testbed_oc.c +++ b/src/testbed/gnunet-service-testbed_oc.c @@ -1908,9 +1908,9 @@ handle_remote_overlay_connect (void *cls, rocc->a_id = msg->peer_identity; GNUNET_TESTING_peer_get_identity (peer->details.local.peer, &pid); - (void) strncpy (pid_str, - GNUNET_i2s (&pid), - 15); + (void) GNUNET_strlcpy (pid_str, + GNUNET_i2s (&pid), + sizeof (pid_str)); LOG_DEBUG ("0x%llx: Remote overlay connect %s to peer %s with hello size: %u\n", rocc->op_id, pid_str, diff --git a/src/testbed/testbed_api.c b/src/testbed/testbed_api.c index 793ed4edd..22ce7eb72 100644 --- a/src/testbed/testbed_api.c +++ b/src/testbed/testbed_api.c @@ -2014,6 +2014,7 @@ GNUNET_TESTBED_create_helper_init_msg_ (const char *trusted_ip, msg->config_size = htons (config_size); (void) strcpy ((char *) &msg[1], trusted_ip); if (0 != hostname_len) + /* intentionally use strncpy (no null byte needed) */ (void) strncpy (((char *) &msg[1]) + trusted_ip_len + 1, hostname, hostname_len); return msg; diff --git a/src/testbed/testbed_api_hosts.c b/src/testbed/testbed_api_hosts.c index 327f84f2a..d6521f766 100644 --- a/src/testbed/testbed_api_hosts.c +++ b/src/testbed/testbed_api_hosts.c @@ -462,6 +462,9 @@ GNUNET_TESTBED_hosts_load_from_file (const char *filename, { size = pmatch[2].rm_eo - pmatch[2].rm_so; username = GNUNET_malloc (size + 1); + /* + * Intentionally use strncpy (buf is not necessarily null-terminated) + */ username[size] = '\0'; GNUNET_assert (NULL != strncpy (username, buf + pmatch[2].rm_so, size)); } @@ -471,6 +474,9 @@ GNUNET_TESTBED_hosts_load_from_file (const char *filename, } size = pmatch[3].rm_eo - pmatch[3].rm_so; hostname = GNUNET_malloc (size + 1); + /* + * Intentionally use strncpy (buf is not necessarily null-terminated) + */ hostname[size] = '\0'; GNUNET_assert (NULL != strncpy (hostname, buf + pmatch[3].rm_so, size)); LOG (GNUNET_ERROR_TYPE_DEBUG, diff --git a/src/transport/tcp_connection_legacy.c b/src/transport/tcp_connection_legacy.c index 6ecf50b79..cfb088361 100644 --- a/src/transport/tcp_connection_legacy.c +++ b/src/transport/tcp_connection_legacy.c @@ -901,7 +901,7 @@ GNUNET_CONNECTION_create_from_connect_to_unixpath (const struct GNUNET_CONFIGURA GNUNET_assert (0 < strlen (unixpath)); /* sanity check */ un = GNUNET_new (struct sockaddr_un); un->sun_family = AF_UNIX; - strncpy (un->sun_path, unixpath, sizeof (un->sun_path) - 1); + GNUNET_strlcpy (un->sun_path, unixpath, sizeof (un->sun_path)); #ifdef LINUX { int abstract; diff --git a/src/transport/tcp_service_legacy.c b/src/transport/tcp_service_legacy.c index 641d0195a..19508a39f 100644 --- a/src/transport/tcp_service_legacy.c +++ b/src/transport/tcp_service_legacy.c @@ -468,7 +468,7 @@ add_unixpath (struct sockaddr **saddrs, un = GNUNET_new (struct sockaddr_un); un->sun_family = AF_UNIX; - strncpy (un->sun_path, unixpath, sizeof (un->sun_path) - 1); + GNUNET_strlcpy (un->sun_path, unixpath, sizeof (un->sun_path)); #ifdef LINUX if (GNUNET_YES == abstract) un->sun_path[0] = '\0'; diff --git a/src/util/client.c b/src/util/client.c index 05e05a328..313cc23af 100644 --- a/src/util/client.c +++ b/src/util/client.c @@ -532,9 +532,9 @@ try_unixpath (const char *service_name, 0, sizeof (s_un)); s_un.sun_family = AF_UNIX; - strncpy (s_un.sun_path, - unixpath, - sizeof (s_un.sun_path) - 1); + GNUNET_strlcpy (s_un.sun_path, + unixpath, + sizeof (s_un.sun_path)); #ifdef LINUX { int abstract; diff --git a/src/util/common_logging.c b/src/util/common_logging.c index b5678e5be..3193878b8 100644 --- a/src/util/common_logging.c +++ b/src/util/common_logging.c @@ -1082,11 +1082,11 @@ mylog (enum GNUNET_ErrorType kind, return; } flush_bulk (date); - strncpy (last_bulk, buf, sizeof (last_bulk)); + GNUNET_strlcpy (last_bulk, buf, sizeof (last_bulk)); last_bulk_repeat = 0; last_bulk_kind = kind; last_bulk_time = GNUNET_TIME_absolute_get (); - strncpy (last_bulk_comp, comp, COMP_TRACK_SIZE); + GNUNET_strlcpy (last_bulk_comp, comp, sizeof (last_bulk_comp)); output_message (kind, comp, date, buf); } } @@ -1364,9 +1364,8 @@ GNUNET_i2s (const struct GNUNET_PeerIdentity *pid) if (NULL == pid) return "NULL"; ret = GNUNET_CRYPTO_eddsa_public_key_to_string (&pid->public_key); - strncpy (buf, ret, sizeof (buf) - 1); + GNUNET_strlcpy (buf, ret, sizeof (buf)); GNUNET_free (ret); - buf[4] = '\0'; return buf; } @@ -1390,9 +1389,8 @@ GNUNET_i2s2 (const struct GNUNET_PeerIdentity *pid) if (NULL == pid) return "NULL"; ret = GNUNET_CRYPTO_eddsa_public_key_to_string (&pid->public_key); - strncpy (buf, ret, sizeof (buf) - 1); + GNUNET_strlcpy (buf, ret, sizeof (buf)); GNUNET_free (ret); - buf[4] = '\0'; return buf; } diff --git a/src/util/gnunet-ecc.c b/src/util/gnunet-ecc.c index 27ef59c9f..94ffb4308 100644 --- a/src/util/gnunet-ecc.c +++ b/src/util/gnunet-ecc.c @@ -96,7 +96,7 @@ create_keys (const char *fn, const char *prefix) } if (NULL != prefix) { - strncpy (vanity, prefix, KEY_STR_LEN); + GNUNET_strlcpy (vanity, prefix, sizeof (vanity)); len = GNUNET_MIN (strlen (prefix), KEY_STR_LEN); n = len * 5 / 8; rest = len * 5 % 8; diff --git a/src/util/service.c b/src/util/service.c index 4fd16f93d..d03650501 100644 --- a/src/util/service.c +++ b/src/util/service.c @@ -1061,9 +1061,9 @@ add_unixpath (struct sockaddr **saddrs, un = GNUNET_new (struct sockaddr_un); un->sun_family = AF_UNIX; - strncpy (un->sun_path, - unixpath, - sizeof (un->sun_path) - 1); + GNUNET_strlcpy (un->sun_path, + unixpath, + sizeof (un->sun_path)); #ifdef LINUX if (GNUNET_YES == abstract) un->sun_path[0] = '\0'; diff --git a/src/util/socks.c b/src/util/socks.c index 7eca04878..9e974e6bb 100644 --- a/src/util/socks.c +++ b/src/util/socks.c @@ -76,7 +76,7 @@ const char * SOCKS5_REP_names(int rep) /** * Encode a string for the SOCKS5 protocol by prefixing it a byte stating its - * length and stipping the trailing zero byte. Truncates any string longer + * length and stripping the trailing zero byte. Truncates any string longer * than 255 bytes. * * @param b buffer to contain the encoded string @@ -96,7 +96,10 @@ SOCK5_proto_string(unsigned char * b, l=255; } *(b++) = (unsigned char) l; - strncpy ((char *)b, s, l); + /* + * intentionally use strncpy (trailing zero byte must be stripped in b) + */ + strncpy ((char*)b, s, l); return b+l; } @@ -489,7 +492,7 @@ GNUNET_SOCKS_init_handshake_noauth () */ void GNUNET_SOCKS_set_handshake_destination (struct GNUNET_SOCKS_Handshake *ih, - const char *host, uint16_t port) + const char *host, uint16_t port) { union { struct in_addr in4; diff --git a/src/vpn/gnunet-helper-vpn-windows.c b/src/vpn/gnunet-helper-vpn-windows.c index 14c0c3fec..4ccecb873 100644 --- a/src/vpn/gnunet-helper-vpn-windows.c +++ b/src/vpn/gnunet-helper-vpn-windows.c @@ -250,6 +250,37 @@ WINBASEAPI HANDLE WINAPI ReOpenFile (HANDLE, DWORD, DWORD, DWORD); */ typedef BOOL (WINAPI *LPFN_ISWOW64PROCESS) (HANDLE, PBOOL); + +/** + * Like strlcpy but portable. The given string @a src is copied in full length + * (until its null byte). The destination buffer is guaranteed to be + * null-terminated. + * + * to a destination buffer + * and ensures that the destination string is null-terminated. + * + * @param dst destination of the copy + * @param src source of the copy, must be null-terminated + * @param n the length of the string to copy, including its terminating null + * byte + * @return the length of the string that was copied, excluding the terminating + * null byte + */ +size_t +GNUNET_strlcpy(char *dst, const char *src, size_t n) +{ + size_t ret; + size_t slen; + + GNUNET_assert (0 != n); + ret = strlen (src); + slen = GNUNET_MIN (ret, n - 1); + memcpy (dst, src, slen); + dst[slen] = '\0'; + return ret; +} + + /** * Determines if the host OS is win32 or win64 * @@ -476,16 +507,21 @@ setup_interface () * Set the device's hardware ID and add it to a list. * This information will later on identify this device in registry. */ - strncpy (hwidlist, HARDWARE_ID, LINE_LEN); + str_len = GNUNET_strlcpy (hwidlist, + HARDWARE_ID, + sizeof (hwidList)) + 1; /** * this is kind of over-complicated, but allows keeps things independent of * how the openvpn-hwid is actually stored. * * A HWID list is double-\0 terminated and \0 separated */ - str_length = strlen (hwidlist) + 1; - strncpy (&hwidlist[str_length], secondary_hwid, LINE_LEN); - str_length += strlen (&hwidlist[str_length]) + 1; + str_len += GNUNET_strlcpy (&hwidlist[str_length], + secondary_hwid, + sizeof (hwidlist) - str_len) + 1; + GNUNET_assert (str_len < sizeof (hwidlist)); + hwidlist[str_len] = '\0'; + ++str_len; /** * Locate the inf-file, we need to store it somewhere where the system can @@ -719,9 +755,11 @@ resolve_interface_name () /* * we have successfully found OUR instance, * save the device GUID before exiting + * + * We can use GNUNET_strlcpy here because instance key is null- + * terminated by RegEnumKeyExA. */ - - strncpy (device_guid, instance_key, 256); + GNUNET_strlcpy (device_guid, instance_key, sizeof (device_guid)); retval = TRUE; fprintf (stderr, "DEBUG: Interface Name lookup succeeded on retry %d, got \"%s\" %s\n", retrys, device_visible_name, device_guid); @@ -1494,7 +1532,7 @@ main (int argc, char **argv) return 1; } - strncpy (hwid, argv[1], LINE_LEN); + GNUNET_strlcpy (hwid, argv[1], sizeof (hwid)); hwid[LINE_LEN - 1] = '\0'; /* -- 2.25.1