From: flichtenheld (C. Review) <ge...@op...> - 2025-10-21 18:25:08
|
Attention is currently required from: plaisthos. Hello plaisthos, I'd like you to do a code review. Please visit http://gerrit.openvpn.net/c/openvpn/+/1293?usp=email to review the following change. Change subject: push: Fix conversion issues related to timeout in send_auth_pending_messages ...................................................................... push: Fix conversion issues related to timeout in send_auth_pending_messages Add additional checking to make sure that the required casts are safe. Change-Id: Icc31b7fa0da86220df45552aecc15dc6c769cd54 Signed-off-by: Frank Lichtenheld <fr...@li...> --- M src/openvpn/push.c M src/openvpn/ssl_verify.c 2 files changed, 15 insertions(+), 19 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/93/1293/1 diff --git a/src/openvpn/push.c b/src/openvpn/push.c index 6f146fc..a6f979d 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -429,11 +429,6 @@ gc_free(&gc); } -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wconversion" -#endif - bool send_auth_pending_messages(struct tls_multi *tls_multi, struct tls_session *session, const char *extra, unsigned int timeout) @@ -449,7 +444,12 @@ /* Calculate the maximum timeout and subtract the time we already waited */ unsigned int max_timeout = max_uint(tls_multi->opt.renegotiate_seconds / 2, tls_multi->opt.handshake_window); - max_timeout = max_timeout - (now - ks->initial); + time_t time_elapsed = now - ks->initial; + if (time_elapsed < 0 || time_elapsed >= (time_t)max_timeout) + { + return false; + } + max_timeout -= (unsigned int)time_elapsed; timeout = min_uint(max_timeout, timeout); struct gc_arena gc = gc_new(); @@ -734,6 +734,11 @@ return true; } +#if defined(__GNUC__) || defined(__clang__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wconversion" +#endif + static bool send_push_options(struct context *c, struct buffer *buf, struct push_list *push_list, int safe_cap, bool *push_sent, bool *multi_push) diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c index 04ef27e..fe95621 100644 --- a/src/openvpn/ssl_verify.c +++ b/src/openvpn/ssl_verify.c @@ -874,11 +874,6 @@ return supported; } -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wconversion" -#endif - /** * Checks if the deferred state should also send auth pending * request to the client. Also removes the auth_pending control file @@ -915,8 +910,9 @@ buf_chomp(iv_buf); buf_chomp(extra_buf); + errno = 0; long timeout = strtol(BSTR(timeout_buf), NULL, 10); - if (timeout == 0) + if (timeout <= 0 || timeout > UINT_MAX || errno) { msg(M_WARN, "could not parse auth pending file timeout"); buffer_list_free(lines); @@ -933,14 +929,13 @@ pending_method); auth_set_client_reason(multi, buf); msg(M_INFO, - "Client does not supported auth pending method " - "'%s'", + "Client does not supported auth pending method '%s'", pending_method); ret = false; } else { - send_auth_pending_messages(multi, session, BSTR(extra_buf), timeout); + send_auth_pending_messages(multi, session, BSTR(extra_buf), (unsigned int)timeout); } } @@ -950,10 +945,6 @@ return ret; } -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic pop -#endif - /** * Removes auth_pending and auth_control files from file system * and key_state structure -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1293?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Icc31b7fa0da86220df45552aecc15dc6c769cd54 Gerrit-Change-Number: 1293 Gerrit-PatchSet: 1 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> |