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...>
|
