diff mbox

libarchive security fixes (was Re: Core-updates timeline)

Message ID 20161002201404.GA9126@jasmine
State New
Headers show

Commit Message

Leo Famulari Oct. 2, 2016, 8:14 p.m. UTC
On Sun, Oct 02, 2016 at 02:50:34PM -0400, Leo Famulari wrote:
> On Sun, Oct 02, 2016 at 03:38:58PM +0200, Ludovic Courtès wrote:
> > We could wait an additional day for libarchive if it’s more convenient,
> > but maybe not longer than that.
> > 
> > What do you think would be the most convenient approach?
> 
> I will send a patch that cherry-picks what I think are the most
> important bug fixes. I can't guess when libarchive 3.2.2 will be
> released.

I've attached a patch.

It cherry-picks some fixes for some filesystem attacks and two overflows
that can be triggered with "crafted" input. The details are in the patch
files.

I understand if this approach of cherry-picking a handful of commits is
not acceptable. It's hard to judge the full impact of taking only these
changes, some of which a quite significant, without being familiar with
the libarchive code.

That's the reason why I've been waiting for a new upstream release. But
I figured I should at least try to get these bug fixes into the next
release of Guix :)
From 042d5a7df4962c3b81fbfefa0027b6f1cf356b5f Mon Sep 17 00:00:00 2001
From: Leo Famulari <leo@famulari.name>
Date: Sun, 2 Oct 2016 15:58:06 -0400
Subject: [PATCH] gnu: libarchive: Fix several security issues.

* gnu/packages/backup.scm (libarchive)[replacement]: New field.
(libarchive/fixed): New variable.
* gnu/packages/patches/libarchive-7zip-heap-overflow.patch,
gnu/packages/patches/libarchive-fix-symlink-check.patch,
gnu/packages/patches/libarchive-fix-filesystem-attacks.patch,
gnu/packages/patches/libarchive-safe_fprintf-buffer-overflow.patch: New files.
* gnu/local.mk (dist_patch_DATA): Add them.
---
 gnu/local.mk                                       |   4 +
 gnu/packages/backup.scm                            |  12 +
 .../patches/libarchive-7zip-heap-overflow.patch    |  77 ++++
 .../libarchive-fix-filesystem-attacks.patch        | 445 +++++++++++++++++++++
 .../libarchive-safe_fprintf-buffer-overflow.patch  |  44 ++
 5 files changed, 582 insertions(+)
 create mode 100644 gnu/packages/patches/libarchive-7zip-heap-overflow.patch
 create mode 100644 gnu/packages/patches/libarchive-fix-filesystem-attacks.patch
 create mode 100644 gnu/packages/patches/libarchive-safe_fprintf-buffer-overflow.patch

Comments

Ludovic Courtès Oct. 3, 2016, 4:10 p.m. UTC | #1
Hi!

Leo Famulari <leo@famulari.name> skribis:

> On Sun, Oct 02, 2016 at 02:50:34PM -0400, Leo Famulari wrote:
>> On Sun, Oct 02, 2016 at 03:38:58PM +0200, Ludovic Courtès wrote:
>> > We could wait an additional day for libarchive if it’s more convenient,
>> > but maybe not longer than that.
>> > 
>> > What do you think would be the most convenient approach?
>> 
>> I will send a patch that cherry-picks what I think are the most
>> important bug fixes. I can't guess when libarchive 3.2.2 will be
>> released.
>
> I've attached a patch.
>
> It cherry-picks some fixes for some filesystem attacks and two overflows
> that can be triggered with "crafted" input. The details are in the patch
> files.
>
> I understand if this approach of cherry-picking a handful of commits is
> not acceptable. It's hard to judge the full impact of taking only these
> changes, some of which a quite significant, without being familiar with
> the libarchive code.
>
> That's the reason why I've been waiting for a new upstream release. But
> I figured I should at least try to get these bug fixes into the next
> release of Guix :)

Sounds reasonable.  :-)

> From 042d5a7df4962c3b81fbfefa0027b6f1cf356b5f Mon Sep 17 00:00:00 2001
> From: Leo Famulari <leo@famulari.name>
> Date: Sun, 2 Oct 2016 15:58:06 -0400
> Subject: [PATCH] gnu: libarchive: Fix several security issues.
>
> * gnu/packages/backup.scm (libarchive)[replacement]: New field.
> (libarchive/fixed): New variable.
> * gnu/packages/patches/libarchive-7zip-heap-overflow.patch,
> gnu/packages/patches/libarchive-fix-symlink-check.patch,
> gnu/packages/patches/libarchive-fix-filesystem-attacks.patch,
> gnu/packages/patches/libarchive-safe_fprintf-buffer-overflow.patch: New files.
> * gnu/local.mk (dist_patch_DATA): Add them.

Don’t they have a CVE assigned?  If so, please make sure to name them
accordingly.  Otherwise LGTM.

I won’t pretend to have a precise understanding of the impact of these
bugs, but clearly they can be triggered with specially-crafted input,
which sounds bad.  So better have these fixes.

Thank you!

Ludo’.
Leo Famulari Oct. 3, 2016, 6:14 p.m. UTC | #2
On Mon, Oct 03, 2016 at 06:10:10PM +0200, Ludovic Courtès wrote:
> Leo Famulari <leo@famulari.name> skribis:
> > I understand if this approach of cherry-picking a handful of commits is
> > not acceptable. It's hard to judge the full impact of taking only these
> > changes, some of which a quite significant, without being familiar with
> > the libarchive code.
> >
> > That's the reason why I've been waiting for a new upstream release. But
> > I figured I should at least try to get these bug fixes into the next
> > release of Guix :)
> 
> Sounds reasonable.  :-)

Okay, as long as the patch itself is reasonable :)

> > Subject: [PATCH] gnu: libarchive: Fix several security issues.
> >
> > * gnu/packages/backup.scm (libarchive)[replacement]: New field.
> > (libarchive/fixed): New variable.
> > * gnu/packages/patches/libarchive-7zip-heap-overflow.patch,
> > gnu/packages/patches/libarchive-fix-symlink-check.patch,
> > gnu/packages/patches/libarchive-fix-filesystem-attacks.patch,
> > gnu/packages/patches/libarchive-safe_fprintf-buffer-overflow.patch: New files.
> > * gnu/local.mk (dist_patch_DATA): Add them.
> 
> Don’t they have a CVE assigned?  If so, please make sure to name them
> accordingly.  Otherwise LGTM.

Not AFAICT, despite the fact that they have all been sent to the oss-sec
mailing list.

Both of the overflow bugs were reported here:
http://seclists.org/oss-sec/2016/q3/516

And the filesystem attacks:
http://seclists.org/oss-sec/2016/q3/255

> I won’t pretend to have a precise understanding of the impact of these
> bugs, but clearly they can be triggered with specially-crafted input,
> which sounds bad.  So better have these fixes.

My understand is the the filesystem and symlink bugs allow the creator
of the archive being parsed by libarchive to overwrite any file on the
target system due to a set of bugs related to symlink checking, via a
variety of mechanisms (detailed explanations are linked to from the
patch files).

The "safe_printf" patch corrects a stack overflow triggered by very
large multibyte characters in filenames to-be-printed. This is under the
control of whoever creates the archive file.

And the 7zip patch corrects a heap overflow when reading crafted 7zip
archives. Again, this is something the attacker can trigger.

I don't know if these two overflows are "exploitable" or not.
diff mbox

Patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 4260a92..02cd680 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -622,6 +622,10 @@  dist_patch_DATA =						\
   %D%/packages/patches/liba52-link-with-libm.patch		\
   %D%/packages/patches/liba52-set-soname.patch			\
   %D%/packages/patches/liba52-use-mtune-not-mcpu.patch		\
+  %D%/packages/patches/libarchive-7zip-heap-overflow.patch	\
+  %D%/packages/patches/libarchive-fix-symlink-check.patch	\
+  %D%/packages/patches/libarchive-fix-filesystem-attacks.patch	\
+  %D%/packages/patches/libarchive-safe_fprintf-buffer-overflow.patch	\
   %D%/packages/patches/libbonobo-activation-test-race.patch	\
   %D%/packages/patches/libcanberra-sound-theme-freedesktop.patch \
   %D%/packages/patches/libcmis-fix-test-onedrive.patch		\
diff --git a/gnu/packages/backup.scm b/gnu/packages/backup.scm
index c6f1321..797c06e 100644
--- a/gnu/packages/backup.scm
+++ b/gnu/packages/backup.scm
@@ -172,6 +172,7 @@  backups (called chunks) to allow easy burning to CD/DVD.")
 (define-public libarchive
   (package
     (name "libarchive")
+    (replacement libarchive/fixed)
     (version "3.2.1")
     (source
      (origin
@@ -227,6 +228,17 @@  archive.  In particular, note that there is currently no built-in support for
 random access nor for in-place modification.")
     (license license:bsd-2)))
 
+(define libarchive/fixed
+  (package
+    (inherit libarchive)
+    (source (origin
+              (inherit (package-source libarchive))
+              (patches (search-patches
+                         "libarchive-7zip-heap-overflow.patch"
+                         "libarchive-fix-symlink-check.patch"
+                         "libarchive-fix-filesystem-attacks.patch"
+                         "libarchive-safe_fprintf-buffer-overflow.patch"))))))
+
 (define-public rdup
   (package
     (name "rdup")
diff --git a/gnu/packages/patches/libarchive-7zip-heap-overflow.patch b/gnu/packages/patches/libarchive-7zip-heap-overflow.patch
new file mode 100644
index 0000000..bef628f
--- /dev/null
+++ b/gnu/packages/patches/libarchive-7zip-heap-overflow.patch
@@ -0,0 +1,77 @@ 
+Fix buffer overflow reading 7Zip files:
+
+https://github.com/libarchive/libarchive/issues/761
+
+Patch copied from upstream repository:
+
+https://github.com/libarchive/libarchive/commit/7f17c791dcfd8c0416e2cd2485b19410e47ef126
+
+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)
+-- 
+2.10.0
+
diff --git a/gnu/packages/patches/libarchive-fix-filesystem-attacks.patch b/gnu/packages/patches/libarchive-fix-filesystem-attacks.patch
new file mode 100644
index 0000000..bce63d5
--- /dev/null
+++ b/gnu/packages/patches/libarchive-fix-filesystem-attacks.patch
@@ -0,0 +1,445 @@ 
+This patch fixes two bugs that allow attackers to overwrite or change
+the permissions of arbitrary files:
+
+https://github.com/libarchive/libarchive/issues/745
+https://github.com/libarchive/libarchive/issues/746
+
+Patch copied from upstream repository:
+
+https://github.com/libarchive/libarchive/commit/dfd6b54ce33960e420fb206d8872fb759b577ad9
+
+From dfd6b54ce33960e420fb206d8872fb759b577ad9 Mon Sep 17 00:00:00 2001
+From: Tim Kientzle <kientzle@acm.org>
+Date: Sun, 11 Sep 2016 13:21:57 -0700
+Subject: [PATCH] Fixes for Issue #745 and Issue #746 from Doran Moppert.
+
+---
+ libarchive/archive_write_disk_posix.c | 294 ++++++++++++++++++++++++++--------
+ 1 file changed, 227 insertions(+), 67 deletions(-)
+
+diff --git a/libarchive/archive_write_disk_posix.c b/libarchive/archive_write_disk_posix.c
+index 8f0421e..abe1a86 100644
+--- a/libarchive/archive_write_disk_posix.c
++++ b/libarchive/archive_write_disk_posix.c
+@@ -326,12 +326,14 @@ struct archive_write_disk {
+ 
+ #define HFS_BLOCKS(s)	((s) >> 12)
+ 
++static int	check_symlinks_fsobj(char *path, int *error_number, struct archive_string *error_string, int flags);
+ static int	check_symlinks(struct archive_write_disk *);
+ static int	create_filesystem_object(struct archive_write_disk *);
+ static struct fixup_entry *current_fixup(struct archive_write_disk *, const char *pathname);
+ #if defined(HAVE_FCHDIR) && defined(PATH_MAX)
+ static void	edit_deep_directories(struct archive_write_disk *ad);
+ #endif
++static int	cleanup_pathname_fsobj(char *path, int *error_number, struct archive_string *error_string, int flags);
+ static int	cleanup_pathname(struct archive_write_disk *);
+ static int	create_dir(struct archive_write_disk *, char *);
+ static int	create_parent_dir(struct archive_write_disk *, char *);
+@@ -2014,6 +2016,10 @@ create_filesystem_object(struct archive_write_disk *a)
+ 	const char *linkname;
+ 	mode_t final_mode, mode;
+ 	int r;
++	/* these for check_symlinks_fsobj */
++	char *linkname_copy;	/* non-const copy of linkname */
++	struct archive_string error_string;
++	int error_number;
+ 
+ 	/* We identify hard/symlinks according to the link names. */
+ 	/* Since link(2) and symlink(2) don't handle modes, we're done here. */
+@@ -2022,6 +2028,27 @@ create_filesystem_object(struct archive_write_disk *a)
+ #if !HAVE_LINK
+ 		return (EPERM);
+ #else
++		archive_string_init(&error_string);
++		linkname_copy = strdup(linkname);
++		if (linkname_copy == NULL) {
++		    return (EPERM);
++		}
++		/* TODO: consider using the cleaned-up path as the link target? */
++		r = cleanup_pathname_fsobj(linkname_copy, &error_number, &error_string, a->flags);
++		if (r != ARCHIVE_OK) {
++			archive_set_error(&a->archive, error_number, "%s", error_string.s);
++			free(linkname_copy);
++			/* EPERM is more appropriate than error_number for our callers */
++			return (EPERM);
++		}
++		r = check_symlinks_fsobj(linkname_copy, &error_number, &error_string, a->flags);
++		if (r != ARCHIVE_OK) {
++			archive_set_error(&a->archive, error_number, "%s", error_string.s);
++			free(linkname_copy);
++			/* EPERM is more appropriate than error_number for our callers */
++			return (EPERM);
++		}
++		free(linkname_copy);
+ 		r = link(linkname, a->name) ? errno : 0;
+ 		/*
+ 		 * New cpio and pax formats allow hardlink entries
+@@ -2362,115 +2389,228 @@ current_fixup(struct archive_write_disk *a, const char *pathname)
+  * recent paths.
+  */
+ /* TODO: Extend this to support symlinks on Windows Vista and later. */
++
++/*
++ * Checks the given path to see if any elements along it are symlinks.  Returns
++ * ARCHIVE_OK if there are none, otherwise puts an error in errmsg.
++ */
+ static int
+-check_symlinks(struct archive_write_disk *a)
++check_symlinks_fsobj(char *path, int *error_number, struct archive_string *error_string, int flags)
+ {
+ #if !defined(HAVE_LSTAT)
+ 	/* Platform doesn't have lstat, so we can't look for symlinks. */
+ 	(void)a; /* UNUSED */
++	(void)path; /* UNUSED */
++	(void)error_number; /* UNUSED */
++	(void)error_string; /* UNUSED */
++	(void)flags; /* UNUSED */
+ 	return (ARCHIVE_OK);
+ #else
+-	char *pn;
++	int res = ARCHIVE_OK;
++	char *tail;
++	char *head;
++	int last;
+ 	char c;
+ 	int r;
+ 	struct stat st;
++	int restore_pwd;
++
++	/* Nothing to do here if name is empty */
++	if(path[0] == '\0')
++	    return (ARCHIVE_OK);
+ 
+ 	/*
+ 	 * Guard against symlink tricks.  Reject any archive entry whose
+ 	 * destination would be altered by a symlink.
++	 *
++	 * Walk the filename in chunks separated by '/'.  For each segment:
++	 *  - if it doesn't exist, continue
++	 *  - if it's symlink, abort or remove it
++	 *  - if it's a directory and it's not the last chunk, cd into it
++	 * As we go:
++	 *  head points to the current (relative) path
++	 *  tail points to the temporary \0 terminating the segment we're currently examining
++	 *  c holds what used to be in *tail
++	 *  last is 1 if this is the last tail
+ 	 */
+-	/* Whatever we checked last time doesn't need to be re-checked. */
+-	pn = a->name;
+-	if (archive_strlen(&(a->path_safe)) > 0) {
+-		char *p = a->path_safe.s;
+-		while ((*pn != '\0') && (*p == *pn))
+-			++p, ++pn;
+-	}
++	restore_pwd = open(".", O_RDONLY | O_BINARY | O_CLOEXEC);
++	__archive_ensure_cloexec_flag(restore_pwd);
++	if (restore_pwd < 0)
++		return (ARCHIVE_FATAL);
++	head = path;
++	tail = path;
++	last = 0;
++	/* TODO: reintroduce a safe cache here? */
+ 	/* Skip the root directory if the path is absolute. */
+-	if(pn == a->name && pn[0] == '/')
+-		++pn;
+-	c = pn[0];
+-	/* Keep going until we've checked the entire name. */
+-	while (pn[0] != '\0' && (pn[0] != '/' || pn[1] != '\0')) {
++	if(tail == path && tail[0] == '/')
++		++tail;
++	/* Keep going until we've checked the entire name.
++	 * head, tail, path all alias the same string, which is
++	 * temporarily zeroed at tail, so be careful restoring the
++	 * stashed (c=tail[0]) for error messages.
++	 * Exiting the loop with break is okay; continue is not.
++	 */
++	while (!last) {
++		/* Skip the separator we just consumed, plus any adjacent ones */
++		while (*tail == '/')
++		    ++tail;
+ 		/* Skip the next path element. */
+-		while (*pn != '\0' && *pn != '/')
+-			++pn;
+-		c = pn[0];
+-		pn[0] = '\0';
++		while (*tail != '\0' && *tail != '/')
++			++tail;
++		/* is this the last path component? */
++		last = (tail[0] == '\0') || (tail[0] == '/' && tail[1] == '\0');
++		/* temporarily truncate the string here */
++		c = tail[0];
++		tail[0] = '\0';
+ 		/* Check that we haven't hit a symlink. */
+-		r = lstat(a->name, &st);
++		r = lstat(head, &st);
+ 		if (r != 0) {
++			tail[0] = c;
+ 			/* We've hit a dir that doesn't exist; stop now. */
+ 			if (errno == ENOENT) {
+ 				break;
+ 			} else {
+-				/* Note: This effectively disables deep directory
++				/* Treat any other error as fatal - best to be paranoid here
++				 * Note: This effectively disables deep directory
+ 				 * support when security checks are enabled.
+ 				 * Otherwise, very long pathnames that trigger
+ 				 * an error here could evade the sandbox.
+ 				 * TODO: We could do better, but it would probably
+ 				 * require merging the symlink checks with the
+ 				 * deep-directory editing. */
+-				return (ARCHIVE_FAILED);
++				if (error_number) *error_number = errno;
++				if (error_string)
++					archive_string_sprintf(error_string,
++							"Could not stat %s",
++							path);
++				res = ARCHIVE_FAILED;
++				break;
++			}
++		} else if (S_ISDIR(st.st_mode)) {
++			if (!last) {
++				if (chdir(head) != 0) {
++					tail[0] = c;
++					if (error_number) *error_number = errno;
++					if (error_string)
++						archive_string_sprintf(error_string,
++								"Could not chdir %s",
++								path);
++					res = (ARCHIVE_FATAL);
++					break;
++				}
++				/* Our view is now from inside this dir: */
++				head = tail + 1;
+ 			}
+ 		} else if (S_ISLNK(st.st_mode)) {
+-			if (c == '\0') {
++			if (last) {
+ 				/*
+ 				 * Last element is symlink; remove it
+ 				 * so we can overwrite it with the
+ 				 * item being extracted.
+ 				 */
+-				if (unlink(a->name)) {
+-					archive_set_error(&a->archive, errno,
+-					    "Could not remove symlink %s",
+-					    a->name);
+-					pn[0] = c;
+-					return (ARCHIVE_FAILED);
++				if (unlink(head)) {
++					tail[0] = c;
++					if (error_number) *error_number = errno;
++					if (error_string)
++						archive_string_sprintf(error_string,
++								"Could not remove symlink %s",
++								path);
++					res = ARCHIVE_FAILED;
++					break;
+ 				}
+-				a->pst = NULL;
+ 				/*
+ 				 * Even if we did remove it, a warning
+ 				 * is in order.  The warning is silly,
+ 				 * though, if we're just replacing one
+ 				 * symlink with another symlink.
+ 				 */
+-				if (!S_ISLNK(a->mode)) {
+-					archive_set_error(&a->archive, 0,
+-					    "Removing symlink %s",
+-					    a->name);
++				tail[0] = c;
++				/* FIXME:  not sure how important this is to restore
++				if (!S_ISLNK(path)) {
++					if (error_number) *error_number = 0;
++					if (error_string)
++						archive_string_sprintf(error_string,
++								"Removing symlink %s",
++								path);
+ 				}
++				*/
+ 				/* Symlink gone.  No more problem! */
+-				pn[0] = c;
+-				return (0);
+-			} else if (a->flags & ARCHIVE_EXTRACT_UNLINK) {
++				res = ARCHIVE_OK;
++				break;
++			} else if (flags & ARCHIVE_EXTRACT_UNLINK) {
+ 				/* User asked us to remove problems. */
+-				if (unlink(a->name) != 0) {
+-					archive_set_error(&a->archive, 0,
+-					    "Cannot remove intervening symlink %s",
+-					    a->name);
+-					pn[0] = c;
+-					return (ARCHIVE_FAILED);
++				if (unlink(head) != 0) {
++					tail[0] = c;
++					if (error_number) *error_number = 0;
++					if (error_string)
++						archive_string_sprintf(error_string,
++								"Cannot remove intervening symlink %s",
++								path);
++					res = ARCHIVE_FAILED;
++					break;
+ 				}
+-				a->pst = NULL;
++				tail[0] = c;
+ 			} else {
+-				archive_set_error(&a->archive, 0,
+-				    "Cannot extract through symlink %s",
+-				    a->name);
+-				pn[0] = c;
+-				return (ARCHIVE_FAILED);
++				tail[0] = c;
++				if (error_number) *error_number = 0;
++				if (error_string)
++					archive_string_sprintf(error_string,
++							"Cannot extract through symlink %s",
++							path);
++				res = ARCHIVE_FAILED;
++				break;
+ 			}
+ 		}
+-		pn[0] = c;
+-		if (pn[0] != '\0')
+-			pn++; /* Advance to the next segment. */
++		/* be sure to always maintain this */
++		tail[0] = c;
++		if (tail[0] != '\0')
++			tail++; /* Advance to the next segment. */
+ 	}
+-	pn[0] = c;
+-	/* We've checked and/or cleaned the whole path, so remember it. */
+-	archive_strcpy(&a->path_safe, a->name);
+-	return (ARCHIVE_OK);
++	/* Catches loop exits via break */
++	tail[0] = c;
++#ifdef HAVE_FCHDIR
++	/* If we changed directory above, restore it here. */
++	if (restore_pwd >= 0) {
++		r = fchdir(restore_pwd);
++		if (r != 0) {
++			if(error_number) *error_number = errno;
++			if(error_string)
++				archive_string_sprintf(error_string,
++						"chdir() failure");
++		}
++		close(restore_pwd);
++		restore_pwd = -1;
++		if (r != 0) {
++			res = (ARCHIVE_FATAL);
++		}
++	}
++#endif
++	/* TODO: reintroduce a safe cache here? */
++	return res;
+ #endif
+ }
+ 
++/*
++ * Check a->name for symlinks, returning ARCHIVE_OK if its clean, otherwise
++ * calls archive_set_error and returns ARCHIVE_{FATAL,FAILED}
++ */
++static int
++check_symlinks(struct archive_write_disk *a)
++{
++	struct archive_string error_string;
++	int error_number;
++	int rc;
++	archive_string_init(&error_string);
++	rc = check_symlinks_fsobj(a->name, &error_number, &error_string, a->flags);
++	if (rc != ARCHIVE_OK) {
++		archive_set_error(&a->archive, error_number, "%s", error_string.s);
++	}
++	archive_string_free(&error_string);
++	a->pst = NULL;	/* to be safe */
++	return rc;
++}
++
++
+ #if defined(__CYGWIN__)
+ /*
+  * 1. Convert a path separator from '\' to '/' .
+@@ -2544,15 +2684,17 @@ cleanup_pathname_win(struct archive_write_disk *a)
+  * is set) if the path is absolute.
+  */
+ static int
+-cleanup_pathname(struct archive_write_disk *a)
++cleanup_pathname_fsobj(char *path, int *error_number, struct archive_string *error_string, int flags)
+ {
+ 	char *dest, *src;
+ 	char separator = '\0';
+ 
+-	dest = src = a->name;
++	dest = src = path;
+ 	if (*src == '\0') {
+-		archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
+-		    "Invalid empty pathname");
++		if (error_number) *error_number = ARCHIVE_ERRNO_MISC;
++		if (error_string)
++		    archive_string_sprintf(error_string,
++			    "Invalid empty pathname");
+ 		return (ARCHIVE_FAILED);
+ 	}
+ 
+@@ -2561,9 +2703,11 @@ cleanup_pathname(struct archive_write_disk *a)
+ #endif
+ 	/* Skip leading '/'. */
+ 	if (*src == '/') {
+-		if (a->flags & ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS) {
+-			archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
+-			                  "Path is absolute");
++		if (flags & ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS) {
++			if (error_number) *error_number = ARCHIVE_ERRNO_MISC;
++			if (error_string)
++			    archive_string_sprintf(error_string,
++				    "Path is absolute");
+ 			return (ARCHIVE_FAILED);
+ 		}
+ 
+@@ -2590,10 +2734,11 @@ cleanup_pathname(struct archive_write_disk *a)
+ 			} else if (src[1] == '.') {
+ 				if (src[2] == '/' || src[2] == '\0') {
+ 					/* Conditionally warn about '..' */
+-					if (a->flags & ARCHIVE_EXTRACT_SECURE_NODOTDOT) {
+-						archive_set_error(&a->archive,
+-						    ARCHIVE_ERRNO_MISC,
+-						    "Path contains '..'");
++					if (flags & ARCHIVE_EXTRACT_SECURE_NODOTDOT) {
++						if (error_number) *error_number = ARCHIVE_ERRNO_MISC;
++						if (error_string)
++						    archive_string_sprintf(error_string,
++							    "Path contains '..'");
+ 						return (ARCHIVE_FAILED);
+ 					}
+ 				}
+@@ -2624,7 +2769,7 @@ cleanup_pathname(struct archive_write_disk *a)
+ 	 * We've just copied zero or more path elements, not including the
+ 	 * final '/'.
+ 	 */
+-	if (dest == a->name) {
++	if (dest == path) {
+ 		/*
+ 		 * Nothing got copied.  The path must have been something
+ 		 * like '.' or '/' or './' or '/././././/./'.
+@@ -2639,6 +2784,21 @@ cleanup_pathname(struct archive_write_disk *a)
+ 	return (ARCHIVE_OK);
+ }
+ 
++static int
++cleanup_pathname(struct archive_write_disk *a)
++{
++	struct archive_string error_string;
++	int error_number;
++	int rc;
++	archive_string_init(&error_string);
++	rc = cleanup_pathname_fsobj(a->name, &error_number, &error_string, a->flags);
++	if (rc != ARCHIVE_OK) {
++		archive_set_error(&a->archive, error_number, "%s", error_string.s);
++	}
++	archive_string_free(&error_string);
++	return rc;
++}
++
+ /*
+  * Create the parent directory of the specified path, assuming path
+  * is already in mutable storage.
diff --git a/gnu/packages/patches/libarchive-safe_fprintf-buffer-overflow.patch b/gnu/packages/patches/libarchive-safe_fprintf-buffer-overflow.patch
new file mode 100644
index 0000000..0e70ac9
--- /dev/null
+++ b/gnu/packages/patches/libarchive-safe_fprintf-buffer-overflow.patch
@@ -0,0 +1,44 @@ 
+Fixes this buffer overflow:
+https://github.com/libarchive/libarchive/commit/e37b620fe8f14535d737e89a4dcabaed4517bf1a
+
+Patch copied from upstream source repository:
+https://github.com/libarchive/libarchive/commit/e37b620fe8f14535d737e89a4dcabaed4517bf1a
+
+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;