tls-crypt-v2: bail out if the client key is too small

The tls-crypt-v2 key should be at least 2 bytes long in order to read
the actual length. Bail out if the key is too short.

This looks like it could be abused to trigger a read of uninitialized
memory, but after close checking it won't:

  We read from BEND(), so this is defined for TCP since the minimum
  length there is 3 bytes (pkt len + opcode)

  For UDP we might read past the beginning of the packet but since they
  are buffers coming from the packet stack we have the headroom/tailroom,
  so might read some random data (but not out of bound!).

  So we copy some more or less random number into net_len/wkc_len but without
  actually reading from undefined memory.

  The next line will then almost definitively fail (buf_advance()).

While at it improve the error message a bit.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20220628094144.17471-1-a@unstable.cc>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24580.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit 462339a45089ef655faf02232d7d792def9b8afb)

diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index 663f5e169a5adb122d371a39cfff67f482234dd0..f2a97462d55b55dede2a80d7612e049410f83a01 100644 (file)
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -585,7 +585,8 @@ tls_crypt_v2_extract_client_key(struct buffer *buf,
 
     if (BLEN(&wrapped_client_key) < sizeof(net_len))
     {
-        msg(D_TLS_ERRORS, "failed to read length");
+        msg(D_TLS_ERRORS, "Can not read tls-crypt-v2 client key length");
+        return false;
     }
     memcpy(&net_len, BEND(&wrapped_client_key) - sizeof(net_len),
            sizeof(net_len));