[gtk-vnc] Correctly validate color map range indexes

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

commit c8583fd3783c5b811590fcb7bae4ce6e7344963e
Author: Daniel P. Berrange <berrange redhat com>
Date:   Thu Feb 2 18:18:48 2017 +0000

    Correctly validate color map range indexes
    
    The color map index could wrap around to zero causing negative
    array index accesses.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=778050
    
    CVE-2017-5885
    
    Signed-off-by: Daniel P. Berrange <berrange redhat com>

 src/vnccolormap.c       |    4 +-
 src/vncconnection.c     |   18 ++++++++--
 src/vncconnectiontest.c |   76 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 92 insertions(+), 6 deletions(-)
---
diff --git a/src/vnccolormap.c b/src/vnccolormap.c
index 25cd2fc..f3e153a 100644
--- a/src/vnccolormap.c
+++ b/src/vnccolormap.c
@@ -119,7 +119,7 @@ gboolean vnc_color_map_set(VncColorMap *map,
                            guint16 green,
                            guint16 blue)
 {
-    if (idx >= (map->size + map->offset))
+    if (idx < map->offset || idx >= (map->size + map->offset))
         return FALSE;
 
     map->colors[idx - map->offset].red = red;
@@ -149,7 +149,7 @@ gboolean vnc_color_map_lookup(VncColorMap *map,
                               guint16 *green,
                               guint16 *blue)
 {
-    if (idx >= (map->size + map->offset))
+    if (idx < map->offset || idx >= (map->size + map->offset))
         return FALSE;
 
     *red = map->colors[idx - map->offset].red;
diff --git a/src/vncconnection.c b/src/vncconnection.c
index f57fc4f..8a52346 100644
--- a/src/vncconnection.c
+++ b/src/vncconnection.c
@@ -3344,8 +3344,13 @@ static gboolean vnc_connection_server_message(VncConnection *conn)
 
         VNC_DEBUG("Colour map from %d with %d entries",
                   first_color, n_colors);
-        map = vnc_color_map_new(first_color, n_colors);
 
+        if (first_color > (65536 - n_colors)) {
+            vnc_connection_set_error(conn, "Colormap start %d out of range %d", first_color, 65536 - 
n_colors);
+            break;
+        }
+
+        map = vnc_color_map_new(first_color, n_colors);
         for (i = 0; i < n_colors; i++) {
             guint16 red, green, blue;
 
@@ -3353,9 +3358,14 @@ static gboolean vnc_connection_server_message(VncConnection *conn)
             green = vnc_connection_read_u16(conn);
             blue = vnc_connection_read_u16(conn);
 
-            vnc_color_map_set(map,
-                              i + first_color,
-                              red, green, blue);
+            if (!vnc_color_map_set(map,
+                                   i + first_color,
+                                   red, green, blue)) {
+                /* Should not be reachable given earlier range check */
+                vnc_connection_set_error(conn, "Colormap index %d out of range %d,%d",
+                                         i + first_color, first_color, 65536 - n_colors);
+                break;
+            }
         }
 
         vnc_framebuffer_set_color_map(priv->fb, map);
diff --git a/src/vncconnectiontest.c b/src/vncconnectiontest.c
index 521529e..4917b2f 100644
--- a/src/vncconnectiontest.c
+++ b/src/vncconnectiontest.c
@@ -445,6 +445,76 @@ static void test_unexpected_cmap_server(GInputStream *is, GOutputStream *os)
 }
 
 
+static void test_overflow_cmap_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, 0);
+
+    /* 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));
+
+    /* n-encodings */
+    test_recv_u8(is, 2);
+    /* pad */
+    test_recv_u8(is, 0);
+    /* num encodings */
+    test_recv_u16(is, 5);
+
+    /* encodings */
+    test_recv_s32(is, 16);
+    test_recv_s32(is, 5);
+    test_recv_s32(is, 2);
+    test_recv_s32(is, 1);
+    test_recv_s32(is, 0);
+
+    /* update request */
+    test_recv_u8(is, 3);
+    /* ! incremental */
+    test_recv_u8(is, 0);
+
+    /* x, y, w, h */
+    test_recv_u16(is, 0);
+    test_recv_u16(is, 0);
+    test_recv_u16(is, 100);
+    test_recv_u16(is, 100);
+
+    /* set color map */
+    test_send_u8(os, 1);
+    /* pad */
+    test_send_u8(os, 0);
+    /* first color, ncolors */
+    test_send_u16(os, 65535);
+    test_send_u16(os, 2);
+
+    /* r,g,b */
+    for (int i = 0 ; i < 2; i++) {
+        test_send_u16(os, i);
+        test_send_u16(os, i);
+        test_send_u16(os, i);
+    }
+}
+
+
 static void test_validation(void (*test_func)(GInputStream *, GOutputStream *))
 {
     struct GVncTest *test;
@@ -526,6 +596,11 @@ static void test_validation_unexpected_cmap(void)
 {
     test_validation(test_unexpected_cmap_server);
 }
+
+static void test_validation_overflow_cmap(void)
+{
+    test_validation(test_overflow_cmap_server);
+}
 #endif
 
 int main(int argc, char **argv) {
@@ -541,6 +616,7 @@ int main(int argc, char **argv) {
     g_test_add_func("/conn/validation/copyrect", test_validation_copyrect);
     g_test_add_func("/conn/validation/hextile", test_validation_hextile);
     g_test_add_func("/conn/validation/unexpectedcmap", test_validation_unexpected_cmap);
+    g_test_add_func("/conn/validation/overflowcmap", test_validation_overflow_cmap);
 #endif
 
     return g_test_run();