[gtk-vnc] Fix bounds checking for RRE, hextile & copyrect encodings

[Daniel P. Berrange <dberrange src gnome org>]

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>

 cfg.mk                  |    2 +-
 src/Makefile.am         |    8 +
 src/vncconnection.c     |   41 +++--
 src/vncconnectiontest.c |  462 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 496 insertions(+), 17 deletions(-)
---
diff --git a/cfg.mk b/cfg.mk
index 524a330..cd4a264 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -109,7 +109,7 @@ sc_copyright_format:
 prev_version_file = /dev/null
 
 
-exclude_file_name_regexp--sc_bindtextdomain = ^examples/
+exclude_file_name_regexp--sc_bindtextdomain = ^examples/|src/.*test.c
 
 exclude_file_name_regexp--sc_preprocessor_indentation = ^*/*.[ch]
 
diff --git a/src/Makefile.am b/src/Makefile.am
index 996f5e0..f7c1d9d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1,6 +1,10 @@
 
 EXTRA_DIST = libgvnc_sym.version libgvncpulse_sym.version libgtk-vnc_sym.version vncmarshal.txt
 
+TESTS = vncconnectiontest
+
+noinst_PROGRAMS = $(TESTS)
+
 lib_LTLIBRARIES = libgvnc-1.0.la
 
 BUILT_SOURCES =
@@ -312,6 +316,10 @@ BUILT_SOURCES += $(MARSHAL_FILES) $(ENUM_FILES)
 
 CLEANFILES = $(MARSHAL_FILES) $(ENUM_FILES)
 
+vncconnectiontest_SOURCES = vncconnectiontest.c
+vncconnectiontest_CFLAGS = $(GOBJECT_CFLAGS)
+vncconnectiontest_LDADD = libgvnc-1.0.la
+
 if WITH_PYTHON
 pyexec_LTLIBRARIES = gtkvnc.la
 
diff --git a/src/vncconnection.c b/src/vncconnection.c
index 95a615d..8af7283 100644
--- a/src/vncconnection.c
+++ b/src/vncconnection.c
@@ -2168,6 +2168,21 @@ static vnc_connection_tight_sum_pixel_func *vnc_connection_tight_sum_pixel_table
     (vnc_connection_tight_sum_pixel_func *)vnc_connection_tight_sum_pixel_32x32,
 };
 
+static gboolean vnc_connection_validate_boundary(VncConnection *conn,
+                                                 guint16 x, guint16 y,
+                                                 guint16 width, guint16 height)
+{
+    VncConnectionPrivate *priv = conn->priv;
+
+    if ((x + width) > priv->width || (y + height) > priv->height) {
+        vnc_connection_set_error(conn, "Framebuffer update %dx%d at %d,%d "
+                                 "outside boundary %dx%d",
+                                 width, height, x, y, priv->width, priv->height);
+    }
+
+    return !vnc_connection_has_error(conn);
+}
+
 
 static void vnc_connection_raw_update(VncConnection *conn,
                                       guint16 x, guint16 y,
@@ -2215,6 +2230,9 @@ static void vnc_connection_copyrect_update(VncConnection *conn,
     src_x = vnc_connection_read_u16(conn);
     src_y = vnc_connection_read_u16(conn);
 
+    if (!vnc_connection_validate_boundary(conn, src_x, src_y, width, height))
+        return;
+
     vnc_framebuffer_copyrect(priv->fb,
                              src_x, src_y,
                              dst_x, dst_y,
@@ -2257,6 +2275,10 @@ static void vnc_connection_hextile_rect(VncConnection *conn,
                 xy = vnc_connection_read_u8(conn);
                 wh = vnc_connection_read_u8(conn);
 
+                if (!vnc_connection_validate_boundary(conn, x + nibhi(xy), y + niblo(xy),
+                                                      nibhi(wh) + 1, niblo(wh) + 1))
+                    return;
+
                 vnc_framebuffer_fill(priv->fb, fg,
                                      x + nibhi(xy), y + niblo(xy),
                                      nibhi(wh) + 1, niblo(wh) + 1);
@@ -2313,6 +2335,9 @@ static void vnc_connection_rre_update(VncConnection *conn,
         sub_w = vnc_connection_read_u16(conn);
         sub_h = vnc_connection_read_u16(conn);
 
+        if (!vnc_connection_validate_boundary(conn, x + sub_x, y + sub_y, sub_w, sub_h))
+            break;
+
         vnc_framebuffer_fill(priv->fb, fg,
                              x + sub_x, y + sub_y, sub_w, sub_h);
     }
@@ -3079,22 +3104,6 @@ static void vnc_connection_ext_key_event(VncConnection *conn)
 }
 
 
-static gboolean vnc_connection_validate_boundary(VncConnection *conn,
-                                                 guint16 x, guint16 y,
-                                                 guint16 width, guint16 height)
-{
-    VncConnectionPrivate *priv = conn->priv;
-
-    if ((x + width) > priv->width || (y + height) > priv->height) {
-        vnc_connection_set_error(conn, "Framebuffer update %dx%d at %d,%d "
-                                 "outside boundary %dx%d",
-                                 width, height, x, y, priv->width, priv->height);
-    }
-
-    return !vnc_connection_has_error(conn);
-}
-
-
 static gboolean vnc_connection_framebuffer_update(VncConnection *conn, gint32 etype,
                                                   guint16 x, guint16 y,
                                                   guint16 width, guint16 height)
diff --git a/src/vncconnectiontest.c b/src/vncconnectiontest.c
new file mode 100644
index 0000000..7ae7265
--- /dev/null
+++ b/src/vncconnectiontest.c
@@ -0,0 +1,462 @@
+/*
+ * GTK VNC Widget
+ *
+ * Copyright (C) 2006  Anthony Liguori <anthony codemonkey ws>
+ * Copyright (C) 2009-2010 Daniel P. Berrange <dan berrange com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.0 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
+ */
+
+#include <config.h>
+
+#include <string.h>
+#include <stdlib.h>
+
+#include "vncconnection.h"
+#include "vncbaseframebuffer.h"
+
+static gboolean debug;
+
+#if GLIB_CHECK_VERSION(2, 22, 0)
+
+struct GVncTest {
+    GMutex lock;
+    GMutex clock;
+    GCond cond;
+    int port;
+    VncConnection *conn;
+    GMainLoop *loop;
+    gboolean connected;
+    gboolean quit;
+    char *error;
+
+    char *pixels;
+
+    void (*test_func)(GInputStream *, GOutputStream *);
+};
+
+
+static void test_send_bytes(GOutputStream *os, const guint8 *str, gsize len)
+{
+    g_assert(g_output_stream_write_all(os, str, len, NULL, NULL, NULL));
+}
+
+static void test_send_u8(GOutputStream *os, guint8 v)
+{
+    g_assert(g_output_stream_write_all(os, &v, 1, NULL, NULL, NULL));
+}
+
+static void test_send_u16(GOutputStream *os, guint16 v)
+{
+    v = GUINT16_TO_BE(v);
+    g_assert(g_output_stream_write_all(os, &v, 2, NULL, NULL, NULL));
+}
+
+static void test_send_u32(GOutputStream *os, guint32 v)
+{
+    v = GUINT32_TO_BE(v);
+    g_assert(g_output_stream_write_all(os, &v, 4, NULL, NULL, NULL));
+}
+
+static void test_send_s32(GOutputStream *os, gint32 v)
+{
+    v = GINT32_TO_BE(v);
+    g_assert(g_output_stream_write_all(os, &v, 4, NULL, NULL, NULL));
+}
+
+static void test_recv_bytes(GInputStream *is, guint8 *str, gsize len)
+{
+    g_assert(g_input_stream_read_all(is, str, len, NULL, NULL, NULL));
+}
+
+static void test_recv_u8(GInputStream *is, guint8 v)
+{
+    guint8 e;
+    g_assert(g_input_stream_read_all(is, &e, 1, NULL, NULL, NULL));
+    g_assert(e == v);
+}
+
+
+static gpointer test_helper_server(gpointer opaque)
+{
+    struct GVncTest *data = opaque;
+    GSocketListener *server;
+    GSocketConnection *client;
+    GIOStream *ios;
+    GInputStream *is;
+    GOutputStream *os;
+
+    server = g_socket_listener_new();
+
+    data->port = g_socket_listener_add_any_inet_port(server, NULL, NULL);
+    g_mutex_unlock(&data->lock);
+
+    client = g_socket_listener_accept(server, NULL, NULL, NULL);
+
+    ios = G_IO_STREAM(client);
+    is = g_io_stream_get_input_stream(ios);
+    os = g_io_stream_get_output_stream(ios);
+
+    guint8 greeting[] = {
+        'R', 'F', 'B', ' ',
+        '0', '0', '3', '.',
+        '0', '0', '8', '\n',
+    };
+
+    /* Greeting */
+    test_send_bytes(os, greeting, G_N_ELEMENTS(greeting));
+    test_recv_bytes(is, greeting, G_N_ELEMENTS(greeting));
+
+    /* N auth */
+    test_send_u8(os, 1);
+    /* auth == none */
+    test_send_u8(os, 1);
+    test_recv_u8(is, 1);
+
+    /* auth result */
+    test_send_u32(os, 0);
+
+    data->test_func(is, os);
+
+    g_mutex_lock(&data->clock);
+    while (!data->quit) {
+        g_cond_wait(&data->cond, &data->clock);
+    }
+
+    g_object_unref(client);
+}
+
+static void test_helper_desktop_resize(VncConnection *conn,
+                                       int width, int height,
+                                       gpointer opaque)
+{
+    struct GVncTest *test = opaque;
+    const VncPixelFormat *remoteFormat;
+    VncPixelFormat localFormat = {
+        .bits_per_pixel = 32,
+        .depth = 32,
+        .byte_order = G_BYTE_ORDER,
+        .true_color_flag = TRUE,
+        .red_max = 255,
+        .green_max = 255,
+        .blue_max = 255,
+        .red_shift = 0,
+        .green_shift = 8,
+        .blue_shift = 16,
+    };
+    VncBaseFramebuffer *fb;
+
+
+    VNC_DEBUG("Resize %dx%d", width, height);
+    remoteFormat = vnc_connection_get_pixel_format(conn);
+
+    /* We'll fix our local copy as rgb888 */
+    test->pixels = g_new0(char, width * height * 4);
+
+    fb = vnc_base_framebuffer_new(test->pixels, width, height, width * 4,
+                                  remoteFormat,
+                                  &localFormat);
+
+    vnc_connection_set_framebuffer(conn, VNC_FRAMEBUFFER(fb));
+
+    g_object_unref(fb);
+}
+
+
+static void test_helper_initialized(VncConnection *conn,
+                                    gpointer opaque)
+{
+    struct GVncTest *test = opaque;
+    gint32 encodings[] = {  VNC_CONNECTION_ENCODING_DESKTOP_RESIZE,
+                            VNC_CONNECTION_ENCODING_ZRLE,
+                            VNC_CONNECTION_ENCODING_HEXTILE,
+                            VNC_CONNECTION_ENCODING_RRE,
+                            VNC_CONNECTION_ENCODING_COPY_RECT,
+                            VNC_CONNECTION_ENCODING_RAW };
+    gint32 *encodingsp;
+    int n_encodings;
+
+    test_helper_desktop_resize(conn,
+                               vnc_connection_get_width(conn),
+                               vnc_connection_get_height(conn),
+                               test);
+
+    encodingsp = encodings;
+    n_encodings = G_N_ELEMENTS(encodings);
+
+    VNC_DEBUG("Sending %d encodings", n_encodings);
+    if (!vnc_connection_set_encodings(conn, n_encodings, encodingsp))
+        goto error;
+
+    VNC_DEBUG("Requesting first framebuffer update");
+    if (!vnc_connection_framebuffer_update_request(test->conn,
+                                                   0, 0, 0,
+                                                   vnc_connection_get_width(test->conn),
+                                                   vnc_connection_get_height(test->conn)))
+        vnc_connection_shutdown(test->conn);
+
+    test->connected = TRUE;
+    return;
+
+ error:
+    vnc_connection_shutdown(conn);
+}
+
+static void test_helper_auth_choose_type(VncConnection *conn,
+                                         GValueArray *types G_GNUC_UNUSED,
+                                         gpointer opaque G_GNUC_UNUSED)
+{
+    vnc_connection_set_auth_type(conn, VNC_CONNECTION_AUTH_NONE);
+}
+
+
+static void test_helper_disconnected(VncConnection *conn G_GNUC_UNUSED,
+                                     gpointer opaque)
+{
+    struct GVncTest *test = opaque;
+    g_main_quit(test->loop);
+}
+
+static void test_helper_error(VncConnection *conn,
+                              const char *str,
+                              gpointer opaque)
+{
+    struct GVncTest *test = opaque;
+    test->error = g_strdup(str);
+}
+
+static void test_common_bounds_server(GInputStream *is, GOutputStream *os)
+{
+    /* Frame buffer width / height */
+    test_send_u16(os, 100);
+    test_send_u16(os, 100);
+
+    /* BPP, depth, endian, true color */
+    test_send_u8(os, 32);
+    test_send_u8(os, 8);
+    test_send_u8(os, 1);
+    test_send_u8(os, 1);
+
+    /* RGB max + shift*/
+    test_send_u16(os, 255);
+    test_send_u16(os, 255);
+    test_send_u16(os, 255);
+    test_send_u8(os, 0);
+    test_send_u8(os, 8);
+    test_send_u8(os, 16);
+
+    guint8 pad[3] = {0};
+    test_send_bytes(os, pad, G_N_ELEMENTS(pad));
+
+    /* name */
+    guint8 name[] = { 'T', 'e', 's', 't' };
+    test_send_u32(os, G_N_ELEMENTS(name));
+    test_send_bytes(os, name, G_N_ELEMENTS(name));
+}
+
+static void test_rre_bounds_server(GInputStream *is, GOutputStream *os)
+{
+    test_common_bounds_server(is, os);
+
+    /* Message type & pad */
+    test_send_u8(os, 0);
+    test_send_u8(os, 0);
+
+    /* num rect */
+    test_send_u16(os, 1);
+    /* x, y, w, h */
+    test_send_u16(os, 90);
+    test_send_u16(os, 90);
+    test_send_u16(os, 10);
+    test_send_u16(os, 10);
+
+    /* encoding=rre */
+    test_send_s32(os, 2);
+
+    /* num rect */
+    test_send_u32(os, 1);
+
+    /* bg pix, fg pix */
+    test_send_u32(os, 0x41414141);
+    test_send_u32(os, 0x42424242);
+
+    /* x, y, w, h */
+    test_send_u16(os, 10);
+    test_send_u16(os, 10000);
+    test_send_u16(os, 1);
+    test_send_u16(os, 1);
+}
+
+
+static void test_hextile_bounds_server(GInputStream *is, GOutputStream *os)
+{
+    test_common_bounds_server(is, os);
+
+    /* Message type & pad */
+    test_send_u8(os, 0);
+    test_send_u8(os, 0);
+
+    /* num rect */
+    test_send_u16(os, 1);
+    /* x, y, w, h */
+    test_send_u16(os, 90);
+    test_send_u16(os, 90);
+    test_send_u16(os, 10);
+    test_send_u16(os, 10);
+
+    /* encoding=hextile */
+    test_send_s32(os, 5);
+
+    /* tile type */
+    test_send_u8(os, 0x18);
+
+    /* num rect */
+    test_send_u8(os, 1);
+
+    /* fg pix */
+    test_send_u32(os, 0x12345678);
+
+    /* x, y */
+    test_send_u8(os, 0xff);
+    test_send_u8(os, 0xff);
+}
+
+
+static void test_copyrect_bounds_server(GInputStream *is, GOutputStream *os)
+{
+    test_common_bounds_server(is, os);
+
+    /* Message type & pad */
+    test_send_u8(os, 0);
+    test_send_u8(os, 0);
+
+    /* num rect */
+    test_send_u16(os, 1);
+    /* x, y, w, h */
+    test_send_u16(os, 90);
+    test_send_u16(os, 90);
+    test_send_u16(os, 10);
+    test_send_u16(os, 10);
+
+    /* encoding=copyrect */
+    test_send_s32(os, 1);
+
+    /* src x, y */
+    test_send_u16(os, 91);
+    test_send_u16(os, 91);
+}
+
+
+static void test_validation(void (*test_func)(GInputStream *, GOutputStream *))
+{
+    struct GVncTest *test;
+    char *port;
+    GMainLoop *loop;
+    GThread *th;
+
+    test = g_new0(struct GVncTest, 1);
+    test->test_func = test_func;
+
+    g_mutex_init(&test->lock);
+    g_mutex_init(&test->clock);
+    g_cond_init(&test->cond);
+    g_mutex_lock(&test->lock);
+
+    loop = g_main_loop_new(g_main_context_default(), FALSE);
+
+    th = g_thread_new("rre-server", test_helper_server, test);
+
+    g_mutex_lock(&test->lock);
+    port = g_strdup_printf("%d", test->port);
+
+    test->conn = vnc_connection_new();
+
+    g_signal_connect(test->conn, "vnc-initialized",
+                     G_CALLBACK(test_helper_initialized), test);
+    g_signal_connect(test->conn, "vnc-disconnected",
+                     G_CALLBACK(test_helper_disconnected), test);
+    g_signal_connect(test->conn, "vnc-auth-choose-type",
+                     G_CALLBACK(test_helper_auth_choose_type), test);
+    g_signal_connect(test->conn, "vnc-desktop-resize",
+                     G_CALLBACK(test_helper_desktop_resize), test);
+    g_signal_connect(test->conn, "vnc-error",
+                     G_CALLBACK(test_helper_error), test);
+
+    vnc_connection_open_host(test->conn, "127.0.0.1", port);
+
+    test->loop = g_main_loop_new(g_main_context_default(), FALSE);
+
+    g_main_loop_run(test->loop);
+
+    g_mutex_lock(&test->clock);
+    test->quit = TRUE;
+    g_mutex_unlock(&test->clock);
+    g_cond_signal(&test->cond);
+
+    g_thread_join(th);
+
+    vnc_connection_shutdown(test->conn);
+    g_object_unref(test->conn);
+    g_free(test->pixels);
+    g_main_loop_unref(test->loop);
+
+    g_assert(test->error);
+    if (debug)
+        g_printerr("Got err %s\n", test->error);
+    g_free(test->error);
+
+    g_free(port);
+    g_free(test);
+}
+
+static void test_validation_rre(void)
+{
+    test_validation(test_rre_bounds_server);
+}
+
+static void test_validation_hextile(void)
+{
+    test_validation(test_hextile_bounds_server);
+}
+
+static void test_validation_copyrect(void)
+{
+    test_validation(test_copyrect_bounds_server);
+}
+#endif
+
+int main(int argc, char **argv) {
+    g_test_init(&argc, &argv, NULL);
+
+    if (getenv("GTK_VNC_DEBUG")) {
+        debug = TRUE;
+        vnc_util_set_debug(TRUE);
+    }
+
+#if GLIB_CHECK_VERSION(2, 22, 0)
+    g_test_add_func("/conn/validation/rre", test_validation_rre);
+    g_test_add_func("/conn/validation/copyrect", test_validation_copyrect);
+    g_test_add_func("/conn/validation/hextile", test_validation_hextile);
+#endif
+
+    return g_test_run();
+}
+/*
+ * Local variables:
+ *  c-indent-level: 4
+ *  c-basic-offset: 4
+ *  indent-tabs-mode: nil
+ * End:
+ */