From 491291cabf78efdeec8f18b09e14726a9030cc8f Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Fri, 25 Apr 2014 23:02:34 -0700 Subject: [PATCH 06/12] CVE-2014-0210: unvalidated length fields in fs_read_query_info() fs_read_query_info() parses a reply from the font server. The reply contains embedded length fields, none of which are validated. This can cause out of bound reads in either fs_read_query_info() or in _fs_convert_props() which it calls to parse the fsPropInfo in the reply. Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Adam Jackson Reviewed-by: Matthieu Herrb --- src/fc/fsconvert.c | 9 +++++++++ src/fc/fserve.c | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/src/fc/fsconvert.c b/src/fc/fsconvert.c index 75b5372..dfa1317 100644 --- a/src/fc/fsconvert.c +++ b/src/fc/fsconvert.c @@ -118,6 +118,10 @@ _fs_convert_props(fsPropInfo *pi, fsPropOffset *po, pointer pd, 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, fsPropOffset *po, pointer pd, 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; diff --git a/src/fc/fserve.c b/src/fc/fserve.c index 3abbacf..ec5336e 100644 --- a/src/fc/fserve.c +++ b/src/fc/fserve.c @@ -854,6 +854,7 @@ fs_read_query_info(FontPathElementPtr fpe, FSBlockDataPtr blockrec) FSFpePtr conn = (FSFpePtr) fpe->private; fsQueryXInfoReply *rep; char *buf; + long bufleft; /* length of reply left to use */ fsPropInfo *pi; fsPropOffset *po; pointer pd; @@ -885,6 +886,9 @@ fs_read_query_info(FontPathElementPtr fpe, FSBlockDataPtr blockrec) buf = (char *) rep; buf += SIZEOF(fsQueryXInfoReply); + bufleft = rep->length << 2; + bufleft -= SIZEOF(fsQueryXInfoReply); + /* move the data over */ fsUnpack_XFontInfoHeader(rep, pInfo); @@ -892,17 +896,50 @@ fs_read_query_info(FontPathElementPtr fpe, FSBlockDataPtr blockrec) _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) -- 1.7.10