Vulnerability: The sandboxing code in libarchive 3.2.0 and earlier mishandles hardlink archive entries of non-zero data size, which might allow remote attackers to write to arbitrary files via a crafted archive file.
Vulnerability ID: CVE-2016-5418
CWE: CWE-20
Severity score: 2.9
Project: https://github.com/libarchive/libarchive
Published: 2016-09-21
File: libarchive/archive_write_disk_posix.c
Commit message: Fixes for Issue #745 and Issue #746 from Doran Moppert.
Commit author: Tim Kientzle

Vulnerable version (before fix):
 struct archive_write_disk {  #define HFS_BLOCKS(s)	((s) >> 12)  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(struct archive_write_disk *); static int	create_dir(struct archive_write_disk *, char *); static int	create_parent_dir(struct archive_write_disk *, char *); create_filesystem_object(struct archive_write_disk *a) 	const char *linkname; 	mode_t final_mode, mode; 	int r;  	/* We identify hard/symlinks according to the link names. */ 	/* Since link(2) and symlink(2) don't handle modes, we're done here. */ create_filesystem_object(struct archive_write_disk *a) #if !HAVE_LINK 		return (EPERM); #else 		r = link(linkname, a->name) ? errno : 0; 		/* 		 * New cpio and pax formats allow hardlink entries current_fixup(struct archive_write_disk *a, const char *pathname)  * recent paths.  */ /* TODO: Extend this to support symlinks on Windows Vista and later. */ static int check_symlinks(struct archive_write_disk *a) { #if !defined(HAVE_LSTAT) 	/* Platform doesn't have lstat, so we can't look for symlinks. */ 	(void)a; /* UNUSED */ 	return (ARCHIVE_OK); #else 	char *pn; 	char c; 	int r; 	struct stat st;  	/* 	 * Guard against symlink tricks.  Reject any archive entry whose 	 * destination would be altered by a symlink. 	 */ 	/* 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; 	} 	/* 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')) { 		/* Skip the next path element. */ 		while (*pn != '\0' && *pn != '/') 			++pn; 		c = pn[0]; 		pn[0] = '\0'; 		/* Check that we haven't hit a symlink. */ 		r = lstat(a->name, &st); 		if (r != 0) { 			/* We've hit a dir that doesn't exist; stop now. */ 			if (errno == ENOENT) { 				break; 			} else { 				/* 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); 			} 		} else if (S_ISLNK(st.st_mode)) { 			if (c == '\0') { 				/* 				 * 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); 				} 				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); 				} 				/* Symlink gone.  No more problem! */ 				pn[0] = c; 				return (0); 			} else if (a->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); 				} 				a->pst = NULL; 			} else { 				archive_set_error(&a->archive, 0, 				    "Cannot extract through symlink %s", 				    a->name); 				pn[0] = c; 				return (ARCHIVE_FAILED); 			} 		} 		pn[0] = c; 		if (pn[0] != '\0') 			pn++; /* 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); #endif }  #if defined(__CYGWIN__) /*  * 1. Convert a path separator from '\' to '/' . cleanup_pathname_win(struct archive_write_disk *a)  * is set) if the path is absolute.  */ static int cleanup_pathname(struct archive_write_disk *a) { 	char *dest, *src; 	char separator = '\0';  	dest = src = a->name; 	if (*src == '\0') { 		archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC, 		    "Invalid empty pathname"); 		return (ARCHIVE_FAILED); 	}  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"); 			return (ARCHIVE_FAILED); 		}  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 '..'"); 						return (ARCHIVE_FAILED); 					} 				} cleanup_pathname(struct archive_write_disk *a) 	 * We've just copied zero or more path elements, not including the 	 * final '/'. 	 */ 	if (dest == a->name) { 		/* 		 * Nothing got copied.  The path must have been something 		 * like '.' or '/' or './' or '/././././/./'. cleanup_pathname(struct archive_write_disk *a) 	return (ARCHIVE_OK); }  /*  * Create the parent directory of the specified path, assuming path  * is already in mutable storage.

Fixed version (after fix):
 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 *); 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. */ 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 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_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 	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 	 */ 	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(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 (*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(head, &st); 		if (r != 0) { 			tail[0] = c; 			/* We've hit a dir that doesn't exist; stop now. */ 			if (errno == ENOENT) { 				break; 			} else { 				/* 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. */ 				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 (last) { 				/* 				 * Last element is symlink; remove it 				 * so we can overwrite it with the 				 * item being extracted. 				 */ 				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; 				} 				/* 				 * 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. 				 */ 				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! */ 				res = ARCHIVE_OK; 				break; 			} else if (flags & ARCHIVE_EXTRACT_UNLINK) { 				/* User asked us to remove problems. */ 				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; 				} 				tail[0] = c; 			} else { 				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; 			} 		} 		/* be sure to always maintain this */ 		tail[0] = c; 		if (tail[0] != '\0') 			tail++; /* Advance to the next segment. */ 	} 	/* 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 '/' . cleanup_pathname_win(struct archive_write_disk *a)  * is set) if the path is absolute.  */ static int cleanup_pathname_fsobj(char *path, int *error_number, struct archive_string *error_string, int flags) { 	char *dest, *src; 	char separator = '\0';  	dest = src = path; 	if (*src == '\0') { 		if (error_number) *error_number = ARCHIVE_ERRNO_MISC; 		if (error_string) 		    archive_string_sprintf(error_string, 			    "Invalid empty pathname"); 		return (ARCHIVE_FAILED); 	}  cleanup_pathname(struct archive_write_disk *a) #endif 	/* Skip leading '/'. */ 	if (*src == '/') { 		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); 		}  cleanup_pathname(struct archive_write_disk *a) 			} else if (src[1] == '.') { 				if (src[2] == '/' || src[2] == '\0') { 					/* Conditionally warn about '..' */ 					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); 					} 				} cleanup_pathname(struct archive_write_disk *a) 	 * We've just copied zero or more path elements, not including the 	 * final '/'. 	 */ 	if (dest == path) { 		/* 		 * Nothing got copied.  The path must have been something 		 * like '.' or '/' or './' or '/././././/./'. 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.

Patch:
patch: @@ -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.
path: libarchive/archive_write_disk_posix.c