Skip to content

Commit

Permalink
Don't assert out on receiving too-large control packets (CVE-2017-7478)
Browse files Browse the repository at this point in the history
Commit 3c1b19e 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>
  • Loading branch information
syzzer authored and dsommers committed May 10, 2017
1 parent 5806f66 commit 5774cf4
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 1 deletion.
8 changes: 8 additions & 0 deletions Changes.rst
Expand Up @@ -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)
7 changes: 6 additions & 1 deletion src/openvpn/ssl.c
Expand Up @@ -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);
}

Expand Down

0 comments on commit 5774cf4

Please sign in to comment.