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.

---
 src/openvpn/ssl_openssl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index 645ccf5..73c07e3 100644
@@ -352,7 +352,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).",

2.7.4

ACK ("stared at code").

Have no ASAN setup here so replicating the original problem involved a bit of extra msg()'ing and then scratching my head, but I can confirm that the original math ended up with an integer underflow / unsigned, and the comarison not firing...

Your patch has been applied to the master, release/2.4 and release/2.3 branch.

commit e6bf7e033d063535a4414a4cf49c8f367ecdbb4f (master) commit b52c1ff43b23c3cf438fb99b807a7309d3229a56 (release/2.4) commit 236807276368514f8ab80155f440a1d377ab412b (release/2.3)

Author: Steffan Karger
Date:   Mon May 22 15:54:13 2017 +0200

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

     Signed-off-by: Steffan Karger <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg14716.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering