Don't assert out on receiving too-large control packets (CVE-2017-7478)

Commit 358f513c changed the maximum size of accepted control channel
packets.  This was needed for crypto negotiation (which is needed for a
nice transition to a new default cipher), but exposed a DoS
vulnerability.  The vulnerability was found during the OpenVPN 2.4 code
audit by Quarkslab (commisioned by OSTIF).

To fix the issue, we should not ASSERT() on external input (in this case
the received packet size), but instead gracefully error out and drop the
invalid packet.

CVE: 2017-7478
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: <1494493257-8125-1-git-send-email-steffan.karger@fox-it.com>
URL: http://www.mail-archive.com/search?l=mid&q=1494493257-8125-1-git-send-email-steffan.karger@fox-it.com
Signed-off-by: David Sommerseth <davids@openvpn.net>

diff --git a/Changes.rst b/Changes.rst
index 183e9fa4ae24e182f6364cb419ce45f402166ba1..761302e4671656aca4af1df4049f4207f935773d 100644 (file)
--- a/Changes.rst
+++ b/Changes.rst
@@ -109,6 +109,12 @@ Version 2.3.15
 
 Security fixes
 --------------
+- Fix a pre-authentication denial-of-service attack on both clients and servers.
+  By sending a too-large control packet, OpenVPN 2.3.12 and newer can be forced
+  to hit an ASSERT() and stop the process.  If ``--tls-auth`` is used, only
+  attackers that have the ``--tls-auth`` key can mount an attack.
+  (OSTIF/Quarkslab audit finding 5.1, CVE-2017-7478)
+
 - Fix an authenticated remote DoS vulnerability that could be triggered by
   causing a packet id roll over.  An attack is rather inefficient; a peer
   would need to get us to send at least about 196 GB of data.

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index c52a0e4f16d6226805ec85b3e3c5309a2aee63b8..e704b73e0b23069f34dc72df029414c74e37a46e 100644 (file)
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -3225,7 +3225,12 @@ tls_pre_decrypt (struct tls_multi *multi,
                            /* Save incoming ciphertext packet to reliable buffer */
                            struct buffer *in = reliable_get_buf (ks->rec_reliable);
                            ASSERT (in);
-                           ASSERT (buf_copy (in, buf));
+                           if (!buf_copy (in, buf))
+                             {
+                               msg (D_MULTI_DROPPED,
+                                    "Incoming control channel packet too big, dropping.");
+                               goto error;
+                             }
                            reliable_mark_active_incoming (ks->rec_reliable, in, id, op);
                          }