ChangeLog | 152 +++++++++++++++++++++++++++++++++++ autogen.sh | 4 configure.ac | 3 debian/changelog | 16 +++ debian/compat | 2 debian/control | 4 debian/rules | 14 +-- src/FSFontInfo.c | 235 +++++++++++++++++-------------------------------------- src/FSFtNames.c | 10 +- src/FSGetCats.c | 7 - src/FSListCats.c | 11 +- src/FSListExt.c | 8 - src/FSOpenServ.c | 37 +++----- src/FSQGlyphs.c | 19 ++-- src/FSQXExt.c | 7 - src/FSQXInfo.c | 7 - src/FSlibInt.c | 10 +- src/Makefile.am | 4 test/Makefile.am | 2 19 files changed, 313 insertions(+), 239 deletions(-)
New commits: commit b5ee87999882460312648316ce543ed426279eb2 Author: Julien Cristau <jcris...@debian.org> Date: Sun Jun 16 14:16:36 2013 +0200 Upload to unstable diff --git a/debian/changelog b/debian/changelog index e7f7729..06726d4 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,11 +1,11 @@ -libfs (2:1.0.5-1) UNRELEASED; urgency=low +libfs (2:1.0.5-1) unstable; urgency=low * New upstream release. * Bump debhelper compat level to 7. * Use dpkg-buildflags. * Disable silent rules. - -- Julien Cristau <jcris...@debian.org> Sun, 16 Jun 2013 13:37:14 +0200 + -- Julien Cristau <jcris...@debian.org> Sun, 16 Jun 2013 14:16:20 +0200 libfs (2:1.0.4-1+deb7u1) wheezy-security; urgency=high commit b0f2ed6857159e09b19e7a5e33920f1f654518b2 Author: Julien Cristau <jcris...@debian.org> Date: Sun Jun 16 14:03:47 2013 +0200 Disable silent rules. diff --git a/debian/changelog b/debian/changelog index 18bee78..e7f7729 100644 --- a/debian/changelog +++ b/debian/changelog @@ -3,6 +3,7 @@ libfs (2:1.0.5-1) UNRELEASED; urgency=low * New upstream release. * Bump debhelper compat level to 7. * Use dpkg-buildflags. + * Disable silent rules. -- Julien Cristau <jcris...@debian.org> Sun, 16 Jun 2013 13:37:14 +0200 diff --git a/debian/rules b/debian/rules index 0bd1562..364b306 100755 --- a/debian/rules +++ b/debian/rules @@ -37,6 +37,7 @@ build-stamp: ../configure --prefix=/usr --mandir=\$${prefix}/share/man \ --libdir=\$${prefix}/lib/$(DEB_HOST_MULTIARCH) \ --infodir=\$${prefix}/share/info \ + --disable-silent-rules \ $(confflags) cd build && $(MAKE) >$@ commit bb0441636666c419d90dca67e4f45cc12ab1633c Author: Julien Cristau <jcris...@debian.org> Date: Sun Jun 16 13:58:01 2013 +0200 Use dpkg-buildflags. diff --git a/debian/changelog b/debian/changelog index b83055c..18bee78 100644 --- a/debian/changelog +++ b/debian/changelog @@ -2,6 +2,7 @@ libfs (2:1.0.5-1) UNRELEASED; urgency=low * New upstream release. * Bump debhelper compat level to 7. + * Use dpkg-buildflags. -- Julien Cristau <jcris...@debian.org> Sun, 16 Jun 2013 13:37:14 +0200 diff --git a/debian/control b/debian/control index 900c24b..eefcef8 100644 --- a/debian/control +++ b/debian/control @@ -11,8 +11,8 @@ Build-Depends: automake, libtool, xutils-dev, -# DEB_HOST_MULTIARCH - dpkg-dev (>= 1.16), +# DEB_HOST_MULTIARCH, dpkg-buildflags --export=configure + dpkg-dev (>= 1.16.1), # misc:Pre-Depends debhelper (>= 8.1.3), Standards-Version: 3.9.3 diff --git a/debian/rules b/debian/rules index 49c8994..0bd1562 100755 --- a/debian/rules +++ b/debian/rules @@ -10,12 +10,6 @@ # set this to the name of the main shlib's binary package PACKAGE = libfs6 -CFLAGS = -Wall -g -ifneq (,$(findstring noopt,$(DEB_BUILD_OPTIONS))) - CFLAGS += -O0 -else - CFLAGS += -O2 -endif ifeq (,$(findstring nostrip,$(DEB_BUILD_OPTIONS))) INSTALL_PROGRAM += -s endif @@ -29,6 +23,7 @@ ifeq ($(DEB_BUILD_GNU_TYPE), $(DEB_HOST_GNU_TYPE)) else confflags += --build=$(DEB_BUILD_GNU_TYPE) --host=$(DEB_HOST_GNU_TYPE) endif +confflags += $(shell DEB_CFLAGS_MAINT_APPEND=-Wall dpkg-buildflags --export=configure) build: build-arch build-indep: @@ -41,8 +36,8 @@ build-stamp: cd build && \ ../configure --prefix=/usr --mandir=\$${prefix}/share/man \ --libdir=\$${prefix}/lib/$(DEB_HOST_MULTIARCH) \ - --infodir=\$${prefix}/share/info $(confflags) \ - CFLAGS="$(CFLAGS)" + --infodir=\$${prefix}/share/info \ + $(confflags) cd build && $(MAKE) >$@ commit e5aa5bf03d79d8b448affc0e51f5831cb03dbc2e Author: Julien Cristau <jcris...@debian.org> Date: Sun Jun 16 13:53:46 2013 +0200 Bump debhelper compat level to 7. diff --git a/debian/changelog b/debian/changelog index 592b353..b83055c 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,6 +1,7 @@ libfs (2:1.0.5-1) UNRELEASED; urgency=low * New upstream release. + * Bump debhelper compat level to 7. -- Julien Cristau <jcris...@debian.org> Sun, 16 Jun 2013 13:37:14 +0200 diff --git a/debian/compat b/debian/compat index 7ed6ff8..7f8f011 100644 --- a/debian/compat +++ b/debian/compat @@ -1 +1 @@ -5 +7 diff --git a/debian/rules b/debian/rules index 88c2ea7..49c8994 100755 --- a/debian/rules +++ b/debian/rules @@ -75,7 +75,7 @@ binary-arch: build install dh_installdocs dh_install --sourcedir=debian/tmp --fail-missing -XlibFS.la - dh_installchangelogs ChangeLog + dh_installchangelogs dh_link dh_strip --dbg-package=$(PACKAGE)-dbg dh_compress commit 6f95ed80a53b011766b0edafa3117d47d99a6f61 Author: Julien Cristau <jcris...@debian.org> Date: Sun Jun 16 13:37:40 2013 +0200 Bump changelogs diff --git a/ChangeLog b/ChangeLog index b8679b6..f70e9a1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,155 @@ +commit 9d1458e02fe8dcac68b32917c9b10fa49d7161e7 +Author: Alan Coopersmith <alan.coopersm...@oracle.com> +Date: Tue May 28 17:17:40 2013 -0700 + + libFS 1.0.5 + + Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> + +commit 26dc23446c2e7818fdebfb46e101bac4883df07e +Author: Alan Coopersmith <alan.coopersm...@oracle.com> +Date: Sun Apr 14 09:07:32 2013 -0700 + + Sign extension issue and integer overflow in FSOpenServer() [CVE-2013-1996] + + > altlen = (int) *ad++; <-- if char is 0xff, will sign extend to int (0xffffffff == -1) + > alts[i].name = (char *) FSmalloc(altlen + 1); <-- -1 + 1 == 0 + > ... + > memmove(alts[i].name, ad, altlen); <-- memory corruption + + Reported-by: Ilja Van Sprundel <ivansprun...@ioactive.com> + Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> + +commit f6030dd569094fb29720a4bf54aec784b1edcac5 +Author: Alan Coopersmith <alan.coopersm...@oracle.com> +Date: Wed Apr 10 21:43:40 2013 -0700 + + Get rid of more duplication in error cleanup code in FSListFontsWithXInfo + + Also get rely on free() to handle null pointers in cleanup code instead + of checking each one ourselves. + + Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> + +commit 1f260bfdcb8d83d6c21db70ad6ed0fa94e5f5abf +Author: Alan Coopersmith <alan.coopersm...@oracle.com> +Date: Wed Apr 10 20:55:36 2013 -0700 + + Avoid accessing freed memory on realloc failure in FSListFontsWithXInfo + + Since we realloc 5 things in a row, and then check for failure, it's + quite possible one of our old pointers is now pointing to something + completely different, so instead update the pointers as we successfully + realloc them and then jump to the normal error processing cleanup if + one fails. + + Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> + +commit 3022dfdcdac08a4950695ded9f372e845f2be008 +Author: Alan Coopersmith <alan.coopersm...@oracle.com> +Date: Wed Apr 10 19:04:22 2013 -0700 + + Avoid reading outside bounds when _FSReply receives an Error response + + Upon receiving a response, _FSReply copies the first 8 bytes into *rep + and then looks at them to determine what type of response. If it's an + error packet, it then converts to an error struct and reads the rest, + but it was copying 16 bytes out of *rep to begin with, due to sloppy + casting. Since we immediately overwrite the second 8 bytes with the + data coming off the wire, this isn't horrible, but it really freaks out + static analysis and memory debugging tools. + + Fixes parfait 1.1 warning: + + Error: Buffer overrun + Read Outside Array Bounds in STD C function: Read outside array bounds in call to llvm.memcpy.p0i8.p0i8.i64. Buffer ((char*)((union fsError*)rep)) of size ??? is read at an offset of 16 + size(((char*)((union fsError*)rep))) is 8, 16 is 16 + at line 751 of src/FSlibInt.c in function '_FSReply'. + called at line 67 of src/FSSync.c in function 'FSSync' with rep = ((union fsReply*)&rep). + + Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> + +commit 932131874109931bb6d50acc47ac94e51a2353de +Author: Alan Coopersmith <alan.coopersm...@oracle.com> +Date: Wed Apr 10 18:54:35 2013 -0700 + + Use NULL instead of 0 for null pointers + + Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> + +commit 90b9754da977cb6804da4c38711ff33db772a9ca +Author: Alan Coopersmith <alan.coopersm...@oracle.com> +Date: Wed Apr 10 18:30:24 2013 -0700 + + Get rid of unnecessary casts in FSfree calls + + No need to cast all other pointers to char *, since C89 free takes + any type of pointer. Casting all of them just hides errors if you + try to free something that's not really a pointer. + + Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> + +commit 2cf3ed903048758ee696d410aba6afefd1582dec +Author: Alan Coopersmith <alan.coopersm...@oracle.com> +Date: Wed Apr 10 18:26:22 2013 -0700 + + Get rid of unnecessary casts in FS*alloc calls + + Stop taking 64-bit size_t, truncating to 32-bit unsigned int, and then + putting into a 64-bit size_t argument to underlying *alloc call. + + Also stop casting results, since in C, that just hides missing prototype + errors that can cause memory corruption when taking an implicit 32-bit + int return value and trying to make a 64-bit pointer out of it. + + Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> + +commit 0ef550010ad1cb08297951b385c0034010e89a9a +Author: Alan Coopersmith <alan.coopersm...@oracle.com> +Date: Fri Jan 18 23:18:47 2013 -0800 + + Replace deprecated Automake INCLUDES variable with AM_CPPFLAGS + + Excerpt https://lists.gnu.org/archive/html/automake/2012-12/msg00038.html + + - Support for the long-deprecated INCLUDES variable will be removed + altogether in Automake 1.14. The AM_CPPFLAGS variable should be + used instead. + + This variable was deprecated in Automake releases prior to 1.10, which is + the current minimum level required to build X. + + Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> + +commit 464fb353a406cbb4c478fae89952979cb5c8775c +Author: Colin Walters <walt...@verbum.org> +Date: Wed Jan 4 17:37:06 2012 -0500 + + autogen.sh: Implement GNOME Build API + + http://people.gnome.org/~walters/docs/build-api.txt + + Signed-off-by: Adam Jackson <a...@redhat.com> + +commit 0e0109c5d035c9f803b52d2189151f600de59866 +Author: Adam Jackson <a...@redhat.com> +Date: Tue Jan 15 14:28:48 2013 -0500 + + configure: Remove AM_MAINTAINER_MODE + + Signed-off-by: Adam Jackson <a...@redhat.com> + +commit 56c83935a873fa7c6bb0c8c7d9f755ee08439aa5 +Author: Thomas Klausner <w...@netbsd.org> +Date: Thu Jul 12 14:39:26 2012 +0200 + + Fix a prototype error + + Per NetBSD PR 41899 from Henning Petersen. + + Signed-off-by: Thomas Klausner <w...@netbsd.org> + Signed-off-by: Julien Cristau <jcris...@debian.org> + commit 589eea0713cacb7b6889d15e4dbcd2914684db9b Author: Alan Coopersmith <alan.coopersm...@oracle.com> Date: Fri Mar 2 19:47:42 2012 -0800 diff --git a/debian/changelog b/debian/changelog index ed0b0b6..592b353 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +libfs (2:1.0.5-1) UNRELEASED; urgency=low + + * New upstream release. + + -- Julien Cristau <jcris...@debian.org> Sun, 16 Jun 2013 13:37:14 +0200 + libfs (2:1.0.4-1+deb7u1) wheezy-security; urgency=high * Sign extension issue and integer overflow in FSOpenServer() commit 9d1458e02fe8dcac68b32917c9b10fa49d7161e7 Author: Alan Coopersmith <alan.coopersm...@oracle.com> Date: Tue May 28 17:17:40 2013 -0700 libFS 1.0.5 Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> diff --git a/configure.ac b/configure.ac index d062381..b00749f 100644 --- a/configure.ac +++ b/configure.ac @@ -22,7 +22,7 @@ # Initialize Autoconf AC_PREREQ([2.60]) -AC_INIT([libFS], [1.0.4], +AC_INIT([libFS], [1.0.5], [https://bugs.freedesktop.org/enter_bug.cgi?product=xorg], [libFS]) AC_CONFIG_SRCDIR([Makefile.am]) AC_CONFIG_HEADERS([config.h]) commit 04dad3fde681c4381b55c24a7bfc828492834764 Author: Julien Cristau <jcris...@debian.org> Date: Mon May 13 23:32:54 2013 +0200 Upload to wheezy-security diff --git a/debian/changelog b/debian/changelog index 5d9547a..ed0b0b6 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,10 @@ +libfs (2:1.0.4-1+deb7u1) wheezy-security; urgency=high + + * Sign extension issue and integer overflow in FSOpenServer() + [CVE-2013-1996] + + -- Julien Cristau <jcris...@debian.org> Mon, 13 May 2013 23:32:48 +0200 + libfs (2:1.0.4-1) unstable; urgency=low * New upstream release. commit ca658fd3238440a73553df48e3292da071bd3635 Author: Alan Coopersmith <alan.coopersm...@oracle.com> Date: Sun Apr 14 09:07:32 2013 -0700 Sign extension issue and integer overflow in FSOpenServer() [CVE-2013-1996] > altlen = (int) *ad++; <-- if char is 0xff, will sign extend to int (0xffffffff == -1) > alts[i].name = (char *) FSmalloc(altlen + 1); <-- -1 + 1 == 0 > ... > memmove(alts[i].name, ad, altlen); <-- memory corruption Reported-by: Ilja Van Sprundel <ivansprun...@ioactive.com> Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> Signed-off-by: Julien Cristau <jcris...@debian.org> diff --git a/src/FSOpenServ.c b/src/FSOpenServ.c index 32f7d25..19efc13 100644 --- a/src/FSOpenServ.c +++ b/src/FSOpenServ.c @@ -111,10 +111,10 @@ FSOpenServer(const char *server) char *setup = NULL; fsConnSetupAccept conn; char *auth_data = NULL; - char *alt_data = NULL, + unsigned char *alt_data = NULL, *ad; AlternateServer *alts = NULL; - int altlen; + unsigned int altlen; char *vendor_string; unsigned long setuplength; @@ -158,7 +158,7 @@ FSOpenServer(const char *server) setuplength = prefix.alternate_len << 2; if (setuplength > (SIZE_MAX>>2) - || (alt_data = (char *) + || (alt_data = (unsigned char *) (setup = FSmalloc((unsigned) setuplength))) == NULL) { goto fail; } @@ -178,7 +178,7 @@ FSOpenServer(const char *server) } for (i = 0; i < prefix.num_alternates; i++) { alts[i].subset = (Bool) *ad++; - altlen = (int) *ad++; + altlen = (unsigned int) *ad++; alts[i].name = (char *) FSmalloc(altlen + 1); if (!alts[i].name) { while (--i) { commit 26dc23446c2e7818fdebfb46e101bac4883df07e Author: Alan Coopersmith <alan.coopersm...@oracle.com> Date: Sun Apr 14 09:07:32 2013 -0700 Sign extension issue and integer overflow in FSOpenServer() [CVE-2013-1996] > altlen = (int) *ad++; <-- if char is 0xff, will sign extend to int (0xffffffff == -1) > alts[i].name = (char *) FSmalloc(altlen + 1); <-- -1 + 1 == 0 > ... > memmove(alts[i].name, ad, altlen); <-- memory corruption Reported-by: Ilja Van Sprundel <ivansprun...@ioactive.com> Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> diff --git a/src/FSOpenServ.c b/src/FSOpenServ.c index f1a6157..15a657a 100644 --- a/src/FSOpenServ.c +++ b/src/FSOpenServ.c @@ -111,10 +111,10 @@ FSOpenServer(const char *server) char *setup = NULL; fsConnSetupAccept conn; char *auth_data = NULL; - char *alt_data = NULL, + unsigned char *alt_data = NULL, *ad; AlternateServer *alts = NULL; - int altlen; + unsigned int altlen; char *vendor_string; unsigned long setuplength; @@ -157,7 +157,7 @@ FSOpenServer(const char *server) setuplength = prefix.alternate_len << 2; if (setuplength > (SIZE_MAX>>2) - || (alt_data = (char *) + || (alt_data = (unsigned char *) (setup = FSmalloc(setuplength))) == NULL) { goto fail; } @@ -176,7 +176,7 @@ FSOpenServer(const char *server) } for (i = 0; i < prefix.num_alternates; i++) { alts[i].subset = (Bool) *ad++; - altlen = (int) *ad++; + altlen = (unsigned int) *ad++; alts[i].name = FSmalloc(altlen + 1); if (!alts[i].name) { while (--i) { commit f6030dd569094fb29720a4bf54aec784b1edcac5 Author: Alan Coopersmith <alan.coopersm...@oracle.com> Date: Wed Apr 10 21:43:40 2013 -0700 Get rid of more duplication in error cleanup code in FSListFontsWithXInfo Also get rely on free() to handle null pointers in cleanup code instead of checking each one ourselves. Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> diff --git a/src/FSFontInfo.c b/src/FSFontInfo.c index 2abff4f..b51e043 100644 --- a/src/FSFontInfo.c +++ b/src/FSFontInfo.c @@ -78,6 +78,7 @@ FSListFontsWithXInfo( fsPropInfo local_pi; fsPropOffset local_po; Status status; + Bool eat_data = True; GetReq(ListFontsWithXInfo, req); req->maxNames = maxNames; @@ -101,26 +102,8 @@ FSListFontsWithXInfo( SIZEOF(fsGenericReply)) >> 2), fsFalse); } if (!status) { - for (j = (i - 1); j >= 0; j--) { - FSfree(fhdr[j]); - FSfree(pi[j]); - FSfree(po[j]); - FSfree(pd[j]); - FSfree(flist[j]); - } - if (flist) - FSfree(flist); - if (fhdr) - FSfree(fhdr); - if (pi) - FSfree(pi); - if (po) - FSfree(po); - if (pd) - FSfree(pd); - - SyncHandle(); - return (char **) NULL; + eat_data = False; + goto badmem; } if (reply.nameLength == 0) /* got last reply in version 1 */ break; @@ -148,30 +131,16 @@ FSListFontsWithXInfo( ResizeArray(po, FSPropOffset) ResizeArray(pd, unsigned char) } else { - if (!(fhdr = FSmalloc(sizeof(FSXFontInfoHeader *) * size))) - goto clearwire; - if (!(flist = FSmalloc(sizeof(char *) * size))) { - FSfree(fhdr); - goto clearwire; - } - if (!(pi = FSmalloc(sizeof(FSPropInfo *) * size))) { - FSfree(fhdr); - FSfree(flist); - goto clearwire; - } - if (!(po = FSmalloc(sizeof(FSPropOffset *) * size))) { - FSfree(fhdr); - FSfree(flist); - FSfree(pi); - goto clearwire; - } - if (!(pd = FSmalloc(sizeof(unsigned char *) * size))) { - FSfree(fhdr); - FSfree(flist); - FSfree(pi); - FSfree(po); - goto clearwire; +#define InitArray(var, type) \ + if ((var = FSmalloc(sizeof(type *) * size)) == NULL) { \ + goto badmem; \ } + + InitArray(fhdr, FSXFontInfoHeader) + InitArray(flist, char) + InitArray(pi, FSPropInfo) + InitArray(po, FSPropOffset) + InitArray(pd, unsigned char) } } fhdr[i] = FSmalloc(sizeof(FSXFontInfoHeader)); @@ -182,45 +151,33 @@ FSListFontsWithXInfo( /* alloc space for the name */ flist[i] = FSmalloc(reply.nameLength + 1); + if (!flist[i]) + goto cleanfhdr; if (FSProtocolVersion(svr) == 1) { /* get the name */ - if (!flist[i]) { - nbytes = (reply.nameLength + 3) & ~3; - _FSEatData(svr, (unsigned long) nbytes); - goto badmem; - } _FSReadPad(svr, flist[i], (long) reply.nameLength); flist[i][reply.nameLength] = '\0'; } pi[i] = FSmalloc(sizeof(FSPropInfo)); - if (!pi[i]) { - FSfree(fhdr[i]); - goto badmem; - } + if (!pi[i]) + goto cleanflist; _FSReadPad(svr, (char *) &local_pi, SIZEOF(fsPropInfo)); pi[i]->num_offsets = local_pi.num_offsets; pi[i]->data_len = local_pi.data_len; #if SIZE_MAX <= UINT_MAX if (pi[i]->num_offsets > SIZE_MAX / sizeof(FSPropOffset)) - goto badmem; + goto cleanpi; #endif po[i] = FSmalloc(pi[i]->num_offsets * sizeof(FSPropOffset)); - if (!po[i]) { - FSfree(fhdr[i]); - FSfree(pi[i]); - goto badmem; - } + if (!po[i]) + goto cleanpi; pd[i] = FSmalloc(pi[i]->data_len); - if (!pd[i]) { - FSfree(fhdr[i]); - FSfree(pi[i]); - FSfree(po[i]); - goto badmem; - } + if (!pd[i]) + goto cleanpo; /* get offsets */ for (j=0; j<pi[i]->num_offsets; j++) { @@ -241,11 +198,6 @@ FSListFontsWithXInfo( if (FSProtocolVersion(svr) != 1) { /* get the name */ - if (!flist[i]) { - nbytes = (reply.nameLength + 3) & ~3; - _FSEatData(svr, (unsigned long) nbytes); - goto badmem; - } _FSRead(svr, flist[i], (long) reply.nameLength); flist[i][reply.nameLength] = '\0'; @@ -254,7 +206,7 @@ FSListFontsWithXInfo( } /* avoid integer overflow */ if (i > INT_MAX - 1) { - goto badmem; + goto cleanpd; } } *info = fhdr; @@ -265,6 +217,18 @@ FSListFontsWithXInfo( SyncHandle(); return flist; +/* Error cleanup for when we're partway through filling in item #i in arrays */ +cleanpd: + FSfree(pd[i]); +cleanpo: + FSfree(po[i]); +cleanpi: + FSfree(pi[i]); +cleanflist: + FSfree(flist[i]); +cleanfhdr: + FSfree(fhdr[i]); +/* Error cleanup for all previously filled in items in the arrays */ badmem: for (j = (i - 1); j >= 0; j--) { FSfree(pi[j]); @@ -273,29 +237,25 @@ badmem: FSfree(flist[j]); FSfree(fhdr[j]); } - if (flist) - FSfree(flist); - if (fhdr) - FSfree(fhdr); - if (pi) - FSfree(pi); - if (po) - FSfree(po); - if (pd) - FSfree(pd); + FSfree(flist); + FSfree(fhdr); + FSfree(pi); + FSfree(po); + FSfree(pd); + if (eat_data) { + do { + fsPropInfo ti; -clearwire: - do { - fsPropInfo ti; - - _FSEatData(svr, (reply.nameLength + 3) & ~3); - _FSReadPad(svr, (char *) &ti, SIZEOF(fsPropInfo)); - _FSEatData(svr, (SIZEOF(fsPropOffset) * ti.num_offsets)); - _FSEatData(svr, ti.data_len); - } while (_FSReply(svr, (fsReply *) & reply, - ((SIZEOF(fsListFontsWithXInfoReply) - - SIZEOF(fsGenericReply)) >> 2), fsFalse) && (reply.nameLength != 0)); + _FSEatData(svr, (reply.nameLength + 3) & ~3); + _FSReadPad(svr, (char *) &ti, SIZEOF(fsPropInfo)); + _FSEatData(svr, (SIZEOF(fsPropOffset) * ti.num_offsets)); + _FSEatData(svr, ti.data_len); + } while (_FSReply(svr, (fsReply *) &reply, + ((SIZEOF(fsListFontsWithXInfoReply) + - SIZEOF(fsGenericReply)) >> 2), fsFalse) + && (reply.nameLength != 0)); + } SyncHandle(); return (char **) NULL; } commit 1f260bfdcb8d83d6c21db70ad6ed0fa94e5f5abf Author: Alan Coopersmith <alan.coopersm...@oracle.com> Date: Wed Apr 10 20:55:36 2013 -0700 Avoid accessing freed memory on realloc failure in FSListFontsWithXInfo Since we realloc 5 things in a row, and then check for failure, it's quite possible one of our old pointers is now pointing to something completely different, so instead update the pointers as we successfully realloc them and then jump to the normal error processing cleanup if one fails. Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> diff --git a/src/FSFontInfo.c b/src/FSFontInfo.c index 0b71719..2abff4f 100644 --- a/src/FSFontInfo.c +++ b/src/FSFontInfo.c @@ -134,52 +134,19 @@ FSListFontsWithXInfo( goto badmem; if (fhdr) { - FSXFontInfoHeader **tmp_fhdr = - FSrealloc(fhdr, sizeof(FSXFontInfoHeader *) * size); - char **tmp_flist = - FSrealloc(flist, sizeof(char *) * size); - FSPropInfo **tmp_pi = - FSrealloc(pi, sizeof(FSPropInfo *) * size); - FSPropOffset **tmp_po = - FSrealloc(po, sizeof(FSPropOffset *) * size); - unsigned char **tmp_pd = - FSrealloc(pd, sizeof(unsigned char *) * size); - - if (!tmp_fhdr || !tmp_flist || !tmp_pi || !tmp_po || !tmp_pd) { - for (j = (i - 1); j >= 0; j--) { - FSfree(flist[j]); - FSfree(fhdr[j]); - FSfree(pi[j]); - FSfree(po[j]); - FSfree(pd[j]); - } - if (tmp_flist) - FSfree(tmp_flist); - else - FSfree(flist); - if (tmp_fhdr) - FSfree(tmp_fhdr); - else - FSfree(fhdr); - if (tmp_pi) - FSfree(tmp_pi); - else - FSfree(pi); - if (tmp_po) - FSfree(tmp_po); - else - FSfree(po); - if (tmp_pd) - FSfree(tmp_pd); - else - FSfree(pd); - goto clearwire; +#define ResizeArray(var, type) { \ + type **tmp = FSrealloc(var, sizeof(type *) * size); \ + if (tmp) \ + var = tmp; \ + else \ + goto badmem; \ } - fhdr = tmp_fhdr; - flist = tmp_flist; - pi = tmp_pi; - po = tmp_po; - pd = tmp_pd; + + ResizeArray(fhdr, FSXFontInfoHeader) + ResizeArray(flist, char) + ResizeArray(pi, FSPropInfo) + ResizeArray(po, FSPropOffset) + ResizeArray(pd, unsigned char) } else { if (!(fhdr = FSmalloc(sizeof(FSXFontInfoHeader *) * size))) goto clearwire; commit 3022dfdcdac08a4950695ded9f372e845f2be008 Author: Alan Coopersmith <alan.coopersm...@oracle.com> Date: Wed Apr 10 19:04:22 2013 -0700 Avoid reading outside bounds when _FSReply receives an Error response Upon receiving a response, _FSReply copies the first 8 bytes into *rep and then looks at them to determine what type of response. If it's an error packet, it then converts to an error struct and reads the rest, but it was copying 16 bytes out of *rep to begin with, due to sloppy casting. Since we immediately overwrite the second 8 bytes with the data coming off the wire, this isn't horrible, but it really freaks out static analysis and memory debugging tools. Fixes parfait 1.1 warning: Error: Buffer overrun Read Outside Array Bounds in STD C function: Read outside array bounds in call to llvm.memcpy.p0i8.p0i8.i64. Buffer ((char*)((union fsError*)rep)) of size ??? is read at an offset of 16 size(((char*)((union fsError*)rep))) is 8, 16 is 16 at line 751 of src/FSlibInt.c in function '_FSReply'. called at line 67 of src/FSSync.c in function 'FSSync' with rep = ((union fsReply*)&rep). Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> diff --git a/src/FSlibInt.c b/src/FSlibInt.c index 0c24f89..96c5e62 100644 --- a/src/FSlibInt.c +++ b/src/FSlibInt.c @@ -748,7 +748,8 @@ _FSReply( unsigned long serial; long err_data; - err = *(fsError *) rep; + /* copy in the part we already read off the wire */ + memcpy(&err, rep, SIZEOF(fsReply)); /* read the rest of the error */ _FSRead(svr, (char *) &err + SIZEOF(fsReply), (long) (SIZEOF(fsError) - SIZEOF(fsReply))); commit 932131874109931bb6d50acc47ac94e51a2353de Author: Alan Coopersmith <alan.coopersm...@oracle.com> Date: Wed Apr 10 18:54:35 2013 -0700 Use NULL instead of 0 for null pointers Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> diff --git a/src/FSFontInfo.c b/src/FSFontInfo.c index bfeeb65..0b71719 100644 --- a/src/FSFontInfo.c +++ b/src/FSFontInfo.c @@ -68,10 +68,10 @@ FSListFontsWithXInfo( int i, j; size_t size = 0; - FSXFontInfoHeader **fhdr = (FSXFontInfoHeader **) 0; - FSPropInfo **pi = (FSPropInfo **) 0; - FSPropOffset **po = (FSPropOffset **) 0; - unsigned char **pd = (unsigned char **) 0; + FSXFontInfoHeader **fhdr = (FSXFontInfoHeader **) NULL; + FSPropInfo **pi = (FSPropInfo **) NULL; + FSPropOffset **po = (FSPropOffset **) NULL; + unsigned char **pd = (unsigned char **) NULL; char **flist = NULL; fsListFontsWithXInfoReply reply; fsListFontsWithXInfoReq *req; diff --git a/src/FSFtNames.c b/src/FSFtNames.c index f884d75..1cac9d4 100644 --- a/src/FSFtNames.c +++ b/src/FSFtNames.c @@ -77,7 +77,7 @@ FSListFonts( _FSSend(svr, pattern, nbytes); if (!_FSReply(svr, (fsReply *) & rep, (SIZEOF(fsListFontsReply) - SIZEOF(fsGenericReply)) >> 2, fsFalse)) - return (char **) 0; + return (char **) NULL; if (rep.nFonts #if (SIZE_MAX >> 2) <= UINT_MAX diff --git a/src/FSListCats.c b/src/FSListCats.c index 0135792..7987f79 100644 --- a/src/FSListCats.c +++ b/src/FSListCats.c @@ -77,7 +77,7 @@ FSListCatalogues( _FSSend(svr, pattern, nbytes); if (!_FSReply(svr, (fsReply *) & rep, (SIZEOF(fsListCataloguesReply) - SIZEOF(fsGenericReply)) >> 2, fsFalse)) - return (char **) 0; + return (char **) NULL; if (rep.num_catalogues #if (SIZE_MAX >> 2) <= UINT_MAX commit 90b9754da977cb6804da4c38711ff33db772a9ca Author: Alan Coopersmith <alan.coopersm...@oracle.com> Date: Wed Apr 10 18:30:24 2013 -0700 Get rid of unnecessary casts in FSfree calls No need to cast all other pointers to char *, since C89 free takes any type of pointer. Casting all of them just hides errors if you try to free something that's not really a pointer. Signed-off-by: Alan Coopersmith <alan.coopersm...@oracle.com> diff --git a/src/FSFontInfo.c b/src/FSFontInfo.c index fcc91ea..bfeeb65 100644 --- a/src/FSFontInfo.c +++ b/src/FSFontInfo.c @@ -102,22 +102,22 @@ FSListFontsWithXInfo( } if (!status) { for (j = (i - 1); j >= 0; j--) { - FSfree((char *) fhdr[j]); - FSfree((char *) pi[j]); - FSfree((char *) po[j]); - FSfree((char *) pd[j]); + FSfree(fhdr[j]); + FSfree(pi[j]); + FSfree(po[j]); + FSfree(pd[j]); FSfree(flist[j]); } if (flist) - FSfree((char *) flist); + FSfree(flist); if (fhdr) - FSfree((char *) fhdr); + FSfree(fhdr); if (pi) - FSfree((char *) pi); + FSfree(pi); if (po) - FSfree((char *) po); + FSfree(po); if (pd) - FSfree((char *) pd); + FSfree(pd); SyncHandle(); return (char **) NULL; @@ -147,32 +147,32 @@ FSListFontsWithXInfo( if (!tmp_fhdr || !tmp_flist || !tmp_pi || !tmp_po || !tmp_pd) { for (j = (i - 1); j >= 0; j--) { - FSfree((char *) flist[j]); - FSfree((char *) fhdr[j]); - FSfree((char *) pi[j]); - FSfree((char *) po[j]); - FSfree((char *) pd[j]); + FSfree(flist[j]); + FSfree(fhdr[j]); + FSfree(pi[j]); + FSfree(po[j]); + FSfree(pd[j]); } if (tmp_flist) - FSfree((char *) tmp_flist); + FSfree(tmp_flist); else - FSfree((char *) flist); + FSfree(flist); if (tmp_fhdr) - FSfree((char *) tmp_fhdr); + FSfree(tmp_fhdr); else - FSfree((char *) fhdr); + FSfree(fhdr); -- To UNSUBSCRIBE, email to debian-x-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/e1uobvo-0006v5...@vasks.debian.org