From c560f95e7038daa3a1b5a08b69b85fb68d4eeef3 Mon Sep 17 00:00:00 2001 From: Steffan Karger Date: Tue, 15 Aug 2017 10:04:33 +0200 Subject: [PATCH] Fix bounds check in read_key() The bounds check in read_key() was performed after using the value, instead of before. If 'key-method 1' is used, this allowed an attacker to send a malformed packet to trigger a stack buffer overflow. Fix this by moving the input validation to before the writes. Note that 'key-method 1' has been replaced by 'key method 2' as the default in OpenVPN 2.0 (released on 2005-04-17), and explicitly deprecated in 2.4 and marked for removal in 2.5. This should limit the amount of users impacted by this issue. CVE: 2017-12166 Signed-off-by: Steffan Karger Acked-by: Gert Doering Acked-by: David Sommerseth Message-Id: <80690690-67ac-3320-1891-9fecedc6a1fa@fox-it.com> URL: https://www.mail-archive.com/search?l=mid&q=80690690-67ac-3320-1891-9fecedc6a1fa@fox-it.com Signed-off-by: David Sommerseth (cherry picked from commit fce34375295151f548a26c2d0eb30141e427c81a) --- crypto.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crypto.c b/crypto.c index d5c8c13c..58c35dfc 100644 --- a/crypto.c +++ b/crypto.c @@ -1442,13 +1442,14 @@ read_key (struct key *key, const struct key_type *kt, struct buffer *buf) if (!buf_read (buf, &hmac_length, 1)) goto read_err; + if (cipher_length != kt->cipher_length || hmac_length != kt->hmac_length) + goto key_len_err; + if (!buf_read (buf, key->cipher, cipher_length)) goto read_err; if (!buf_read (buf, key->hmac, hmac_length)) goto read_err; - if (cipher_length != kt->cipher_length || hmac_length != kt->hmac_length) - goto key_len_err; return 1;