Attention is currently required from: Arne Schwabe.
Hello Arne Schwabe,
I'd like you to do a code review.
Please visit
https://gerrit.openvpn.net/c/openvpn/+/2155?usp=email
to review the following change.
Change subject: Check the result of unchecked allocations in the server
......................................................................
Check the result of unchecked allocations in the server
Several allocations were used without a NULL check, so a failed
allocation crashed the server on a NULL pointer instead of stopping
with "Out of Memory" like the rest of the code:
- multi_init(): calloc() of m->instances; the server started and then
crashed in multi_assign_peer_id() on the first client
- generate_auth_token(), verify_user_pass(): strdup() of auth tokens;
generate_auth_token() then called strdup(NULL)
- override_locked_username(): strdup() of the override username; the
strlcpy() from locked_username then crashed (with --auth-gen-token)
- backend_x509_get_serial_hex(): malloc() of the serial buffer, which
BN_bn2binpad() then wrote to
Use string_alloc(..., NULL) instead of strdup(), and add
check_malloc_return() after the calloc() and malloc().
GitHub: closes OpenVPN/openvpn#1142
Assisted-by: Claude Code:claude-opus-5-5
Change-Id: I603321ae6893dbdb054deae5a9bff5e52cbb7429
Signed-off-by: Alexandre Laroche <[email protected]>
---
A src/openvpn/.dirstamp
M src/openvpn/auth_token.c
M src/openvpn/multi.c
M src/openvpn/ssl_verify.c
M src/openvpn/ssl_verify_openssl.c
5 files changed, 7 insertions(+), 5 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/55/2155/1
diff --git a/src/openvpn/.dirstamp b/src/openvpn/.dirstamp
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/src/openvpn/.dirstamp
diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c
index 84558af..10c656e 100644
--- a/src/openvpn/auth_token.c
+++ b/src/openvpn/auth_token.c
@@ -275,7 +275,7 @@
/* 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));
+ multi->auth_token = string_alloc((char *)BPTR(&session_token), NULL);
dmsg(D_SHOW_KEYS, "Generated token for client: %s (%s)",
multi->auth_token, up->username);
@@ -285,7 +285,7 @@
* Save the initial auth token to continue using the same session ID
* and timestamp in updates
*/
- multi->auth_token_initial = strdup(multi->auth_token);
+ multi->auth_token_initial = string_alloc(multi->auth_token, NULL);
}
gc_free(&gc);
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 3f514d6..662fc9d 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -395,6 +395,7 @@
m->max_clients = t->options.max_clients;
m->instances = calloc(m->max_clients, sizeof(struct multi_instance *));
+ check_malloc_return(m->instances);
m->top.c2.event_set = t->c2.event_set;
@@ -2653,7 +2654,7 @@
}
multi->locked_original_username = multi->locked_username;
- multi->locked_username = strdup(options->override_username);
+ multi->locked_username = string_alloc(options->override_username,
NULL);
/* Override also the common name if username should be set as common
* name */
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 0cdec1e..9b3f86d 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -1658,7 +1658,7 @@
* using the same session ID and initial timestamp in new tokens */
if (!multi->auth_token_initial && (ks->auth_token_state_flags &
AUTH_TOKEN_HMAC_OK))
{
- multi->auth_token_initial = strdup(up->password);
+ multi->auth_token_initial = string_alloc(up->password, NULL);
}
if (session->opt->auth_token_call_auth)
@@ -1777,7 +1777,7 @@
if (!multi->auth_token && (ks->auth_token_state_flags &
AUTH_TOKEN_HMAC_OK)
&& !(ks->auth_token_state_flags & AUTH_TOKEN_EXPIRED))
{
- multi->auth_token = strdup(up->password);
+ multi->auth_token = string_alloc(up->password, NULL);
}
/*
diff --git a/src/openvpn/ssl_verify_openssl.c b/src/openvpn/ssl_verify_openssl.c
index 0c36cf2..3f9f702 100644
--- a/src/openvpn/ssl_verify_openssl.c
+++ b/src/openvpn/ssl_verify_openssl.c
@@ -324,6 +324,7 @@
BIGNUM *bn_serial = ASN1_INTEGER_to_BN(asn1_i, NULL);
int len_serial = BN_num_bytes(bn_serial);
unsigned char *buf = malloc(len_serial);
+ check_malloc_return(buf);
BN_bn2binpad(bn_serial, buf, len_serial);
char *ret = format_hex_ex(buf, len_serial, 0, 1, ":", gc);
--
To view, visit https://gerrit.openvpn.net/c/openvpn/+/2155?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I603321ae6893dbdb054deae5a9bff5e52cbb7429
Gerrit-Change-Number: 2155
Gerrit-PatchSet: 1
Gerrit-Owner: Alexandre Laroche
Gerrit-Reviewer: Arne Schwabe <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: Arne Schwabe <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
