Skip to content

Commit

Permalink
openssl: fix overflow check for long --tls-cipher option
Browse files Browse the repository at this point in the history
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 <[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]>
  • Loading branch information
syzzer authored and cron2 committed Jun 13, 2017
1 parent 534c8f2 commit e6bf7e0
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/openvpn/ssl_openssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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).",
Expand Down

0 comments on commit e6bf7e0

Please sign in to comment.