[Pkg-libvirt-commits] [gtk-vnc] 03/05: CVE-2017-5885: Correctly validate color map range indexes
Guido Guenther
agx at moszumanska.debian.org
Fri Feb 10 13:28:41 UTC 2017
This is an automated email from the git hooks/post-receive script.
agx pushed a commit to branch debian/sid
in repository gtk-vnc.
commit ca87acec18d3e63a6145472e4230d5b83e71e3d4
Author: Guido Günther <agx at sigxcpu.org>
Date: Fri Feb 10 10:27:10 2017 +0100
CVE-2017-5885: Correctly validate color map range indexes
Closes: #854450
---
...orrectly-validate-color-map-range-indexes.patch | 178 +++++++++++++++++++++
...or-map-entries-for-true-color-pixel-forma.patch | 171 ++++++++++++++++++++
debian/patches/series | 2 +
3 files changed, 351 insertions(+)
diff --git a/debian/patches/security/Correctly-validate-color-map-range-indexes.patch b/debian/patches/security/Correctly-validate-color-map-range-indexes.patch
new file mode 100644
index 0000000..7bf63e0
--- /dev/null
+++ b/debian/patches/security/Correctly-validate-color-map-range-indexes.patch
@@ -0,0 +1,178 @@
+From: "Daniel P. Berrange" <berrange at redhat.com>
+Date: Thu, 2 Feb 2017 18:18:48 +0000
+Subject: 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 at 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 8290b65..e95811f 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();
diff --git a/debian/patches/security/Don-t-accept-color-map-entries-for-true-color-pixel-forma.patch b/debian/patches/security/Don-t-accept-color-map-entries-for-true-color-pixel-forma.patch
new file mode 100644
index 0000000..1d9ddc5
--- /dev/null
+++ b/debian/patches/security/Don-t-accept-color-map-entries-for-true-color-pixel-forma.patch
@@ -0,0 +1,171 @@
+From: "Daniel P. Berrange" <berrange at redhat.com>
+Date: Thu, 2 Feb 2017 18:01:53 +0000
+Subject: Don't accept color map entries for true-color pixel format
+
+The color map entries should only be sent by the server
+when true-color flag is false.
+
+Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
+---
+ src/vncconnection.c | 5 +++
+ src/vncconnectiontest.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++--
+ 2 files changed, 99 insertions(+), 2 deletions(-)
+
+diff --git a/src/vncconnection.c b/src/vncconnection.c
+index 39f1966..8290b65 100644
+--- a/src/vncconnection.c
++++ b/src/vncconnection.c
+@@ -3333,6 +3333,11 @@ static gboolean vnc_connection_server_message(VncConnection *conn)
+ VncColorMap *map;
+ int i;
+
++ if (priv->fmt.true_color_flag) {
++ vnc_connection_set_error(conn, "Got color map entries in true-color pix format");
++ break;
++ }
++
+ vnc_connection_read(conn, pad, 1);
+ first_color = vnc_connection_read_u16(conn);
+ n_colors = vnc_connection_read_u16(conn);
+diff --git a/src/vncconnectiontest.c b/src/vncconnectiontest.c
+index 7ae7265..521529e 100644
+--- a/src/vncconnectiontest.c
++++ b/src/vncconnectiontest.c
+@@ -88,6 +88,22 @@ static void test_recv_u8(GInputStream *is, guint8 v)
+ g_assert(e == v);
+ }
+
++static void test_recv_u16(GInputStream *is, guint16 v)
++{
++ guint16 e;
++ g_assert(g_input_stream_read_all(is, &e, 2, NULL, NULL, NULL));
++ e = GINT16_FROM_BE(e);
++ g_assert(e == v);
++}
++
++static void test_recv_s32(GInputStream *is, gint32 v)
++{
++ gint32 e;
++ g_assert(g_input_stream_read_all(is, &e, 4, NULL, NULL, NULL));
++ e = GINT32_FROM_BE(e);
++ g_assert(e == v);
++}
++
+
+ static gpointer test_helper_server(gpointer opaque)
+ {
+@@ -128,6 +144,9 @@ static gpointer test_helper_server(gpointer opaque)
+ /* auth result */
+ test_send_u32(os, 0);
+
++ /* shared flag */
++ test_recv_u8(is, 0);
++
+ data->test_func(is, os);
+
+ g_mutex_lock(&data->clock);
+@@ -179,8 +198,7 @@ static void test_helper_initialized(VncConnection *conn,
+ gpointer opaque)
+ {
+ struct GVncTest *test = opaque;
+- gint32 encodings[] = { VNC_CONNECTION_ENCODING_DESKTOP_RESIZE,
+- VNC_CONNECTION_ENCODING_ZRLE,
++ gint32 encodings[] = { VNC_CONNECTION_ENCODING_ZRLE,
+ VNC_CONNECTION_ENCODING_HEXTILE,
+ VNC_CONNECTION_ENCODING_RRE,
+ VNC_CONNECTION_ENCODING_COPY_RECT,
+@@ -359,6 +377,74 @@ static void test_copyrect_bounds_server(GInputStream *is, GOutputStream *os)
+ }
+
+
++static void test_unexpected_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, 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));
++
++ /* 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, 0);
++ test_send_u16(os, 1);
++
++ /* r,g,b */
++ test_send_u16(os, 128);
++ test_send_u16(os, 128);
++ test_send_u16(os, 128);
++}
++
++
+ static void test_validation(void (*test_func)(GInputStream *, GOutputStream *))
+ {
+ struct GVncTest *test;
+@@ -435,6 +521,11 @@ static void test_validation_copyrect(void)
+ {
+ test_validation(test_copyrect_bounds_server);
+ }
++
++static void test_validation_unexpected_cmap(void)
++{
++ test_validation(test_unexpected_cmap_server);
++}
+ #endif
+
+ int main(int argc, char **argv) {
+@@ -449,6 +540,7 @@ int main(int argc, char **argv) {
+ 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);
++ g_test_add_func("/conn/validation/unexpectedcmap", test_validation_unexpected_cmap);
+ #endif
+
+ return g_test_run();
diff --git a/debian/patches/series b/debian/patches/series
index 8b95969..2ae11db 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1,3 +1,5 @@
Remove-GNUmakefile-links.patch
Add-I-m4-to-Makefile.am.patch
security/Fix-bounds-checking-for-RRE-hextile-copyrect-encodings.patch
+security/Don-t-accept-color-map-entries-for-true-color-pixel-forma.patch
+security/Correctly-validate-color-map-range-indexes.patch
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-libvirt/gtk-vnc.git
More information about the Pkg-libvirt-commits
mailing list