Commit c8583fd3 authored by Daniel P. Berrange's avatar Daniel P. Berrange

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: 's avatarDaniel P. Berrange <berrange@redhat.com>
parent 661a676e
......@@ -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;
......
......@@ -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);
......
......@@ -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();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment