[Openvpn-devel] [XS] Change in openvpn[master]: Use snprintf instead of sprintf for get_ssl_library_version

[Openvpn-devel] [XS] Change in openvpn[master]: Use snprintf instead of sprintf for get_ssl_library_version

[plaisthos (Code Review)]

[-- Attachment #1: Type: text/plain, Size: 2043 bytes --]

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/+/545?usp=email

to review the following change.


Change subject: Use snprintf instead of sprintf for get_ssl_library_version
......................................................................

Use snprintf instead of sprintf for get_ssl_library_version

This is avoid a warning/error (when using -Werror) under current macOS
of sprintf:

   __deprecated_msg("This function is provided for compatibility
   reasons only.  Due to security concerns inherent in the design
   of sprintf(3), it is highly recommended that you use snprintf(3)
   instead.")

Change-Id: I3c6fd36eb9daee9244d6dc6d9f22de1c5cf9d039
---
M src/openvpn/ssl_mbedtls.c
1 file changed, 1 insertion(+), 1 deletion(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/45/545/1

diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index b44ddd5..0730d25 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -1614,7 +1614,7 @@
 {
     static char mbedtls_version[30];
     unsigned int pv = mbedtls_version_get_number();
-    sprintf( mbedtls_version, "mbed TLS %d.%d.%d",
+    snprintf(mbedtls_version, sizeof(mbedtls_version), "mbed TLS %d.%d.%d",
              (pv>>24)&0xff, (pv>>16)&0xff, (pv>>8)&0xff );
     return mbedtls_version;
 }

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/545?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: I3c6fd36eb9daee9244d6dc6d9f22de1c5cf9d039
Gerrit-Change-Number: 545
Gerrit-PatchSet: 1
Gerrit-Owner: plaisthos <arne-openvpn@...1227...>
Gerrit-Reviewer: flichtenheld <frank@...2641...>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: flichtenheld <frank@...2641...>
Gerrit-MessageType: newchange

[-- Attachment #2: Type: text/html, Size: 3190 bytes --]

[Openvpn-devel] [XS] Change in openvpn[master]: Use snprintf instead of sprintf for get_ssl_library_version

[flichtenheld (Code Review)]

[-- Attachment #1: Type: text/plain, Size: 1264 bytes --]

Attention is currently required from: plaisthos.

flichtenheld has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/545?usp=email )

Change subject: Use snprintf instead of sprintf for get_ssl_library_version
......................................................................


Patch Set 1:

(1 comment)

File src/openvpn/ssl_mbedtls.c:

http://gerrit.openvpn.net/c/openvpn/+/545/comment/655ebb20_a5900f83 :
PS1, Line 1617:     snprintf(mbedtls_version, sizeof(mbedtls_version), "mbed TLS %d.%d.%d",
Do we want to use openvpn_snprintf here?



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/545?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: I3c6fd36eb9daee9244d6dc6d9f22de1c5cf9d039
Gerrit-Change-Number: 545
Gerrit-PatchSet: 1
Gerrit-Owner: plaisthos <arne-openvpn@...1227...>
Gerrit-Reviewer: flichtenheld <frank@...2641...>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-openvpn@...1227...>
Gerrit-Comment-Date: Fri, 22 Mar 2024 17:40:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[-- Attachment #2: Type: text/html, Size: 2247 bytes --]

[Openvpn-devel] [XS] Change in openvpn[master]: Use snprintf instead of sprintf for get_ssl_library_version

[plaisthos (Code Review)]

[-- Attachment #1: Type: text/plain, Size: 1580 bytes --]

Attention is currently required from: flichtenheld.

plaisthos has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/545?usp=email )

Change subject: Use snprintf instead of sprintf for get_ssl_library_version
......................................................................


Patch Set 1:

(1 comment)

File src/openvpn/ssl_mbedtls.c:

http://gerrit.openvpn.net/c/openvpn/+/545/comment/7c92da42_95f3de4f :
PS1, Line 1617:     snprintf(mbedtls_version, sizeof(mbedtls_version), "mbed TLS %d.%d.%d",
> Do we want to use openvpn_snprintf here?
I think we probably want to get rid of openvpn_snprintf instead. I will prepare a patch to remove it and add unit tests to check if it behaves correctly according to https://en.cppreference.com/w/c/io/snprintf it actually guarantees the behaviour that we expect.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/545?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: I3c6fd36eb9daee9244d6dc6d9f22de1c5cf9d039
Gerrit-Change-Number: 545
Gerrit-PatchSet: 1
Gerrit-Owner: plaisthos <arne-openvpn@...1227...>
Gerrit-Reviewer: flichtenheld <frank@...2641...>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: flichtenheld <frank@...2641...>
Gerrit-Comment-Date: Fri, 22 Mar 2024 18:39:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: flichtenheld <frank@...2641...>
Gerrit-MessageType: comment

[-- Attachment #2: Type: text/html, Size: 2709 bytes --]

[Openvpn-devel] [XS] Change in openvpn[master]: Use snprintf instead of sprintf for get_ssl_library_version

[flichtenheld (Code Review)]

[-- Attachment #1: Type: text/plain, Size: 1437 bytes --]

Attention is currently required from: plaisthos.

flichtenheld has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/545?usp=email )

Change subject: Use snprintf instead of sprintf for get_ssl_library_version
......................................................................


Patch Set 1: Code-Review+2

(1 comment)

File src/openvpn/ssl_mbedtls.c:

http://gerrit.openvpn.net/c/openvpn/+/545/comment/d36b1b05_0b5bc893 :
PS1, Line 1617:     snprintf(mbedtls_version, sizeof(mbedtls_version), "mbed TLS %d.%d.%d",
> I think we probably want to get rid of openvpn_snprintf instead. […]
Acknowledged



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/545?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: I3c6fd36eb9daee9244d6dc6d9f22de1c5cf9d039
Gerrit-Change-Number: 545
Gerrit-PatchSet: 1
Gerrit-Owner: plaisthos <arne-openvpn@...1227...>
Gerrit-Reviewer: flichtenheld <frank@...2641...>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-openvpn@...1227...>
Gerrit-Comment-Date: Mon, 25 Mar 2024 12:42:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: plaisthos <arne-openvpn@...1227...>
Comment-In-Reply-To: flichtenheld <frank@...2641...>
Gerrit-MessageType: comment

[-- Attachment #2: Type: text/html, Size: 2744 bytes --]

[Openvpn-devel] [PATCH v1] Use snprintf instead of sprintf for get_ssl_library_version

[Gert Doering]

From: Arne Schwabe <arne@...1227...>

This is avoid a warning/error (when using -Werror) under current macOS
of sprintf:

   __deprecated_msg("This function is provided for compatibility
   reasons only.  Due to security concerns inherent in the design
   of sprintf(3), it is highly recommended that you use snprintf(3)
   instead.")

Change-Id: I3c6fd36eb9daee9244d6dc6d9f22de1c5cf9d039
Signed-off-by: Arne Schwabe <arne-openvpn@...1227...>
Acked-by: Frank Lichtenheld <frank@...2641...>
---

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/+/545
This mail reflects revision 1 of this Change.

Signed-off-by line for the author was added as per our policy.

Acked-by according to Gerrit (reflected above):
Frank Lichtenheld <frank@...2641...>

        
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index b44ddd5..0730d25 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -1614,7 +1614,7 @@
 {
     static char mbedtls_version[30];
     unsigned int pv = mbedtls_version_get_number();
-    sprintf( mbedtls_version, "mbed TLS %d.%d.%d",
+    snprintf(mbedtls_version, sizeof(mbedtls_version), "mbed TLS %d.%d.%d",
              (pv>>24)&0xff, (pv>>16)&0xff, (pv>>8)&0xff );
     return mbedtls_version;
 }

[Openvpn-devel] [PATCH applied] Re: Use snprintf instead of sprintf for get_ssl_library_version

[Gert Doering]

Your patch has been applied to the master and release/2.6 branch
(because this is good behaviour, even if we know there can not
be an overrun - today).

Tested on...

Linux, with "library versions: mbed TLS 2.28.7, LZO 2.10"
FreeBSD, with "library versions: mbed TLS 3.5.1, LZO 2.10"

commit 6a60d1bef424088df55f4d07efd45ce080fc7132 (master)
commit 11ca69cfac1c6d3ed34652650688a4b3c99573b0 (release/2.6)
Author: Arne Schwabe
Date:   Mon Mar 25 13:50:52 2024 +0100

     Use snprintf instead of sprintf for get_ssl_library_version

     Signed-off-by: Arne Schwabe <arne-openvpn@...1227...>
     Acked-by: Frank Lichtenheld <frank@...2641...>
     Message-Id: <20240325125052.14135-1-gert@...1296...>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28458.html
     Signed-off-by: Gert Doering <gert@...1296...>


--
kind regards,

Gert Doering

[Openvpn-devel] [XS] Change in openvpn[master]: Use snprintf instead of sprintf for get_ssl_library_version

[cron2 (Code Review)]

[-- Attachment #1: Type: text/plain, Size: 2293 bytes --]

cron2 has uploaded a new patch set (#2) to the change originally created by plaisthos. ( http://gerrit.openvpn.net/c/openvpn/+/545?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by flichtenheld


Change subject: Use snprintf instead of sprintf for get_ssl_library_version
......................................................................

Use snprintf instead of sprintf for get_ssl_library_version

This is avoid a warning/error (when using -Werror) under current macOS
of sprintf:

   __deprecated_msg("This function is provided for compatibility
   reasons only.  Due to security concerns inherent in the design
   of sprintf(3), it is highly recommended that you use snprintf(3)
   instead.")

Change-Id: I3c6fd36eb9daee9244d6dc6d9f22de1c5cf9d039
Signed-off-by: Arne Schwabe <arne-openvpn@...1227...>
Acked-by: Frank Lichtenheld <frank@...2641...>
Message-Id: <20240325125052.14135-1-gert@...1296...>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28458.html
Signed-off-by: Gert Doering <gert@...1296...>
---
M src/openvpn/ssl_mbedtls.c
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/45/545/2

diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index b44ddd5..0730d25 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -1614,7 +1614,7 @@
 {
     static char mbedtls_version[30];
     unsigned int pv = mbedtls_version_get_number();
-    sprintf( mbedtls_version, "mbed TLS %d.%d.%d",
+    snprintf(mbedtls_version, sizeof(mbedtls_version), "mbed TLS %d.%d.%d",
              (pv>>24)&0xff, (pv>>16)&0xff, (pv>>8)&0xff );
     return mbedtls_version;
 }

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/545?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: I3c6fd36eb9daee9244d6dc6d9f22de1c5cf9d039
Gerrit-Change-Number: 545
Gerrit-PatchSet: 2
Gerrit-Owner: plaisthos <arne-openvpn@...1227...>
Gerrit-Reviewer: flichtenheld <frank@...2641...>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-MessageType: newpatchset

[-- Attachment #2: Type: text/html, Size: 3465 bytes --]

[Openvpn-devel] [XS] Change in openvpn[master]: Use snprintf instead of sprintf for get_ssl_library_version

[cron2 (Code Review)]

[-- Attachment #1: Type: text/plain, Size: 2072 bytes --]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/545?usp=email )

Change subject: Use snprintf instead of sprintf for get_ssl_library_version
......................................................................

Use snprintf instead of sprintf for get_ssl_library_version

This is avoid a warning/error (when using -Werror) under current macOS
of sprintf:

   __deprecated_msg("This function is provided for compatibility
   reasons only.  Due to security concerns inherent in the design
   of sprintf(3), it is highly recommended that you use snprintf(3)
   instead.")

Change-Id: I3c6fd36eb9daee9244d6dc6d9f22de1c5cf9d039
Signed-off-by: Arne Schwabe <arne-openvpn@...1227...>
Acked-by: Frank Lichtenheld <frank@...2641...>
Message-Id: <20240325125052.14135-1-gert@...1296...>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28458.html
Signed-off-by: Gert Doering <gert@...1296...>
---
M src/openvpn/ssl_mbedtls.c
1 file changed, 1 insertion(+), 1 deletion(-)




diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index b44ddd5..0730d25 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -1614,7 +1614,7 @@
 {
     static char mbedtls_version[30];
     unsigned int pv = mbedtls_version_get_number();
-    sprintf( mbedtls_version, "mbed TLS %d.%d.%d",
+    snprintf(mbedtls_version, sizeof(mbedtls_version), "mbed TLS %d.%d.%d",
              (pv>>24)&0xff, (pv>>16)&0xff, (pv>>8)&0xff );
     return mbedtls_version;
 }

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/545?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: I3c6fd36eb9daee9244d6dc6d9f22de1c5cf9d039
Gerrit-Change-Number: 545
Gerrit-PatchSet: 2
Gerrit-Owner: plaisthos <arne-openvpn@...1227...>
Gerrit-Reviewer: flichtenheld <frank@...2641...>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-MessageType: merged

[-- Attachment #2: Type: text/html, Size: 3219 bytes --]