[Openvpn-devel] [S] Change in openvpn[master]: Fix snprintf/swnprintf related compiler warnings
cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/549?usp=email ) Change subject: Fix snprintf/swnprintf related compiler warnings .. Fix snprintf/swnprintf related compiler warnings When openvpn_snprintf is replaced by snprintf the GCC/MSVC compiler will perform additional checks that the result is not truncated. This warning can be avoid by either explicitly checking the return value of snprintf (proxy) or ensuring that it is never truncated(tls crypt) Change-Id: If23988a05dd53a519c5e57f2aa3b2d10bd29df1d Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld Message-Id: <20240326104101.531291-1-fr...@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28475.html Signed-off-by: Gert Doering --- M src/openvpn/proxy.c M src/openvpn/socks.c M src/openvpn/ssl_openssl.c M src/openvpn/tls_crypt.c M src/openvpnserv/interactive.c 5 files changed, 25 insertions(+), 17 deletions(-) diff --git a/src/openvpn/proxy.c b/src/openvpn/proxy.c index c904301..5c1cdcb 100644 --- a/src/openvpn/proxy.c +++ b/src/openvpn/proxy.c @@ -948,17 +948,21 @@ } /* send digest response */ -openvpn_snprintf(buf, sizeof(buf), "Proxy-Authorization: Digest username=\"%s\", realm=\"%s\", nonce=\"%s\", uri=\"%s\", qop=%s, nc=%s, cnonce=\"%s\", response=\"%s\"%s", - username, - realm, - nonce, - uri, - qop, - nonce_count, - cnonce, - response, - opaque_kv - ); +int sret = openvpn_snprintf(buf, sizeof(buf), "Proxy-Authorization: Digest username=\"%s\", realm=\"%s\", nonce=\"%s\", uri=\"%s\", qop=%s, nc=%s, cnonce=\"%s\", response=\"%s\"%s", +username, +realm, +nonce, +uri, +qop, +nonce_count, +cnonce, +response, +opaque_kv +); +if (sret >= sizeof(buf)) +{ +goto error; +} msg(D_PROXY, "Send to HTTP proxy: '%s'", buf); if (!send_line_crlf(sd, buf)) { diff --git a/src/openvpn/socks.c b/src/openvpn/socks.c index d842666..b046910 100644 --- a/src/openvpn/socks.c +++ b/src/openvpn/socks.c @@ -109,8 +109,11 @@ "Authentication not possible."); goto cleanup; } -openvpn_snprintf(to_send, sizeof(to_send), "\x01%c%s%c%s", (int) strlen(creds.username), - creds.username, (int) strlen(creds.password), creds.password); +int sret = openvpn_snprintf(to_send, sizeof(to_send), "\x01%c%s%c%s", +(int) strlen(creds.username), creds.username, +(int) strlen(creds.password), creds.password); +ASSERT(sret <= sizeof(to_send)); + size = send(sd, to_send, strlen(to_send), MSG_NOSIGNAL); if (size != strlen(to_send)) diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index 4383e98..6f29c3d 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -2069,7 +2069,7 @@ #endif #ifndef OPENSSL_NO_EC -char groupname[256]; +char groupname[64]; if (is_ec) { size_t len; @@ -2130,7 +2130,7 @@ print_cert_details(X509 *cert, char *buf, size_t buflen) { EVP_PKEY *pkey = X509_get_pubkey(cert); -char pkeybuf[128] = { 0 }; +char pkeybuf[64] = { 0 }; print_pkey_details(pkey, pkeybuf, sizeof(pkeybuf)); char sig[128] = { 0 }; diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c index 975d31f..6ef1c7d 100644 --- a/src/openvpn/tls_crypt.c +++ b/src/openvpn/tls_crypt.c @@ -575,7 +575,7 @@ char metadata_type_str[4] = { 0 }; /* Max value: 255 */ openvpn_snprintf(metadata_type_str, sizeof(metadata_type_str), - "%i", metadata_type); + "%i", (uint8_t) metadata_type); struct env_set *es = env_set_create(NULL); setenv_str(es, "script_type", "tls-crypt-v2-verify"); setenv_str(es, "metadata_type", metadata_type_str); diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c index 452633c..d32223c 100644 --- a/src/openvpnserv/interactive.c +++ b/src/openvpnserv/interactive.c @@ -33,6 +33,7 @@ #include #include #include
[Openvpn-devel] [S] Change in openvpn[master]: Fix snprintf/swnprintf related compiler warnings
cron2 has uploaded a new patch set (#2) to the change originally created by plaisthos. ( http://gerrit.openvpn.net/c/openvpn/+/549?usp=email ) The following approvals got outdated and were removed: Code-Review+2 by flichtenheld Change subject: Fix snprintf/swnprintf related compiler warnings .. Fix snprintf/swnprintf related compiler warnings When openvpn_snprintf is replaced by snprintf the GCC/MSVC compiler will perform additional checks that the result is not truncated. This warning can be avoid by either explicitly checking the return value of snprintf (proxy) or ensuring that it is never truncated(tls crypt) Change-Id: If23988a05dd53a519c5e57f2aa3b2d10bd29df1d Signed-off-by: Arne Schwabe Acked-by: Frank Lichtenheld Message-Id: <20240326104101.531291-1-fr...@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28475.html Signed-off-by: Gert Doering --- M src/openvpn/proxy.c M src/openvpn/socks.c M src/openvpn/ssl_openssl.c M src/openvpn/tls_crypt.c M src/openvpnserv/interactive.c 5 files changed, 25 insertions(+), 17 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/49/549/2 diff --git a/src/openvpn/proxy.c b/src/openvpn/proxy.c index c904301..5c1cdcb 100644 --- a/src/openvpn/proxy.c +++ b/src/openvpn/proxy.c @@ -948,17 +948,21 @@ } /* send digest response */ -openvpn_snprintf(buf, sizeof(buf), "Proxy-Authorization: Digest username=\"%s\", realm=\"%s\", nonce=\"%s\", uri=\"%s\", qop=%s, nc=%s, cnonce=\"%s\", response=\"%s\"%s", - username, - realm, - nonce, - uri, - qop, - nonce_count, - cnonce, - response, - opaque_kv - ); +int sret = openvpn_snprintf(buf, sizeof(buf), "Proxy-Authorization: Digest username=\"%s\", realm=\"%s\", nonce=\"%s\", uri=\"%s\", qop=%s, nc=%s, cnonce=\"%s\", response=\"%s\"%s", +username, +realm, +nonce, +uri, +qop, +nonce_count, +cnonce, +response, +opaque_kv +); +if (sret >= sizeof(buf)) +{ +goto error; +} msg(D_PROXY, "Send to HTTP proxy: '%s'", buf); if (!send_line_crlf(sd, buf)) { diff --git a/src/openvpn/socks.c b/src/openvpn/socks.c index d842666..b046910 100644 --- a/src/openvpn/socks.c +++ b/src/openvpn/socks.c @@ -109,8 +109,11 @@ "Authentication not possible."); goto cleanup; } -openvpn_snprintf(to_send, sizeof(to_send), "\x01%c%s%c%s", (int) strlen(creds.username), - creds.username, (int) strlen(creds.password), creds.password); +int sret = openvpn_snprintf(to_send, sizeof(to_send), "\x01%c%s%c%s", +(int) strlen(creds.username), creds.username, +(int) strlen(creds.password), creds.password); +ASSERT(sret <= sizeof(to_send)); + size = send(sd, to_send, strlen(to_send), MSG_NOSIGNAL); if (size != strlen(to_send)) diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index 4383e98..6f29c3d 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -2069,7 +2069,7 @@ #endif #ifndef OPENSSL_NO_EC -char groupname[256]; +char groupname[64]; if (is_ec) { size_t len; @@ -2130,7 +2130,7 @@ print_cert_details(X509 *cert, char *buf, size_t buflen) { EVP_PKEY *pkey = X509_get_pubkey(cert); -char pkeybuf[128] = { 0 }; +char pkeybuf[64] = { 0 }; print_pkey_details(pkey, pkeybuf, sizeof(pkeybuf)); char sig[128] = { 0 }; diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c index 975d31f..6ef1c7d 100644 --- a/src/openvpn/tls_crypt.c +++ b/src/openvpn/tls_crypt.c @@ -575,7 +575,7 @@ char metadata_type_str[4] = { 0 }; /* Max value: 255 */ openvpn_snprintf(metadata_type_str, sizeof(metadata_type_str), - "%i", metadata_type); + "%i", (uint8_t) metadata_type); struct env_set *es = env_set_create(NULL); setenv_str(es, "script_type", "tls-crypt-v2-verify"); setenv_str(es, "metadata_type", metadata_type_str); diff --git
[Openvpn-devel] [S] Change in openvpn[master]: Fix snprintf/swnprintf related compiler warnings
Attention is currently required from: plaisthos. flichtenheld has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/549?usp=email ) Change subject: Fix snprintf/swnprintf related compiler warnings .. Patch Set 1: Code-Review+2 (2 comments) Commit Message: http://gerrit.openvpn.net/c/openvpn/+/549/comment/2124d3d9_d4975207 : PS1, Line 12: This warning can be avoid by either explicitly the return value missing "checking" Patchset: PS1: Except the error in commit message looks good -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/549?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: If23988a05dd53a519c5e57f2aa3b2d10bd29df1d Gerrit-Change-Number: 549 Gerrit-PatchSet: 1 Gerrit-Owner: plaisthos Gerrit-Reviewer: flichtenheld Gerrit-CC: openvpn-devel Gerrit-Attention: plaisthos Gerrit-Comment-Date: Tue, 26 Mar 2024 10:28:44 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [S] Change in openvpn[master]: Fix snprintf/swnprintf related compiler warnings
Attention is currently required from: flichtenheld. Hello flichtenheld, I'd like you to do a code review. Please visit http://gerrit.openvpn.net/c/openvpn/+/549?usp=email to review the following change. Change subject: Fix snprintf/swnprintf related compiler warnings .. Fix snprintf/swnprintf related compiler warnings When openvpn_snprintf is replaced by snprintf the GCC/MSVC compiler will perform additional checks that the result is not truncated. This warning can be avoid by either explicitly the return value of snprintf (proxy) or ensuring that it is never truncated(tls crypt) Change-Id: If23988a05dd53a519c5e57f2aa3b2d10bd29df1d Signed-off-by: Arne Schwabe --- M src/openvpn/proxy.c M src/openvpn/socks.c M src/openvpn/ssl_openssl.c M src/openvpn/tls_crypt.c M src/openvpnserv/interactive.c 5 files changed, 25 insertions(+), 17 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/49/549/1 diff --git a/src/openvpn/proxy.c b/src/openvpn/proxy.c index c904301..5c1cdcb 100644 --- a/src/openvpn/proxy.c +++ b/src/openvpn/proxy.c @@ -948,17 +948,21 @@ } /* send digest response */ -openvpn_snprintf(buf, sizeof(buf), "Proxy-Authorization: Digest username=\"%s\", realm=\"%s\", nonce=\"%s\", uri=\"%s\", qop=%s, nc=%s, cnonce=\"%s\", response=\"%s\"%s", - username, - realm, - nonce, - uri, - qop, - nonce_count, - cnonce, - response, - opaque_kv - ); +int sret = openvpn_snprintf(buf, sizeof(buf), "Proxy-Authorization: Digest username=\"%s\", realm=\"%s\", nonce=\"%s\", uri=\"%s\", qop=%s, nc=%s, cnonce=\"%s\", response=\"%s\"%s", +username, +realm, +nonce, +uri, +qop, +nonce_count, +cnonce, +response, +opaque_kv +); +if (sret >= sizeof(buf)) +{ +goto error; +} msg(D_PROXY, "Send to HTTP proxy: '%s'", buf); if (!send_line_crlf(sd, buf)) { diff --git a/src/openvpn/socks.c b/src/openvpn/socks.c index d842666..b046910 100644 --- a/src/openvpn/socks.c +++ b/src/openvpn/socks.c @@ -109,8 +109,11 @@ "Authentication not possible."); goto cleanup; } -openvpn_snprintf(to_send, sizeof(to_send), "\x01%c%s%c%s", (int) strlen(creds.username), - creds.username, (int) strlen(creds.password), creds.password); +int sret = openvpn_snprintf(to_send, sizeof(to_send), "\x01%c%s%c%s", +(int) strlen(creds.username), creds.username, +(int) strlen(creds.password), creds.password); +ASSERT(sret <= sizeof(to_send)); + size = send(sd, to_send, strlen(to_send), MSG_NOSIGNAL); if (size != strlen(to_send)) diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index 4383e98..6f29c3d 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -2069,7 +2069,7 @@ #endif #ifndef OPENSSL_NO_EC -char groupname[256]; +char groupname[64]; if (is_ec) { size_t len; @@ -2130,7 +2130,7 @@ print_cert_details(X509 *cert, char *buf, size_t buflen) { EVP_PKEY *pkey = X509_get_pubkey(cert); -char pkeybuf[128] = { 0 }; +char pkeybuf[64] = { 0 }; print_pkey_details(pkey, pkeybuf, sizeof(pkeybuf)); char sig[128] = { 0 }; diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c index 975d31f..6ef1c7d 100644 --- a/src/openvpn/tls_crypt.c +++ b/src/openvpn/tls_crypt.c @@ -575,7 +575,7 @@ char metadata_type_str[4] = { 0 }; /* Max value: 255 */ openvpn_snprintf(metadata_type_str, sizeof(metadata_type_str), - "%i", metadata_type); + "%i", (uint8_t) metadata_type); struct env_set *es = env_set_create(NULL); setenv_str(es, "script_type", "tls-crypt-v2-verify"); setenv_str(es, "metadata_type", metadata_type_str); diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c index 452633c..d32223c 100644 --- a/src/openvpnserv/interactive.c +++ b/src/openvpnserv/interactive.c @@ -33,6 +33,7 @@ #include #include #include +#include