From cbb64aef35960b2882be721f4b8fbaa0fb649d12 Mon Sep 17 00:00:00 2001

From: Alan Coopersmith <alan.coopersmith@oracle.com>

Date: Fri, 25 Apr 2014 23:02:12 -0700

Subject: [PATCH 04/12] CVE-2014-0210: unvalidated lengths when reading replies from font server

Functions to handle replies to font server requests were casting replies

from the generic form to reply specific structs without first checking

that the reply was at least as long as the struct being cast to.

Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com>

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>

Reviewed-by: Adam Jackson <ajax@redhat.com>

Reviewed-by: Matthieu Herrb <matthieu@herrb.eu>

---

 src/fc/fserve.c |   44 ++++++++++++++++++++++++++++++++++++++------

 1 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/src/fc/fserve.c b/src/fc/fserve.c

index aa9acdb..f08028f 100644

--- a/src/fc/fserve.c

+++ b/src/fc/fserve.c

@@ -91,6 +91,12 @@ in this Software without prior written authorization from The Open Group.

 			     (pci)->descent || \

 			     (pci)->characterWidth)

+/*

+ * SIZEOF(r) is in bytes, length fields in the protocol are in 32-bit words,

+ * so this converts for doing size comparisons.

+ */

+#define LENGTHOF(r)	(SIZEOF(r) >> 2)

+

 extern void ErrorF(const char *f, ...);

 static int fs_read_glyphs ( FontPathElementPtr fpe, FSBlockDataPtr blockrec );

@@ -206,9 +212,22 @@ _fs_add_rep_log (FSFpePtr conn, fsGenericReply *rep)

 		 rep->sequenceNumber,

 		 conn->reqbuffer[i].opcode);

 }

+

+#define _fs_reply_failed(rep, name, op) do {                            \

+    if (rep) {                                                          \

+        if (rep->type == FS_Error)                                      \

+            fprintf (stderr, "Error: %d Request: %s\n",                 \

+                     ((fsError *)rep)->request, #name);                 \

+        else                                                            \

+            fprintf (stderr, "Bad Length for %s Reply: %d %s %d\n",     \

+                     #name, rep->length, op, LENGTHOF(name));           \

+    }                                                                   \

+} while (0)

+

 #else

 #define _fs_add_req_log(conn,op)    ((conn)->current_seq++)

 #define _fs_add_rep_log(conn,rep)

+#define _fs_reply_failed(rep,name,op)

 #endif

 static Bool

@@ -682,13 +701,15 @@ fs_read_open_font(FontPathElementPtr fpe, FSBlockDataPtr blockrec)

     int			    ret;

     rep = (fsOpenBitmapFontReply *) fs_get_reply (conn, &ret;;

-    if (!rep || rep->type == FS_Error)

+    if (!rep || rep->type == FS_Error ||

+	(rep->length != LENGTHOF(fsOpenBitmapFontReply)))

     {

 	if (ret == FSIO_BLOCK)

 	    return StillWorking;

 	if (rep)

 	    _fs_done_read (conn, rep->length << 2);

 	fs_cleanup_bfont (bfont);

+	_fs_reply_failed (rep, fsOpenBitmapFontReply, "!=");

 	return BadFontName;

     }

@@ -824,13 +845,15 @@ fs_read_query_info(FontPathElementPtr fpe, FSBlockDataPtr blockrec)

     int			ret;

     rep = (fsQueryXInfoReply *) fs_get_reply (conn, &ret;;

-    if (!rep || rep->type == FS_Error)

+    if (!rep || rep->type == FS_Error ||

+	(rep->length < LENGTHOF(fsQueryXInfoReply)))

     {

 	if (ret == FSIO_BLOCK)

 	    return StillWorking;

 	if (rep)

 	    _fs_done_read (conn, rep->length << 2);

 	fs_cleanup_bfont (bfont);

+	_fs_reply_failed (rep, fsQueryXInfoReply, "<");

 	return BadFontName;

     }

@@ -951,13 +974,15 @@ fs_read_extent_info(FontPathElementPtr fpe, FSBlockDataPtr blockrec)

     FontInfoRec		    *fi = &bfont->pfont->info;

     rep = (fsQueryXExtents16Reply *) fs_get_reply (conn, &ret;;

-    if (!rep || rep->type == FS_Error)

+    if (!rep || rep->type == FS_Error ||

+	(rep->length < LENGTHOF(fsQueryXExtents16Reply)))

     {

 	if (ret == FSIO_BLOCK)

 	    return StillWorking;

 	if (rep)

 	    _fs_done_read (conn, rep->length << 2);

 	fs_cleanup_bfont (bfont);

+	_fs_reply_failed (rep, fsQueryXExtents16Reply, "<");

 	return BadFontName;

     }

@@ -1823,13 +1848,15 @@ fs_read_glyphs(FontPathElementPtr fpe, FSBlockDataPtr blockrec)

     unsigned long	    minchar, maxchar;

     rep = (fsQueryXBitmaps16Reply *) fs_get_reply (conn, &ret;;

-    if (!rep || rep->type == FS_Error)

+    if (!rep || rep->type == FS_Error ||

+	(rep->length < LENGTHOF(fsQueryXBitmaps16Reply)))

     {

 	if (ret == FSIO_BLOCK)

 	    return StillWorking;

 	if (rep)

 	    _fs_done_read (conn, rep->length << 2);

 	err = AllocError;

+	_fs_reply_failed (rep, fsQueryXBitmaps16Reply, "<");

 	goto bail;

     }

@@ -2232,12 +2259,14 @@ fs_read_list(FontPathElementPtr fpe, FSBlockDataPtr blockrec)

     int			err;

     rep = (fsListFontsReply *) fs_get_reply (conn, &ret;;

-    if (!rep || rep->type == FS_Error)

+    if (!rep || rep->type == FS_Error ||

+	(rep->length < LENGTHOF(fsListFontsReply)))

     {

 	if (ret == FSIO_BLOCK)

 	    return StillWorking;

 	if (rep)

 	    _fs_done_read (conn, rep->length << 2);

+	_fs_reply_failed (rep, fsListFontsReply, "<");

 	return AllocError;

     }

     data = (char *) rep + SIZEOF (fsListFontsReply);

@@ -2356,12 +2385,15 @@ fs_read_list_info(FontPathElementPtr fpe, FSBlockDataPtr blockrec)

     _fs_free_props (&binfo->info);

     rep = (fsListFontsWithXInfoReply *) fs_get_reply (conn, &ret;;

-    if (!rep || rep->type == FS_Error)

+    if (!rep || rep->type == FS_Error ||

+	((rep->nameLength != 0) &&

+	 (rep->length < LENGTHOF(fsListFontsWithXInfoReply))))

     {

 	if (ret == FSIO_BLOCK)

 	    return StillWorking;

 	binfo->status = FS_LFWI_FINISHED;

 	err = AllocError;

+	_fs_reply_failed (rep, fsListFontsWithXInfoReply, "<");

 	goto done;

     }

     /*

-- 

1.7.1