[Openvpn-devel] auth_token.c: add NULL initialization

Message ID 20220107123550.188-1-lstipakov@gmail.com
State Accepted
Headers show
Series
  • [Openvpn-devel] auth_token.c: add NULL initialization
Related show

Commit Message

Lev Stipakov Jan. 7, 2022, 12:35 p.m.
From: Lev Stipakov <lev@openvpn.net>

This fixes

  error C4703: potentially uninitialized local pointer variable 'b64output' used

found by arm64 msvc compiler with SDL enabled.

Not sure why this is not triggered on x86/x64.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
---
 src/openvpn/auth_token.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Antonio Quartulli Jan. 7, 2022, 12:46 p.m. | #1
Hi,

On 07/01/2022 13:35, Lev Stipakov wrote:
> From: Lev Stipakov <lev@openvpn.net>
> 
> This fixes
> 
>    error C4703: potentially uninitialized local pointer variable 'b64output' used
> 
> found by arm64 msvc compiler with SDL enabled.
> 
> Not sure why this is not triggered on x86/x64.
> 
> Signed-off-by: Lev Stipakov <lev@openvpn.net>
> ---
>   src/openvpn/auth_token.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c
> index e8875464..ceae68f6 100644
> --- a/src/openvpn/auth_token.c
> +++ b/src/openvpn/auth_token.c
> @@ -259,7 +259,7 @@ generate_auth_token(const struct user_pass *up, struct tls_multi *multi)
>       ASSERT(buf_write(&token, &timestamp, sizeof(timestamp)));
>       ASSERT(buf_write(&token, hmac_output, sizeof(hmac_output)));
>   
> -    char *b64output;
> +    char *b64output = NULL;
>       openvpn_base64_encode(BPTR(&token), BLEN(&token), &b64output);

It's impossible to leave b64output uninitialized, but the compiler is 
probably not smart enough to understand it.

On the other hand, passing uninitialized variables by reference to a 
function (without checking its return value) and using them later is 
never a good pattern..

Acked-by: Antonio Quartulli <a@unstable.cc>
Arne Schwabe Jan. 7, 2022, 12:47 p.m. | #2
Am 07.01.22 um 13:35 schrieb Lev Stipakov:
> From: Lev Stipakov <lev@openvpn.net>
> 
> This fixes
> 
>    error C4703: potentially uninitialized local pointer variable 'b64output' used
> 
> found by arm64 msvc compiler with SDL enabled.
> 
> Not sure why this is not triggered on x86/x64.
> 
> Signed-off-by: Lev Stipakov <lev@openvpn.net>
> ---
>   src/openvpn/auth_token.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c
> index e8875464..ceae68f6 100644
> --- a/src/openvpn/auth_token.c
> +++ b/src/openvpn/auth_token.c
> @@ -259,7 +259,7 @@ generate_auth_token(const struct user_pass *up, struct tls_multi *multi)
>       ASSERT(buf_write(&token, &timestamp, sizeof(timestamp)));
>       ASSERT(buf_write(&token, hmac_output, sizeof(hmac_output)));
>   
> -    char *b64output;
> +    char *b64output = NULL;
>       openvpn_base64_encode(BPTR(&token), BLEN(&token), &b64output);
>   
>       struct buffer session_token = alloc_buf_gc(

Fine with me

Acked-By: Arne Schwabe <arne@rfc2549.org>
Gert Doering Jan. 10, 2022, 4:59 p.m. | #3
Your patch has been applied to the master and release/2.5 branch.

I have not done real testing, as it's quite obvious what it's doing :)

commit 4b6073b8253dafeb425361fb55bab0f2cdc5474f (master)
commit 813d1ee3c8b6a914599e4705eee3b8835d606e4b (release/2.5)
Author: Lev Stipakov
Date:   Fri Jan 7 14:35:50 2022 +0200

     auth_token.c: add NULL initialization

     Signed-off-by: Lev Stipakov <lev@openvpn.net>
     Acked-by: Antonio Quartulli <antonio@openvpn.net>
     Acked-by: Arne Schwabe <arne@rfc2549.org>
     Message-Id: <20220107123550.188-1-lstipakov@gmail.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23511.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c
index e8875464..ceae68f6 100644
--- a/src/openvpn/auth_token.c
+++ b/src/openvpn/auth_token.c
@@ -259,7 +259,7 @@  generate_auth_token(const struct user_pass *up, struct tls_multi *multi)
     ASSERT(buf_write(&token, &timestamp, sizeof(timestamp)));
     ASSERT(buf_write(&token, hmac_output, sizeof(hmac_output)));
 
-    char *b64output;
+    char *b64output = NULL;
     openvpn_base64_encode(BPTR(&token), BLEN(&token), &b64output);
 
     struct buffer session_token = alloc_buf_gc(