Fix potential divide-by-zero in rfbserver.c

[greneholt]

LibVNC found a potential divide-by-zero issue that appears to also be present in TurboVNC's version of Xvnc. This change applies the same fix used by LibVNC.

Original issue: https://github.com/LibVNC/libvncserver/issues/409 CVE: https://nvd.nist.gov/vuln/detail/CVE-2020-25708

[dcommander]

This is timely, because I just had to explain to a customer why the vulnerability in question (CVE-2020-25708) doesn't exist in TurboVNC:

Our implementation of rfbSendRectEncodingRaw(), which is similar to the one in LibVNCServer, does not check for 0 width and height arguments. However, it will never encounter those in TurboVNC, because rfbSendRectEncodingRaw() is only called internally for non-empty rectangles. This code:

https://github.com/TurboVNC/turbovnc/blob/ffdb57d943a3f5c442bedf2d82d1d90a55e2f483/unix/Xvnc/programs/Xserver/hw/vnc/rfbserver.c#L1873-L1877

ensures that, and even if that code didn't exist, this code:

https://github.com/TurboVNC/turbovnc/blob/ffdb57d943a3f5c442bedf2d82d1d90a55e2f483/unix/Xvnc/programs/Xserver/hw/vnc/rfbserver.c#L2123

would further ensure it. (The region computations are such that any 0-width or 0-height rectangles are excluded from the final region.) The other RFB encoding functions would likely experience problems with 0-width or 0-height rectangles as well, so the TurboVNC Server guarantees that those functions are never called with 0-width or 0-height rectangles. LibVNCServer is vulnerable because its region computations are not as fault-tolerant. Also, LibVNCServer isn't tied to an X server. The TurboVNC X server provides an additional layer of protection against this issue, since the requested region from the client (the attack surface, in this case) is always intersected with the changed region from the X server, and the latter can never be empty.

In the case of my customer, their firewall was flagging TurboVNC connections as LibVNCServer connections and assuming incorrectly that they were vulnerable to CVE-2020-25708. I'm still waiting for information regarding how or why the firewall did that. To the best of my understanding, a firewall could only analyze the protocol and see that it looks like TightVNC, but the majority of TightVNC-compatible solutions-- including the legacy TightVNC 1.3.x releases upon which TurboVNC was originally based, TurboVNC itself, and TigerVNC-- are not vulnerable to this attack. In fact, I verified that using the test case provided at

https://github.com/LibVNC/libvncserver/issues/409

(NOTE: You have to enable the "None" security type in order for the test case to trigger a framebuffer update. Otherwise, the server will reject the client before that ever happens.)

Although this patch is innocuous, at the moment I don't see any need for it.

[greneholt]

That's good to know, as this was the exact same issue that we are encountering at my company. The firewall was blocking TurboVNC connections as vulnerable to this CVE. Thank you for the thorough explanation!

[dcommander]

No problem. If there is anything I can do in TurboVNC to mitigate the false positive in the firewall, I'm happy to do that, but I can't imagine that applying this patch would make any difference in that regard. I'm guessing that the firewall has some sort of AI that detects what looks like a LibVNCServer connection and incorrectly assumes that anything that looks like that is vulnerable to CVE-2020-25708. If so, then that's a bad policy on the part of the firewall, since only one solution out of a handful of TightVNC-compatible solutions are vulnerable. Also, it's bad policy because the vulnerability exists only with Raw encoding, and the firewall is flagging connections that don't use Raw encoding. (TurboVNC will not, in fact, ever use Raw encoding unless you connect with a legacy VNC viewer and specifically request that encoding, so even if this vulnerability existed, it would never be encountered with normal use cases.) Thus, it's possible that the firewall is creating false negatives as well as false positives. Since Raw encoding is part of the core RFB protocol, any VNC server could potentially be vulnerable to this.