Browse code

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 <steffan.karger@fox-it.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Acked-by: David Sommerseth <davids@openvpn.net>
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 <davids@openvpn.net>

Steffan Karger authored on 2017/08/15 17:04:33
Showing 1 changed files
... ...
@@ -1666,6 +1666,11 @@ read_key(struct key *key, const struct key_type *kt, struct buffer *buf)
1666 1666
         goto read_err;
1667 1667
     }
1668 1668
 
1669
+    if (cipher_length != kt->cipher_length || hmac_length != kt->hmac_length)
1670
+    {
1671
+        goto key_len_err;
1672
+    }
1673
+
1669 1674
     if (!buf_read(buf, key->cipher, cipher_length))
1670 1675
     {
1671 1676
         goto read_err;
... ...
@@ -1675,11 +1680,6 @@ read_key(struct key *key, const struct key_type *kt, struct buffer *buf)
1675 1675
         goto read_err;
1676 1676
     }
1677 1677
 
1678
-    if (cipher_length != kt->cipher_length || hmac_length != kt->hmac_length)
1679
-    {
1680
-        goto key_len_err;
1681
-    }
1682
-
1683 1678
     return 1;
1684 1679
 
1685 1680
 read_err: