[gtk-vnc] Add strict bounds checking on framebuffer updates

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

commit 3f8c8e67e33738bc8fea398ac874ab579698be9a
Author: Daniel P. Berrange <berrange redhat com>
Date:   Mon Oct 25 12:20:07 2010 +0100

    Add strict bounds checking on framebuffer updates
    
    Some broken VNC servers send frame buffer updates which stray
    outside the boundary of the desktop. This will lead to memory
    corruption when rendering, so explicitly check for this and
    drop the VNC connection.
    
    * src/vncconnection.c: Add framebuffer update bounds checking

 src/vncconnection.c |   56 +++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 50 insertions(+), 6 deletions(-)
---
diff --git a/src/vncconnection.c b/src/vncconnection.c
index 3727be7..009b30a 100644
--- a/src/vncconnection.c
+++ b/src/vncconnection.c
@@ -2639,37 +2639,68 @@ static void vnc_connection_ext_key_event(VncConnection *conn)
 	priv->has_ext_key_event = TRUE;
 }
 
-static void vnc_connection_framebuffer_update(VncConnection *conn, gint32 etype,
-					      guint16 x, guint16 y,
-					      guint16 width, guint16 height)
+
+static gboolean vnc_connection_validate_boundary(VncConnection *conn,
+						 guint16 width, guint16 height)
+{
+	VncConnectionPrivate *priv = conn->priv;
+
+	if (width > priv->width || height > priv->height) {
+		VNC_DEBUG("Framebuffer update %dx%d outside boundary %dx%d",
+			  width, height, priv->width, priv->height);
+		priv->has_error = TRUE;
+	}
+
+	return !vnc_connection_has_error(conn);
+}
+
+
+static gboolean vnc_connection_framebuffer_update(VncConnection *conn, gint32 etype,
+						  guint16 x, guint16 y,
+						  guint16 width, guint16 height)
 {
 	VncConnectionPrivate *priv = conn->priv;
 
 	VNC_DEBUG("FramebufferUpdate type=%d area (%dx%d) at location %d,%d",
 		   etype, width, height, x, y);
 
+	if (vnc_connection_has_error(conn))
+		return !vnc_connection_has_error(conn);
+
 	switch (etype) {
 	case VNC_CONNECTION_ENCODING_RAW:
+		if (!vnc_connection_validate_boundary(conn, width, height))
+			break;
 		vnc_connection_raw_update(conn, x, y, width, height);
 		vnc_connection_update(conn, x, y, width, height);
 		break;
 	case VNC_CONNECTION_ENCODING_COPY_RECT:
+		if (!vnc_connection_validate_boundary(conn, width, height))
+			break;
 		vnc_connection_copyrect_update(conn, x, y, width, height);
 		vnc_connection_update(conn, x, y, width, height);
 		break;
 	case VNC_CONNECTION_ENCODING_RRE:
+		if (!vnc_connection_validate_boundary(conn, width, height))
+			break;
 		vnc_connection_rre_update(conn, x, y, width, height);
 		vnc_connection_update(conn, x, y, width, height);
 		break;
 	case VNC_CONNECTION_ENCODING_HEXTILE:
+		if (!vnc_connection_validate_boundary(conn, width, height))
+			break;
 		vnc_connection_hextile_update(conn, x, y, width, height);
 		vnc_connection_update(conn, x, y, width, height);
 		break;
 	case VNC_CONNECTION_ENCODING_ZRLE:
+		if (!vnc_connection_validate_boundary(conn, width, height))
+			break;
 		vnc_connection_zrle_update(conn, x, y, width, height);
 		vnc_connection_update(conn, x, y, width, height);
 		break;
 	case VNC_CONNECTION_ENCODING_TIGHT:
+		if (!vnc_connection_validate_boundary(conn, width, height))
+			break;
 		vnc_connection_tight_update(conn, x, y, width, height);
 		vnc_connection_update(conn, x, y, width, height);
 		break;
@@ -2701,6 +2732,8 @@ static void vnc_connection_framebuffer_update(VncConnection *conn, gint32 etype,
 		priv->has_error = TRUE;
 		break;
 	}
+
+	return !vnc_connection_has_error(conn);
 }
 
 static gboolean vnc_connection_server_message(VncConnection *conn)
@@ -2709,6 +2742,9 @@ static gboolean vnc_connection_server_message(VncConnection *conn)
 	guint8 msg;
 	int ret;
 
+	if (vnc_connection_has_error(conn))
+		return !vnc_connection_has_error(conn);
+
 	/* NB: make sure that all server message functions
 	   handle has_error appropriately */
 
@@ -2743,7 +2779,8 @@ static gboolean vnc_connection_server_message(VncConnection *conn)
 			h = vnc_connection_read_u16(conn);
 			etype = vnc_connection_read_s32(conn);
 
-			vnc_connection_framebuffer_update(conn, etype, x, y, w, h);
+			if (!vnc_connection_framebuffer_update(conn, etype, x, y, w, h))
+				break;
 		}
 	}	break;
 	case 1: { /* SetColorMapEntries */
@@ -4467,11 +4504,16 @@ static gboolean vnc_connection_initialize(VncConnection *conn)
 	priv->absPointer = TRUE;
 
 	vnc_connection_read(conn, version, 12);
+	if (vnc_connection_has_error(conn)) {
+		VNC_DEBUG("Error while reading server version");
+		goto fail;
+	}
+
 	version[12] = 0;
 
  	ret = sscanf(version, "RFB %03d.%03d\n", &priv->major, &priv->minor);
 	if (ret != 2) {
-		VNC_DEBUG("Error while getting server version");
+		VNC_DEBUG("Error while parsing server version");
 		goto fail;
 	}
 
@@ -4505,6 +4547,8 @@ static gboolean vnc_connection_initialize(VncConnection *conn)
 	if (vnc_connection_has_error(conn))
 		return FALSE;
 
+	VNC_DEBUG("Initial desktop size %dx%d", priv->width, priv->height);
+
 	vnc_connection_read_pixel_format(conn, &priv->fmt);
 
 	n_name = vnc_connection_read_u32(conn);
@@ -4529,7 +4573,7 @@ static gboolean vnc_connection_initialize(VncConnection *conn)
 	return !vnc_connection_has_error(conn);
 
  fail:
-	priv->has_error = 1;
+	priv->has_error = TRUE;
 	return !vnc_connection_has_error(conn);
 }