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

Commit 3c1b19e0 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: David Sommerseth <davids@openvpn.net>
Message-Id: <1494358209-4568-2-git-send-email-steffan.karger@fox-it.com>
URL: http://www.mail-archive.com/search?l=mid&q=1494358209-4568-2-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 3dba7e0ef5144892e34e352104a9793981e34e7c..734ef7310ca6cefcc7e8ee9e01a7dec72be91147 100644 (file)
--- a/Changes.rst
+++ b/Changes.rst
@@ -327,3 +327,11 @@ Bugfixes
 --------
 - Fix memory leak introduced in 2.4.1: if --remote-cert-tls is used, we leaked
   some memory on each TLS (re)negotiation.
+
+Security
+--------
+- Fix a pre-authentication denial-of-service attack on both clients and servers.
+  By sending a too-large control packet, OpenVPN 2.4.0 or 2.4.1 can be forced
+  to hit an ASSERT() and stop the process.  If ``--tls-auth`` or ``--tls-crypt``
+  is used, only attackers that have the ``--tls-auth`` or ``--tls-crypt`` key
+  can mount an attack. (OSTIF/Quarkslab audit finding 5.1, CVE-2017-7478)

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 5f290fefdd4fade9ea8bc491f2d463d30e2218d0..21c9d6f7648437d228775b9dd693a0bfb8342d65 100644 (file)
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -3720,7 +3720,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);
                             }