From 662481cd478578e112a53beed2cda86f2acc2994 Mon Sep 17 00:00:00 2001
From: DRC <information@virtualgl.org>
Date: Fri, 1 Nov 2024 14:34:45 -0400
Subject: [PATCH] Server/PeekExactTimeout(): Guard against inf. loop

If an ill-behaved RFB client preemptively sends fewer than 4 bytes of
the RFB version header before receiving the version header from the
server, then the first call to PeekExactTimeout() in webSocketsCheck()
may get into an infinite loop.  More specifically, when this happens,
the recv() call in PeekExactTimeout() returns n < len with errno set to
EAGAIN, PeekExactTimeout() falls through to the select() call, select()
returns 1 with errno unchanged, and the loop repeats indefinitely
because the client is waiting to receive the server's version header
before it sends additional data.  Such RFB clients are technically
incorrect but work accidentally with most RFB servers, because most RFB
servers don't peek the socket to check for a WebSocket header.  This
issue was known to affect certain versions of Apache Guacamole
sporadically.  However, the issue could reliably be triggered by running

    echo XX | netcat {TurboVNC_host} {TurboVNC_session_port}

Thus, it represents a DoS vulnerability.

The right thing to do seems to be to enforce the timeout ourselves
if recv() returns n < len with errno set to EAGAIN or EWOULDBLOCK, and
if select() subsequently returns 1.  In that case, select() has not
enforced the timeout for us.

Fixes #427
---
 ChangeLog.md                                |  6 ++++++
 unix/Xvnc/programs/Xserver/hw/vnc/sockets.c | 18 ++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/ChangeLog.md b/ChangeLog.md
index 0ec464915..f633da35f 100644
--- a/ChangeLog.md
+++ b/ChangeLog.md
@@ -29,6 +29,12 @@ file keyword or the server did not support `rsa-sha2-256`.
 TurboVNC Viewer when it attempted to receive a clipboard update from QEMU's VNC
 server.
 
+7. Fixed a denial-of-service (DoS) vulnerability in the TurboVNC Server,
+introduced by 3.0 beta1[20], that triggered an infinite loop in the server's
+automatic WebSocket detection code if an ill-behaved client sent 3 or fewer
+bytes of data immediately after connecting.  Certain versions of Apache
+Guacamole were known to trigger this issue sporadically.
+
 
 3.1.2
 =====
diff --git a/unix/Xvnc/programs/Xserver/hw/vnc/sockets.c b/unix/Xvnc/programs/Xserver/hw/vnc/sockets.c
index b9249766a..f52c7a3f3 100644
--- a/unix/Xvnc/programs/Xserver/hw/vnc/sockets.c
+++ b/unix/Xvnc/programs/Xserver/hw/vnc/sockets.c
@@ -472,8 +472,11 @@ int PeekExactTimeout(rfbClientPtr cl, char *buf, int len, int timeout)
   int n;
   fd_set readfds, exceptfds;
   struct timeval tv;
+  CARD32 start, now;
   int sock = cl->sock;
 
+  start = GetTimeInMillis();
+
   while (len > 0) {
     do {
 #if USETLS
@@ -519,6 +522,21 @@ int PeekExactTimeout(rfbClientPtr cl, char *buf, int len, int timeout)
         errno = ETIMEDOUT;
         return -1;
       }
+      /* If the client has sent less than len bytes and is waiting for the
+         server before sending more bytes, then we need to enforce the timeout
+         ourselves in order to prevent an infinite loop and subsequent denial
+         of service.  Otherwise recv() will keep returning n < len with errno
+         set to EAGAIN, and select() will keep returning 1, since recv() has
+         not removed any data from the queue.  We need to loop back in order to
+         give the client an opportunity to send more data, but we can't do that
+         forever. */
+      if (errno == EWOULDBLOCK || errno == EAGAIN) {
+        now = GetTimeInMillis();
+        if (now - start >= (CARD32)timeout) {
+          errno = ETIMEDOUT;
+          return -1;
+        }
+      }
     }
   }
   return 1;