Blob Blame History Raw
commit 3c378bb5e4ea8d4c9a5345083f278882322f9c9b
Author: Doran Moppert <dmoppert@redhat.com>
Date:   Fri Aug 12 13:58:57 2016 +0930

    Fix for hardlinks with .. in target path
     - factor cleanup_pathname into cleanup_pathname_fsobj
     - rename check_path_for_symlinks to check_path_fsobj for consistency

diff --git a/libarchive/archive_write_disk_posix.c b/libarchive/archive_write_disk_posix.c
index 66fc0f5..37261a5 100644
--- a/libarchive/archive_write_disk_posix.c
+++ b/libarchive/archive_write_disk_posix.c
@@ -326,13 +326,14 @@ struct archive_write_disk {
 
 #define HFS_BLOCKS(s)	((s) >> 12)
 
-static int check_path_for_symlinks(char *path, int *error_number, struct archive_string *error_string, int flags);
+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 *);
@@ -1997,7 +1998,7 @@ create_filesystem_object(struct archive_write_disk *a)
 	const char *linkname;
 	mode_t final_mode, mode;
 	int r;
-	/* these for check_path_for_symlinks */
+	/* these for check_symlinks_fsobj */
 	char *linkname_copy;	/* non-const copy of linkname */
 	struct archive_string error_string;
 	int error_number;
@@ -2014,13 +2015,22 @@ create_filesystem_object(struct archive_write_disk *a)
 		if (linkname_copy == NULL) {
 		    return (EPERM);
 		}
-		r = check_path_for_symlinks(linkname_copy, &error_number, &error_string, a->flags);
-		free(linkname_copy);
+		/* 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
@@ -2365,7 +2375,8 @@ current_fixup(struct archive_write_disk *a, const char *pathname)
  * 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_path_for_symlinks(char *path, int *error_number, struct archive_string *error_string, int flags)
+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. */
@@ -2558,7 +2569,7 @@ check_symlinks(struct archive_write_disk *a)
 	int error_number;
 	int rc;
 	archive_string_init(&error_string);
-	rc = check_path_for_symlinks(a->name, &error_number, &error_string, a->flags);
+	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);
 	}
@@ -2640,15 +2651,17 @@ cleanup_pathname_win(struct archive_write_disk *a)
  * set) any '..' in the path.
  */
 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);
 	}
 
@@ -2679,10 +2692,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);
 					}
 				}
@@ -2713,7 +2727,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 '/././././/./'.
@@ -2728,6 +2742,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.