From: Frank Lichtenheld <[email protected]> - Some users where re-creating strcpynt - Some users should have used strcpynt since they were not handling truncation at all - Some users where actually using it like memcpy and so should have used that
Partly based on a commit by Alejandro Pacheco <[email protected]>. Change-Id: I5ee597fb3b9efb93dbe19458ae6e7580d27ae3d2 Signed-off-by: Frank Lichtenheld <[email protected]> Acked-by: Razvan Cojocaru <[email protected]> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1993 --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1993 This mail reflects revision 2 of this Change. Acked-by according to Gerrit (reflected above): Razvan Cojocaru <[email protected]> diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c index 7f4dde4..713f41e 100644 --- a/src/openvpn/dns.c +++ b/src/openvpn/dns.c @@ -110,7 +110,7 @@ { return false; } - strncpy(addrcopy, addr, copylen); + memcpy(addrcopy, addr, copylen); addr = addrcopy; } @@ -391,7 +391,7 @@ { *(dst + offset++) = '.'; } - strncpy(dst + offset, src->name, len); + memcpy(dst + offset, src->name, len); offset += len; src = src->next; diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c index 0aef2d4..55e8105 100644 --- a/src/openvpn/manage.c +++ b/src/openvpn/manage.c @@ -2374,7 +2374,7 @@ } struct user_pass up; CLEAR(up); - strncpy(up.username, msg, sizeof(up.username) - 1); + strncpynt(up.username, msg, sizeof(up.username)); management_query_user_pass(management, &up, command, GET_USER_PASS_NEED_OK, (void *)0); return strcmp("ok", up.password) == 0; diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c index 6e9591f..0e4669a 100644 --- a/src/openvpn/misc.c +++ b/src/openvpn/misc.c @@ -303,7 +303,7 @@ if (password_buf[0]) { - strncpy(up->password, password_buf, USER_PASS_LEN); + strncpynt(up->password, password_buf, USER_PASS_LEN); } else { @@ -452,7 +452,7 @@ /* Use tag for blank password if we are not prompting for one */ if (flags & GET_USER_PASS_USERNAME_ONLY) { - strncpy(up->password, blank_up, sizeof(up->password)); + strncpynt(up->password, blank_up, sizeof(up->password)); } string_mod(up->username, CC_PRINT, CC_CRLF, 0); diff --git a/src/openvpn/networking_sitnl.c b/src/openvpn/networking_sitnl.c index 83915db..d6510bb 100644 --- a/src/openvpn/networking_sitnl.c +++ b/src/openvpn/networking_sitnl.c @@ -602,7 +602,7 @@ /* save result in output variables */ memcpy(best_gw, &res.gw, res.addr_size); - strncpy(best_iface, res.iface, IFNAMSIZ); + strncpynt(best_iface, res.iface, IFNAMSIZ); err: return ret; } diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 6664c73..4385419 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -6310,12 +6310,12 @@ else if (streq(p[0], "override-username") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_INSTANCE); - if (strlen(p[1]) > USER_PASS_LEN) + if (strlen(p[1]) >= USER_PASS_LEN) { msg(msglevel, "override-username exceeds the maximum length of %d " "characters", - USER_PASS_LEN); + USER_PASS_LEN - 1); /* disable the connection since ignoring the request to * set another username might cause serious problems */ diff --git a/src/openvpn/ps.c b/src/openvpn/ps.c index 08c2673..9a36d12 100644 --- a/src/openvpn/ps.c +++ b/src/openvpn/ps.c @@ -75,17 +75,6 @@ char *jfn; }; -#if 0 -static const char * -headc(const struct buffer *buf) -{ - static char foo[16]; - strncpy(foo, BSTR(buf), 15); - foo[15] = 0; - return foo; -} -#endif - static inline void close_socket_if_defined(const socket_descriptor_t sd) { diff --git a/src/openvpn/route.c b/src/openvpn/route.c index 33b2387..9209a76 100644 --- a/src/openvpn/route.c +++ b/src/openvpn/route.c @@ -2968,8 +2968,7 @@ if (msg.iface.index == TUN_ADAPTER_INDEX_INVALID) { - strncpy(msg.iface.name, tt->actual_name, sizeof(msg.iface.name)); - msg.iface.name[sizeof(msg.iface.name) - 1] = '\0'; + strncpynt(msg.iface.name, tt->actual_name, sizeof(msg.iface.name)); } status = do_route_service(add, &msg, sizeof(msg), tt->options.msg_channel); @@ -3747,7 +3746,7 @@ { rgi->gateway.addr = ntohl(((struct sockaddr_in *)route.gateway)->sin_addr.s_addr); rgi->flags = RGI_ADDR_DEFINED | RGI_IFACE_DEFINED; - strncpy(rgi->iface, interface->ifr_name, sizeof(rgi->iface)); + strncpynt(rgi->iface, interface->ifr_name, sizeof(rgi->iface)); } int32 address_size = 0; diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index 96e90f82..02fd556 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -433,7 +433,7 @@ len - 1); } - strncpy(openssl_ciphers, ciphers, len); + strncpynt(openssl_ciphers, ciphers, len); for (size_t i = 0; i < strlen(openssl_ciphers); i++) { diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c index 6631718..43ed980 100644 --- a/src/openvpn/ssl_verify.c +++ b/src/openvpn/ssl_verify.c @@ -1640,7 +1640,7 @@ "TLS: Replacing client provided username '%s' with " "username from override-user '%s'", up->username, multi->locked_username); - strncpy(up->username, multi->locked_username, sizeof(up->username)); + strncpynt(up->username, multi->locked_username, sizeof(up->username)); } /* diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 133f670..9e7e7d7 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -127,8 +127,7 @@ if (addr.iface.index == TUN_ADAPTER_INDEX_INVALID) { - strncpy(addr.iface.name, tt->actual_name, sizeof(addr.iface.name)); - addr.iface.name[sizeof(addr.iface.name) - 1] = '\0'; + strncpynt(addr.iface.name, tt->actual_name, sizeof(addr.iface.name)); } if (addr.family == AF_INET) @@ -212,7 +211,7 @@ { dns.domains[dstlen++] = ','; } - strncpy(dns.domains + dstlen, o->domain_search_list[i], srclen + 1); + memcpy(dns.domains + dstlen, o->domain_search_list[i], srclen + 1); } msg(D_LOW, "%s DNS domains on '%s' (if_index = %lu) using service", @@ -261,8 +260,7 @@ .addr_len = addr_len }; /* interface name is required */ - strncpy(dns.iface.name, tt->actual_name, sizeof(dns.iface.name)); - dns.iface.name[sizeof(dns.iface.name) - 1] = '\0'; + strncpynt(dns.iface.name, tt->actual_name, sizeof(dns.iface.name)); if (addr_len > _countof(dns.addr)) { @@ -326,8 +324,7 @@ .addr_len = addr_len }; /* interface name is required */ - strncpy(wins.iface.name, tt->actual_name, sizeof(wins.iface.name)); - wins.iface.name[sizeof(wins.iface.name) - 1] = '\0'; + strncpynt(wins.iface.name, tt->actual_name, sizeof(wins.iface.name)); if (addr_len > _countof(wins.addr)) { diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c index 9a68bbb..33151cb 100644 --- a/src/openvpnserv/interactive.c +++ b/src/openvpnserv/interactive.c @@ -1810,7 +1810,7 @@ wide_domains = NULL; goto out; } - strncpy(undo_data->itf_name, itf_name, sizeof(undo_data->itf_name)); + strncpy_s(undo_data->itf_name, sizeof(undo_data->itf_name), itf_name, _TRUNCATE); undo_data->domains = wide_domains; if (AddDnsSearchDomains(list_key, have_list, wide_domains) == FALSE diff --git a/tests/unit_tests/openvpn/test_auth_token.c b/tests/unit_tests/openvpn/test_auth_token.c index 9e7b5b9..89c418e 100644 --- a/tests/unit_tests/openvpn/test_auth_token.c +++ b/tests/unit_tests/openvpn/test_auth_token.c @@ -273,7 +273,7 @@ } else if (streq(name, "session_id")) { - strncpy(last_session_id_value, value, sizeof(last_session_id_value)); + strncpynt(last_session_id_value, value, sizeof(last_session_id_value)); } } diff --git a/tests/unit_tests/openvpn/test_user_pass.c b/tests/unit_tests/openvpn/test_user_pass.c index c0a0866..5ad4730 100644 --- a/tests/unit_tests/openvpn/test_user_pass.c +++ b/tests/unit_tests/openvpn/test_user_pass.c @@ -53,7 +53,7 @@ for (int i = 0; i < QUERY_USER_NUMSLOTS && query_user[i].response != NULL; i++) { check_expected_ptr(query_user[i].prompt); - strncpy(query_user[i].response, mock_ptr_type(char *), query_user[i].response_len); + strncpynt(query_user[i].response, mock_ptr_type(char *), query_user[i].response_len); } return mock(); _______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
