| Message ID | 20240708210912.566-4-chipitsine@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | handle strdup errors | expand |
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
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 --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);
Signed-off-by: Ilia Shipitsin <chipitsine@gmail.com> --- src/openvpn/auth_token.c | 8 ++++++++ 1 file changed, 8 insertions(+)