X Font Service Protocol Erratum
Ted Unangst <[email protected]>
| Newsgroups | gmane.os.openbsd.announce |
|---|---|
| Message-ID | <[email protected]> |
From http://www.openbsd.org/errata55.html:
X Font Service Protocol & Font metadata file handling issues in libXfont
CVE-2014-0209: integer overflow of allocations in font metadata file parsing
CVE-2014-0210: unvalidated length fields when parsing xfs protocol replies
CVE-2014-0211: integer overflows calculating memory needs for xfs replies
Please see the advisory for more information.
http://lists.x.org/archives/xorg-announce/2014-May/002431.html
Source code patch:
untrusted comment: signature from openbsd 5.5 base secret key
RWRGy8gxk9N93+eLgi55eB+q+iJdk3vT7fqMhrHUN7dUsETsdek0CEyTtx7kXq9vjF5sYa/lCtsUIEgykH7yxDmuIuNUmE3wegc=
OpenBSD 5.5 errata 6, May 24, 2014: X Font Service Protocol
& Font metadata file handling issues in libXfont
This is revision 2 of the patch (the first version forgot to use
signify).
Apply patch using:
signify -Vep /etc/signify/openbsd-55-base.pub -x 006_libXfont.patch.sig \
-m - | (cd /usr/xenocara && patch -p0)
Then build and install libXfont
cd /usr/xenocara/lib/libXfont
make -f Makefile.bsd-wrapper obj
make -f Makefile.bsd-wrapper build
Index: lib/libXfont/src/fc/fsconvert.c
===================================================================
RCS file: /cvs/OpenBSD/xenocara/lib/libXfont/src/fc/fsconvert.c,v
retrieving revision 1.4
diff -u -p -u -r1.4 fsconvert.c
--- lib/libXfont/src/fc/fsconvert.c 4 Mar 2012 18:13:47 -0000 1.4
+++ lib/libXfont/src/fc/fsconvert.c 13 May 2014 19:42:23 -0000
@@ -118,6 +118,10 @@ _fs_convert_props(fsPropInfo *pi, fsProp
for (i = 0; i < nprops; i++, dprop++, is_str++)
{
memcpy(&local_off, off_adr, SIZEOF(fsPropOffset));
+ if ((local_off.name.position >= pi->data_len) ||
+ (local_off.name.length >
+ (pi->data_len - local_off.name.position)))
+ goto bail;
dprop->name = MakeAtom(&pdc[local_off.name.position],
local_off.name.length, 1);
if (local_off.type != PropTypeString) {
@@ -125,10 +129,15 @@ _fs_convert_props(fsPropInfo *pi, fsProp
dprop->value = local_off.value.position;
} else {
*is_str = TRUE;
+ if ((local_off.value.position >= pi->data_len) ||
+ (local_off.value.length >
+ (pi->data_len - local_off.value.position)))
+ goto bail;
dprop->value = (INT32) MakeAtom(&pdc[local_off.value.position],
local_off.value.length, 1);
if (dprop->value == BAD_RESOURCE)
{
+ bail:
free (pfi->props);
pfi->nprops = 0;
pfi->props = 0;
@@ -712,7 +721,12 @@ fs_alloc_glyphs (FontPtr pFont, int size
FSGlyphPtr glyphs;
FSFontPtr fsfont = (FSFontPtr) pFont->fontPrivate;
- glyphs = malloc (sizeof (FSGlyphRec) + size);
+ if (size < (INT_MAX - sizeof (FSGlyphRec)))
+ glyphs = malloc (sizeof (FSGlyphRec) + size);
+ else
+ glyphs = NULL;
+ if (glyphs == NULL)
+ return NULL;
glyphs->next = fsfont->glyphs;
fsfont->glyphs = glyphs;
return (pointer) (glyphs + 1);
Index: lib/libXfont/src/fc/fserve.c
===================================================================
RCS file: /cvs/OpenBSD/xenocara/lib/libXfont/src/fc/fserve.c,v
retrieving revision 1.4
diff -u -p -u -r1.4 fserve.c
--- lib/libXfont/src/fc/fserve.c 4 Mar 2012 18:13:47 -0000 1.4
+++ lib/libXfont/src/fc/fserve.c 13 May 2014 19:42:23 -0000
@@ -70,6 +70,7 @@ in this Software without prior written a
#include "fservestr.h"
#include <X11/fonts/fontutil.h>
#include <errno.h>
+#include <limits.h>
#include <time.h>
#define Time_t time_t
@@ -91,6 +92,15 @@ in this Software without prior written a
(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)
+
+/* Somewhat arbitrary limit on maximum reply size we'll try to read. */
+#define MAX_REPLY_LENGTH ((64 * 1024 * 1024) >> 2)
+
extern void ErrorF(const char *f, ...);
static int fs_read_glyphs ( FontPathElementPtr fpe, FSBlockDataPtr blockrec );
@@ -206,9 +216,22 @@ _fs_add_rep_log (FSFpePtr conn, fsGeneri
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
@@ -600,6 +623,21 @@ fs_get_reply (FSFpePtr conn, int *error)
rep = (fsGenericReply *) buf;
+ /*
+ * Refuse to accept replies longer than a maximum reasonable length,
+ * before we pass to _fs_start_read, since it will try to resize the
+ * incoming connection buffer to this size. Also avoids integer overflow
+ * on 32-bit systems.
+ */
+ if (rep->length > MAX_REPLY_LENGTH)
+ {
+ ErrorF("fserve: reply length %d > MAX_REPLY_LENGTH, disconnecting"
+ " from font server\n", rep->length);
+ _fs_connection_died (conn);
+ *error = FSIO_ERROR;
+ return 0;
+ }
+
ret = _fs_start_read (conn, rep->length << 2, &buf);
if (ret != FSIO_READY)
{
@@ -682,13 +720,15 @@ fs_read_open_font(FontPathElementPtr fpe
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;
}
@@ -815,6 +855,7 @@ fs_read_query_info(FontPathElementPtr fp
FSFpePtr conn = (FSFpePtr) fpe->private;
fsQueryXInfoReply *rep;
char *buf;
+ long bufleft; /* length of reply left to use */
fsPropInfo *pi;
fsPropOffset *po;
pointer pd;
@@ -824,13 +865,15 @@ fs_read_query_info(FontPathElementPtr fp
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;
}
@@ -844,6 +887,9 @@ fs_read_query_info(FontPathElementPtr fp
buf = (char *) rep;
buf += SIZEOF(fsQueryXInfoReply);
+ bufleft = rep->length << 2;
+ bufleft -= SIZEOF(fsQueryXInfoReply);
+
/* move the data over */
fsUnpack_XFontInfoHeader(rep, pInfo);
@@ -851,17 +897,50 @@ fs_read_query_info(FontPathElementPtr fp
_fs_init_fontinfo(conn, pInfo);
/* Compute offsets into the reply */
+ if (bufleft < SIZEOF(fsPropInfo))
+ {
+ ret = -1;
+#ifdef DEBUG
+ fprintf(stderr, "fsQueryXInfo: bufleft (%ld) < SIZEOF(fsPropInfo)\n",
+ bufleft);
+#endif
+ goto bail;
+ }
pi = (fsPropInfo *) buf;
buf += SIZEOF (fsPropInfo);
+ bufleft -= SIZEOF(fsPropInfo);
+ if ((bufleft / SIZEOF(fsPropOffset)) < pi->num_offsets)
+ {
+ ret = -1;
+#ifdef DEBUG
+ fprintf(stderr,
+ "fsQueryXInfo: bufleft (%ld) / SIZEOF(fsPropOffset) < %d\n",
+ bufleft, pi->num_offsets);
+#endif
+ goto bail;
+ }
po = (fsPropOffset *) buf;
buf += pi->num_offsets * SIZEOF(fsPropOffset);
+ bufleft -= pi->num_offsets * SIZEOF(fsPropOffset);
+ if (bufleft < pi->data_len)
+ {
+ ret = -1;
+#ifdef DEBUG
+ fprintf(stderr,
+ "fsQueryXInfo: bufleft (%ld) < data_len (%d)\n",
+ bufleft, pi->data_len);
+#endif
+ goto bail;
+ }
pd = (pointer) buf;
buf += pi->data_len;
+ bufleft -= pi->data_len;
/* convert the properties and step over the reply */
ret = _fs_convert_props(pi, po, pd, pInfo);
+ bail:
_fs_done_read (conn, rep->length << 2);
if (ret == -1)
@@ -951,13 +1030,15 @@ fs_read_extent_info(FontPathElementPtr f
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;
}
@@ -970,7 +1051,26 @@ fs_read_extent_info(FontPathElementPtr f
numInfos *= 2;
haveInk = TRUE;
}
- ci = pCI = malloc(sizeof(CharInfoRec) * numInfos);
+ if (numInfos >= (INT_MAX / sizeof(CharInfoRec))) {
+#ifdef DEBUG
+ fprintf(stderr,
+ "fsQueryXExtents16: numInfos (%d) >= %ld\n",
+ numInfos, (INT_MAX / sizeof(CharInfoRec)));
+#endif
+ pCI = NULL;
+ }
+ else if (numExtents > ((rep->length - LENGTHOF(fsQueryXExtents16Reply))
+ / LENGTHOF(fsXCharInfo))) {
+#ifdef DEBUG
+ fprintf(stderr,
+ "fsQueryXExtents16: numExtents (%d) > (%d - %d) / %d\n",
+ numExtents, rep->length,
+ LENGTHOF(fsQueryXExtents16Reply), LENGTHOF(fsXCharInfo));
+#endif
+ pCI = NULL;
+ }
+ else
+ pCI = malloc(sizeof(CharInfoRec) * numInfos);
if (!pCI)
{
@@ -1809,6 +1909,7 @@ fs_read_glyphs(FontPathElementPtr fpe, F
FontInfoPtr pfi = &pfont->info;
fsQueryXBitmaps16Reply *rep;
char *buf;
+ long bufleft; /* length of reply left to use */
fsOffset32 *ppbits;
fsOffset32 local_off;
char *off_adr;
@@ -1825,22 +1926,48 @@ fs_read_glyphs(FontPathElementPtr fpe, F
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;
}
buf = (char *) rep;
buf += SIZEOF (fsQueryXBitmaps16Reply);
+ bufleft = rep->length << 2;
+ bufleft -= SIZEOF (fsQueryXBitmaps16Reply);
+
+ if ((bufleft / SIZEOF (fsOffset32)) < rep->num_chars)
+ {
+#ifdef DEBUG
+ fprintf(stderr,
+ "fsQueryXBitmaps16: num_chars (%d) > bufleft (%ld) / %d\n",
+ rep->num_chars, bufleft, SIZEOF (fsOffset32));
+#endif
+ err = AllocError;
+ goto bail;
+ }
ppbits = (fsOffset32 *) buf;
buf += SIZEOF (fsOffset32) * (rep->num_chars);
+ bufleft -= SIZEOF (fsOffset32) * (rep->num_chars);
+ if (bufleft < rep->nbytes)
+ {
+#ifdef DEBUG
+ fprintf(stderr,
+ "fsQueryXBitmaps16: nbytes (%d) > bufleft (%ld)\n",
+ rep->nbytes, bufleft);
+#endif
+ err = AllocError;
+ goto bail;
+ }
pbitmaps = (pointer ) buf;
if (blockrec->type == FS_LOAD_GLYPHS)
@@ -1898,7 +2025,9 @@ fs_read_glyphs(FontPathElementPtr fpe, F
*/
if (NONZEROMETRICS(&fsdata->encoding[minchar].metrics))
{
- if (local_off.length)
+ if (local_off.length &&
+ (local_off.position < rep->nbytes) &&
+ (local_off.length <= (rep->nbytes - local_off.position)))
{
bits = allbits;
allbits += local_off.length;
@@ -2228,31 +2357,48 @@ fs_read_list(FontPathElementPtr fpe, FSB
FSBlockedListPtr blist = (FSBlockedListPtr) blockrec->data;
fsListFontsReply *rep;
char *data;
+ long dataleft; /* length of reply left to use */
int length,
i,
ret;
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);
+ dataleft = (rep->length << 2) - SIZEOF (fsListFontsReply);
err = Successful;
/* copy data into FontPathRecord */
for (i = 0; i < rep->nFonts; i++)
{
+ if (dataleft < 1)
+ break;
length = *(unsigned char *)data++;
+ dataleft--; /* used length byte */
+ if (length > dataleft) {
+#ifdef DEBUG
+ fprintf(stderr,
+ "fsListFonts: name length (%d) > dataleft (%ld)\n",
+ length, dataleft);
+#endif
+ err = BadFontName;
+ break;
+ }
err = AddFontNamesName(blist->names, data, length);
if (err != Successful)
break;
data += length;
+ dataleft -= length;
}
_fs_done_read (conn, rep->length << 2);
return err;
@@ -2347,6 +2493,7 @@ fs_read_list_info(FontPathElementPtr fpe
FSBlockedListInfoPtr binfo = (FSBlockedListInfoPtr) blockrec->data;
fsListFontsWithXInfoReply *rep;
char *buf;
+ long bufleft;
FSFpePtr conn = (FSFpePtr) fpe->private;
fsPropInfo *pi;
fsPropOffset *po;
@@ -2358,12 +2505,15 @@ fs_read_list_info(FontPathElementPtr fpe
_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;
}
/*
@@ -2380,6 +2530,7 @@ fs_read_list_info(FontPathElementPtr fpe
}
buf = (char *) rep + SIZEOF (fsListFontsWithXInfoReply);
+ bufleft = (rep->length << 2) - SIZEOF (fsListFontsWithXInfoReply);
/*
* The original FS implementation didn't match
@@ -2388,19 +2539,71 @@ fs_read_list_info(FontPathElementPtr fpe
*/
if (conn->fsMajorVersion <= 1)
{
+ if (rep->nameLength > bufleft) {
+#ifdef DEBUG
+ fprintf(stderr,
+ "fsListFontsWithXInfo: name length (%d) > bufleft (%ld)\n",
+ (int) rep->nameLength, bufleft);
+#endif
+ err = AllocError;
+ goto done;
+ }
+ /* binfo->name is a 256 char array, rep->nameLength is a CARD8 */
memcpy (binfo->name, buf, rep->nameLength);
buf += _fs_pad_length (rep->nameLength);
+ bufleft -= _fs_pad_length (rep->nameLength);
}
pi = (fsPropInfo *) buf;
+ if (SIZEOF (fsPropInfo) > bufleft) {
+#ifdef DEBUG
+ fprintf(stderr,
+ "fsListFontsWithXInfo: PropInfo length (%d) > bufleft (%ld)\n",
+ (int) SIZEOF (fsPropInfo), bufleft);
+#endif
+ err = AllocError;
+ goto done;
+ }
+ bufleft -= SIZEOF (fsPropInfo);
buf += SIZEOF (fsPropInfo);
po = (fsPropOffset *) buf;
+ if (pi->num_offsets > (bufleft / SIZEOF (fsPropOffset))) {
+#ifdef DEBUG
+ fprintf(stderr,
+ "fsListFontsWithXInfo: offset length (%d * %d) > bufleft (%ld)\n",
+ pi->num_offsets, (int) SIZEOF (fsPropOffset), bufleft);
+#endif
+ err = AllocError;
+ goto done;
+ }
+ bufleft -= pi->num_offsets * SIZEOF (fsPropOffset);
buf += pi->num_offsets * SIZEOF (fsPropOffset);
pd = (pointer) buf;
+ if (pi->data_len > bufleft) {
+#ifdef DEBUG
+ fprintf(stderr,
+ "fsListFontsWithXInfo: data length (%d) > bufleft (%ld)\n",
+ pi->data_len, bufleft);
+#endif
+ err = AllocError;
+ goto done;
+ }
+ bufleft -= pi->data_len;
buf += pi->data_len;
if (conn->fsMajorVersion > 1)
{
+ if (rep->nameLength > bufleft) {
+#ifdef DEBUG
+ fprintf(stderr,
+ "fsListFontsWithXInfo: name length (%d) > bufleft (%ld)\n",
+ (int) rep->nameLength, bufleft);
+#endif
+ err = AllocError;
+ goto done;
+ }
+ /* binfo->name is a 256 char array, rep->nameLength is a CARD8 */
memcpy (binfo->name, buf, rep->nameLength);
buf += _fs_pad_length (rep->nameLength);
+ bufleft -= _fs_pad_length (rep->nameLength);
}
#ifdef DEBUG
@@ -2786,7 +2989,7 @@ _fs_recv_conn_setup (FSFpePtr conn)
int ret = FSIO_ERROR;
fsConnSetup *setup;
FSFpeAltPtr alts;
- int i, alt_len;
+ unsigned int i, alt_len;
int setup_len;
char *alt_save, *alt_names;
@@ -2813,8 +3016,9 @@ _fs_recv_conn_setup (FSFpePtr conn)
}
if (setup->num_alternates)
{
+ size_t alt_name_len = setup->alternate_len << 2;
alts = malloc (setup->num_alternates * sizeof (FSFpeAltRec) +
- (setup->alternate_len << 2));
+ alt_name_len);
if (alts)
{
alt_names = (char *) (setup + 1);
@@ -2823,10 +3027,25 @@ _fs_recv_conn_setup (FSFpePtr conn)
{
alts[i].subset = alt_names[0];
alt_len = alt_names[1];
+ if (alt_len >= alt_name_len) {
+ /*
+ * Length is longer than setup->alternate_len
+ * told us to allocate room for, assume entire
+ * alternate list is corrupted.
+ */
+#ifdef DEBUG
+ fprintf (stderr,
+ "invalid alt list (length %lx >= %lx)\n",
+ (long) alt_len, (long) alt_name_len);
+#endif
+ free(alts);
+ return FSIO_ERROR;
+ }
alts[i].name = alt_save;
memcpy (alt_save, alt_names + 2, alt_len);
alt_save[alt_len] = '\0';
alt_save += alt_len + 1;
+ alt_name_len -= alt_len + 1;
alt_names += _fs_pad_length (alt_len + 2);
}
conn->numAlts = setup->num_alternates;
Index: lib/libXfont/src/fontfile/dirfile.c
===================================================================
RCS file: /cvs/OpenBSD/xenocara/lib/libXfont/src/fontfile/dirfile.c,v
retrieving revision 1.4
diff -u -p -u -r1.4 dirfile.c
--- lib/libXfont/src/fontfile/dirfile.c 4 Mar 2012 18:13:47 -0000 1.4
+++ lib/libXfont/src/fontfile/dirfile.c 13 May 2014 19:42:23 -0000
@@ -42,6 +42,7 @@ in this Software without prior written a
#include <sys/types.h>
#include <sys/stat.h>
#include <errno.h>
+#include <limits.h>
static Bool AddFileNameAliases ( FontDirectoryPtr dir );
static int ReadFontAlias ( char *directory, Bool isFile,
@@ -374,6 +375,9 @@ lexAlias(FILE *file, char **lexToken)
int nsize;
char *nbuf;
+ if (tokenSize >= (INT_MAX >> 2))
+ /* Stop before we overflow */
+ return EALLOC;
nsize = tokenSize ? (tokenSize << 1) : 64;
nbuf = realloc(tokenBuf, nsize);
if (!nbuf)
Index: lib/libXfont/src/fontfile/fontdir.c
===================================================================
RCS file: /cvs/OpenBSD/xenocara/lib/libXfont/src/fontfile/fontdir.c,v
retrieving revision 1.8
diff -u -p -u -r1.8 fontdir.c
--- lib/libXfont/src/fontfile/fontdir.c 7 Jan 2014 20:42:20 -0000 1.8
+++ lib/libXfont/src/fontfile/fontdir.c 13 May 2014 19:42:23 -0000
@@ -177,6 +177,11 @@ FontFileAddEntry(FontTablePtr table, Fon
if (table->sorted)
return (FontEntryPtr) 0; /* "cannot" happen */
if (table->used == table->size) {
+ if (table->size >= ((INT32_MAX / sizeof(FontEntryRec)) - 100))
+ /* If we've read so many entries we're going to ask for 2gb
+ or more of memory, something is so wrong with this font
+ directory that we should just give up before we overflow. */
+ return NULL;
newsize = table->size + 100;
entry = realloc(table->entries, newsize * sizeof(FontEntryRec));
if (!entry)