Skip to content

Commit

Permalink
Server/PeekExactTimeout(): Guard against inf. loop
Browse files Browse the repository at this point in the history
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
  • Loading branch information
dcommander committed Nov 1, 2024
1 parent 1549eaa commit 662481c
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 0 deletions.
6 changes: 6 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
=====
Expand Down
18 changes: 18 additions & 0 deletions unix/Xvnc/programs/Xserver/hw/vnc/sockets.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 662481c

Please sign in to comment.