diff mbox series

[Openvpn-devel,3/5] src/openvpn/auth_token.c: handle strdup errors

Message ID 20240708210912.566-4-chipitsine@gmail.com
State Changes Requested
Headers
Series handle strdup errors |

Commit Message

Илья Шипицин July 8, 2024, 9:08 p.m. UTC 
  Signed-off-by: Ilia Shipitsin <chipitsine@gmail.com>
---
 src/openvpn/auth_token.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Gert Doering Sept. 9, 2024, 7:30 a.m. UTC | #1 
Hi,

On Mon, Jul 08, 2024 at 11:08:20PM +0200, Ilia Shipitsin wrote:
>      multi->auth_token = strdup((char *)BPTR(&session_token));
> +    if (!multi->auth_token)
> +    {
> +        msg( M_FATAL, "Failed allocate memory for multi->auth_token");
> +    }

I do wonder if this is the right approach here.  For "openvpn itself"
we have the check_malloc_return() macro, which will purposely not call
msg() - as msg() does internal malloc()s, and if we cannot allocate
20 bytes for an auth_token, the chance that msg() will not succeed is
fairly high...

In plugins or unit tests, the infrastructure is different, but for 3/ and
4/, please resend with

      multi->auth_token = strdup((char *)BPTR(&session_token));
 +    check_malloc_return(multi->auth_token);

(etc)

gert
Илья Шипицин Sept. 9, 2024, 12:51 p.m. UTC | #2 
I'll have a look, thanks!

On Mon, Sep 9, 2024, 09:30 Gert Doering <gert@greenie.muc.de> wrote:

> Hi,
>
> On Mon, Jul 08, 2024 at 11:08:20PM +0200, Ilia Shipitsin wrote:
> >      multi->auth_token = strdup((char *)BPTR(&session_token));
> > +    if (!multi->auth_token)
> > +    {
> > +        msg( M_FATAL, "Failed allocate memory for multi->auth_token");
> > +    }
>
> I do wonder if this is the right approach here.  For "openvpn itself"
> we have the check_malloc_return() macro, which will purposely not call
> msg() - as msg() does internal malloc()s, and if we cannot allocate
> 20 bytes for an auth_token, the chance that msg() will not succeed is
> fairly high...
>
> In plugins or unit tests, the infrastructure is different, but for 3/ and
> 4/, please resend with
>
>       multi->auth_token = strdup((char *)BPTR(&session_token));
>  +    check_malloc_return(multi->auth_token);
>
> (etc)
>
> gert
> --
> "If was one thing all people took for granted, was conviction that if you
>  feed honest figures into a computer, honest figures come out. Never
> doubted
>  it myself till I met a computer with a sense of humor."
>                              Robert A. Heinlein, The Moon is a Harsh
> Mistress
>
> Gert Doering - Munich, Germany
> gert@greenie.muc.de
>
diff mbox series

Patch

diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c
index 6787ea7d..2278afe6 100644
--- a/src/openvpn/auth_token.c
+++ b/src/openvpn/auth_token.c
@@ -260,6 +260,10 @@  generate_auth_token(const struct user_pass *up, struct tls_multi *multi)
     /* free the auth-token if defined, we will replace it with a new one */
     free(multi->auth_token);
     multi->auth_token = strdup((char *)BPTR(&session_token));
+    if (!multi->auth_token)
+    {
+        msg( M_FATAL, "Failed allocate memory for multi->auth_token");
+    }
 
     dmsg(D_SHOW_KEYS, "Generated token for client: %s (%s)",
          multi->auth_token, up->username);
@@ -271,6 +275,10 @@  generate_auth_token(const struct user_pass *up, struct tls_multi *multi)
          * and timestamp in updates
          */
         multi->auth_token_initial = strdup(multi->auth_token);
+        if (!multi->auth_token_initial)
+        {
+            msg( M_FATAL, "Failed allocate memory for multi->auth_token_initial");
+        }
     }
 
     gc_free(&gc);