fix some stack and heap overflows

- resolves (rhbz#1378669, rhbz#1378668, rhbz#1378666)
2016-09-26 12:48:04 +02:00 · 2016-09-26 12:48:04 +02:00 · 1dcf5b9480
commit 1dcf5b9480
parent fd7dd348fc
4 changed files with 274 additions and 20 deletions
--- a/RHBZ#1378666.patch
+++ b/RHBZ#1378666.patch
@ -0,0 +1,38 @@
+From e37b620fe8f14535d737e89a4dcabaed4517bf1a Mon Sep 17 00:00:00 2001
+From: Tim Kientzle <kientzle@acm.org>
+Date: Sun, 21 Aug 2016 10:51:43 -0700
+Subject: [PATCH] Issue #767:  Buffer overflow printing a filename
+
+The safe_fprintf function attempts to ensure clean output for an
+arbitrary sequence of bytes by doing a trial conversion of the
+multibyte characters to wide characters -- if the resulting wide
+character is printable then we pass through the corresponding bytes
+unaltered, otherwise, we convert them to C-style ASCII escapes.
+
+The stack trace in Issue #767 suggest that the 20-byte buffer
+was getting overflowed trying to format a non-printable multibyte
+character.  This should only happen if there is a valid multibyte
+character of more than 5 bytes that was unprintable.  (Each byte
+would get expanded to a four-charcter octal-style escape of the form
+"\123" resulting in >20 characters for the >5 byte multibyte character.)
+
+I've not been able to reproduce this, but have expanded the conversion
+buffer to 128 bytes on the belief that no multibyte character set
+has a single character of more than 32 bytes.
+---
+ tar/util.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/tar/util.c b/tar/util.c
+index 9ff22f2..2b4aebe 100644
+--- a/tar/util.c
+++ b/tar/util.c
+@@ -182,7 +182,7 @@ safe_fprintf(FILE *f, const char *fmt, ...)
+ 		}
+ 
+ 		/* If our output buffer is full, dump it and keep going. */
+-		if (i > (sizeof(outbuff) - 20)) {
+		if (i > (sizeof(outbuff) - 128)) {
+ 			outbuff[i] = '\0';
+ 			fprintf(f, "%s", outbuff);
+ 			i = 0;
--- a/RHBZ#1378668.patch
+++ b/RHBZ#1378668.patch
@ -0,0 +1,66 @@
+From 7f17c791dcfd8c0416e2cd2485b19410e47ef126 Mon Sep 17 00:00:00 2001
+From: Tim Kientzle <kientzle@acm.org>
+Date: Sun, 18 Sep 2016 18:14:58 -0700
+Subject: [PATCH] Issue 761:  Heap overflow reading corrupted 7Zip files
+
+The sample file that demonstrated this had multiple 'EmptyStream'
+attributes.  The first one ended up being used to calculate
+certain statistics, then was overwritten by the second which
+was incompatible with those statistics.
+
+The fix here is to reject any header with multiple EmptyStream
+attributes.  While here, also reject headers with multiple
+EmptyFile, AntiFile, Name, or Attributes markers.
+---
+ libarchive/archive_read_support_format_7zip.c | 10 ++++++++++
+ 1 file changed, 10 insertions(+)
+
+diff --git a/libarchive/archive_read_support_format_7zip.c b/libarchive/archive_read_support_format_7zip.c
+index 1dfe52b..c0a536c 100644
+--- a/libarchive/archive_read_support_format_7zip.c
+++ b/libarchive/archive_read_support_format_7zip.c
+@@ -2431,6 +2431,8 @@ read_Header(struct archive_read *a, struct _7z_header_info *h,
+ 
+ 		switch (type) {
+ 		case kEmptyStream:
+			if (h->emptyStreamBools != NULL)
+				return (-1);
+ 			h->emptyStreamBools = calloc((size_t)zip->numFiles,
+ 			    sizeof(*h->emptyStreamBools));
+ 			if (h->emptyStreamBools == NULL)
+@@ -2451,6 +2453,8 @@ read_Header(struct archive_read *a, struct _7z_header_info *h,
+ 					return (-1);
+ 				break;
+ 			}
+			if (h->emptyFileBools != NULL)
+				return (-1);
+ 			h->emptyFileBools = calloc(empty_streams,
+ 			    sizeof(*h->emptyFileBools));
+ 			if (h->emptyFileBools == NULL)
+@@ -2465,6 +2469,8 @@ read_Header(struct archive_read *a, struct _7z_header_info *h,
+ 					return (-1);
+ 				break;
+ 			}
+			if (h->antiBools != NULL)
+				return (-1);
+ 			h->antiBools = calloc(empty_streams,
+ 			    sizeof(*h->antiBools));
+ 			if (h->antiBools == NULL)
+@@ -2491,6 +2497,8 @@ read_Header(struct archive_read *a, struct _7z_header_info *h,
+ 			if ((ll & 1) || ll < zip->numFiles * 4)
+ 				return (-1);
+ 
+			if (zip->entry_names != NULL)
+				return (-1);
+ 			zip->entry_names = malloc(ll);
+ 			if (zip->entry_names == NULL)
+ 				return (-1);
+@@ -2543,6 +2551,8 @@ read_Header(struct archive_read *a, struct _7z_header_info *h,
+ 			if ((p = header_bytes(a, 2)) == NULL)
+ 				return (-1);
+ 			allAreDefined = *p;
+			if (h->attrBools != NULL)
+				return (-1);
+ 			h->attrBools = calloc((size_t)zip->numFiles,
+ 			    sizeof(*h->attrBools));
+ 			if (h->attrBools == NULL)
--- a/RHBZ#1378669.patch
+++ b/RHBZ#1378669.patch
@ -0,0 +1,137 @@
+From eec077f52bfa2d3f7103b4b74d52572ba8a15aca Mon Sep 17 00:00:00 2001
+From: Tim Kientzle <kientzle@acm.org>
+Date: Sun, 18 Sep 2016 17:27:47 -0700
+Subject: [PATCH] Issue 747 (and others?):  Avoid OOB read when parsing
+ multiple long lines
+
+The mtree bidder needs to look several lines ahead
+in the input.  It does this by extending the read-ahead
+and parsing subsequent lines from the same growing buffer.
+A bookkeeping error when extending the read-ahead would
+sometimes lead it to significantly over-count the
+size of the line being read.
+---
+ Makefile.am                                        |  1 +
+ libarchive/archive_read_support_format_mtree.c     | 11 +++++-
+ libarchive/test/CMakeLists.txt                     |  1 +
+ libarchive/test/test_read_format_mtree_crash747.c  | 44 ++++++++++++++++++++++
+ .../test_read_format_mtree_crash747.mtree.bz2.uu   |  6 +++
+ 5 files changed, 62 insertions(+), 1 deletion(-)
+ create mode 100644 libarchive/test/test_read_format_mtree_crash747.c
+ create mode 100644 libarchive/test/test_read_format_mtree_crash747.mtree.bz2.uu
+
+diff --git a/Makefile.am b/Makefile.am
+index e161c5d..90c9ae1 100644
+--- a/Makefile.am
+++ b/Makefile.am
+@@ -449,6 +449,7 @@ libarchive_test_SOURCES= \
+ 	libarchive/test/test_read_format_lha_bugfix_0.c \
+ 	libarchive/test/test_read_format_lha_filename.c \
+ 	libarchive/test/test_read_format_mtree.c \
+	libarchive/test/test_read_format_mtree_crash747.c \
+ 	libarchive/test/test_read_format_pax_bz2.c \
+ 	libarchive/test/test_read_format_rar.c \
+ 	libarchive/test/test_read_format_rar_encryption_data.c \
+diff --git a/libarchive/archive_read_support_format_mtree.c b/libarchive/archive_read_support_format_mtree.c
+index 8c3be9a..ae58e87 100644
+--- a/libarchive/archive_read_support_format_mtree.c
+++ b/libarchive/archive_read_support_format_mtree.c
+@@ -301,6 +301,15 @@ get_line_size(const char *b, ssize_t avail, ssize_t *nlsize)
+ 	return (avail);
+ }
+ 
+/*
+ *  <---------------- ravail --------------------->
+ *  <-- diff ------> <---  avail ----------------->
+ *                   <---- len ----------->
+ * | Previous lines | line being parsed  nl extra |
+ *                  ^
+ *                  b
+ *
+ */
+ static ssize_t
+ next_line(struct archive_read *a,
+     const char **b, ssize_t *avail, ssize_t *ravail, ssize_t *nl)
+@@ -339,7 +348,7 @@ next_line(struct archive_read *a,
+ 		*b += diff;
+ 		*avail -= diff;
+ 		tested = len;/* Skip some bytes we already determinated. */
+-		len = get_line_size(*b, *avail, nl);
+		len = get_line_size(*b + len, *avail - len, nl);
+ 		if (len >= 0)
+ 			len += tested;
+ 	}
+diff --git a/libarchive/test/CMakeLists.txt b/libarchive/test/CMakeLists.txt
+index 345e42a..0cb5966 100644
+--- a/libarchive/test/CMakeLists.txt
+++ b/libarchive/test/CMakeLists.txt
+@@ -138,6 +138,7 @@ IF(ENABLE_TEST)
+     test_read_format_lha_bugfix_0.c
+     test_read_format_lha_filename.c
+     test_read_format_mtree.c
+    test_read_format_mtree_crash747.c
+     test_read_format_pax_bz2.c
+     test_read_format_rar.c
+     test_read_format_rar_encryption_data.c
+diff --git a/libarchive/test/test_read_format_mtree_crash747.c b/libarchive/test/test_read_format_mtree_crash747.c
+new file mode 100644
+index 0000000..c082845
+--- /dev/null
+++ b/libarchive/test/test_read_format_mtree_crash747.c
+@@ -0,0 +1,44 @@
+/*-
+ * Copyright (c) 2003-2016 Tim Kientzle
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR(S) ``AS IS'' AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE AUTHOR(S) BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+#include "test.h"
+
+
+/*
+ * Reproduce the crash reported in Github Issue #747.
+ */
+DEFINE_TEST(test_read_format_mtree_crash747)
+{
+	const char *reffile = "test_read_format_mtree_crash747.mtree.bz2";
+	struct archive *a;
+
+	extract_reference_file(reffile);
+
+	assert((a = archive_read_new()) != NULL);
+	assertEqualIntA(a, ARCHIVE_OK, archive_read_support_filter_bzip2(a));
+	assertEqualIntA(a, ARCHIVE_OK, archive_read_support_format_mtree(a));
+	assertEqualIntA(a, ARCHIVE_FATAL, archive_read_open_filename(a, reffile, 10240));
+	assertEqualInt(ARCHIVE_OK, archive_read_free(a));
+}
+
+diff --git a/libarchive/test/test_read_format_mtree_crash747.mtree.bz2.uu b/libarchive/test/test_read_format_mtree_crash747.mtree.bz2.uu
+new file mode 100644
+index 0000000..84f3895
+--- /dev/null
+++ b/libarchive/test/test_read_format_mtree_crash747.mtree.bz2.uu
+@@ -0,0 +1,6 @@
+begin 600 test_read_format_mtree_crash747.mtree.bz2
+M0EIH.3%!62936:OH@(@``'/[@,`0`@!``'^```)A@9\`$`@@`'4)049!IIH!
+MM021-0,F@&@6````9%>$(K!GIC*XFR0`$```J0+:$XP```!D-F)H[#SE9+2'
+4+E"L=ASXUI%R(I"HD'ZA(5?1`Q``
+`
+end
--- a/libarchive.spec
+++ b/libarchive.spec
@ -1,11 +1,18 @@
-Name:           libarchive
-Version:        3.2.1
-Release:        3%{?dist}
-Summary:        A library for handling streaming archive formats
+Name:          libarchive
+Version:       3.2.1
+Release:       4%{?dist}
+Summary:       A library for handling streaming archive formats

-License:        BSD
-URL:            http://www.libarchive.org/
-Source0:        http://www.libarchive.org/downloads/%{name}-%{version}.tar.gz
+License:       BSD
+URL:           http://www.libarchive.org/
+Source0:       http://www.libarchive.org/downloads/%{name}-%{version}.tar.gz
+
+# heap based buffer overflow in detect_form (archive_read_support_format_mtree.c)
+Patch0:        RHBZ#1378669.patch
+# heap based buffer overflow in read_header (archive_read_support_format_7zip.c)
+Patch1:        RHBZ#1378668.patch
+# stack based buffer overflow in bsdtar_expand_char (util.c)
+Patch2:        RHBZ#1378666.patch

 BuildRequires: bison
 BuildRequires: sharutils
@ -19,6 +26,8 @@ BuildRequires: libattr-devel
 BuildRequires: openssl-devel
 BuildRequires: libxml2-devel

+BuildRequires: automake
+

 %description
 Libarchive is a programming library that can create and read several different
@ -27,36 +36,36 @@ formats, and both BSD and GNU ar variants. It can also write shar archives and
 read ISO9660 CDROM images and ZIP archives.


-%package        devel
-Summary:        Development files for %{name}
-Requires:       %{name}%{?_isa} = %{version}-%{release}
+%package       devel
+Summary:       Development files for %{name}
+Requires:      %{name}%{?_isa} = %{version}-%{release}

-%description    devel
+%description devel
 The %{name}-devel package contains libraries and header files for
 developing applications that use %{name}.


-%package -n     bsdtar
-Summary:        Manipulate tape archives
-Requires:       %{name}%{?_isa} = %{version}-%{release}
+%package -n bsdtar
+Summary:       Manipulate tape archives
+Requires:      %{name}%{?_isa} = %{version}-%{release}

 %description -n bsdtar
 The bsdtar package contains standalone bsdtar utility split off regular
 libarchive packages.


-%package -n     bsdcpio
-Summary:        Copy files to and from archives
-Requires:       %{name}%{?_isa} = %{version}-%{release}
+%package -n bsdcpio
+Summary:       Copy files to and from archives
+Requires:      %{name}%{?_isa} = %{version}-%{release}

 %description -n bsdcpio
 The bsdcpio package contains standalone bsdcpio utility split off regular
 libarchive packages.


-%package -n     bsdcat
-Summary:        Expand files to standard output
-Requires:       %{name}%{?_isa} = %{version}-%{release}
+%package -n bsdcat
+Summary:       Expand files to standard output
+Requires:      %{name}%{?_isa} = %{version}-%{release}

 %description -n bsdcat
 The bsdcat program typically takes a filename as an argument or reads standard
@ -210,6 +219,10 @@ run_testsuite


 %changelog
+* Mon Sep 26 2016 Tomas Repik <trepik@redhat.com> - 3.2.1-4
+- fix some stack and heap overflows
+- resolves (rhbz#1378669, rhbz#1378668, rhbz#1378666)
+
 * Mon Aug 08 2016 Tomas Repik <trepik@redhat.com> - 3.2.1-3
 - bump release for upgradepath