openssl: fix overflow check for long --tls-cipher option

The length check in tls_ctx_restrict_ciphers() did not check for overflow,
which could lead to a stack buffer overflow.

This has no real-world impact, because --tls-cipher can only be specified
by entities that are allowed to supply config settings.  Since those
entities can also change --script-security and call scripts and/or
plugins, these users already have code execution at the level of the
openvpn process.  In other words: the attacker would not gain any
capabilities.  Nevertheless, a nasty bug that we should fix.

This bug was discovered and reported to the OpenVPN security team by
Guido Vranken.

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1495461253-20111-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14716.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit e6bf7e033d063535a4414a4cf49c8f367ecdbb4f)

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index cda0ebb023b14303b63891dda1e17bd6f1546427..b459afe10ade1557852f658ee87baac0cbb193f7 100644 (file)
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -328,7 +328,8 @@ tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
        }
 
       /* Make sure new cipher name fits in cipher string */
-      if (((sizeof(openssl_ciphers)-1) - openssl_ciphers_len) < current_cipher_len)
+      if ((SIZE_MAX - openssl_ciphers_len) < current_cipher_len
+            || ((sizeof(openssl_ciphers)-1) < openssl_ciphers_len + current_cipher_len))
        {
          msg (M_FATAL,
              "Failed to set restricted TLS cipher list, too long (>%d).",