[gtk-vnc] Correctly validate color map range indexes
- From: Daniel P. Berrange <dberrange src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gtk-vnc] Correctly validate color map range indexes
- Date: Tue, 7 Feb 2017 13:55:32 +0000 (UTC)
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();
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]