commit 8a1bd5c18e896f0411a991240ce0d772bb02c840

Author: Martin Matuska <martin@matuska.org>

Date:   Fri Aug 27 10:56:28 2021 +0200

    Fix following symlinks when processing the fixup list

    The previous fix in b41daecb5 was incomplete. Fixup entries are

    given the original path without calling cleanup_pathname().

    To make sure we don't follow a symlink, we must strip trailing

    slashes from the path.

    The fixup entries are always directories. Make sure we try to modify

    only directories by providing O_DIRECTORY to open() (if supported)

    and if it fails to check directory via lstat().

    Fixes #1566

diff --git a/libarchive/archive_write_disk_posix.c b/libarchive/archive_write_disk_posix.c

index fcd733af..aadc5871 100644

--- a/libarchive/archive_write_disk_posix.c

+++ b/libarchive/archive_write_disk_posix.c

@@ -2462,6 +2462,7 @@ _archive_write_disk_close(struct archive *_a)

 	struct archive_write_disk *a = (struct archive_write_disk *)_a;

 	struct fixup_entry *next, *p;

 	struct stat st;

+	char *c;

 	int fd, ret;

 	archive_check_magic(&a->archive, ARCHIVE_WRITE_DISK_MAGIC,

@@ -2475,24 +2476,49 @@ _archive_write_disk_close(struct archive *_a)

 	while (p != NULL) {

 		fd = -1;

 		a->pst = NULL; /* Mark stat cache as out-of-date. */

-		if (p->fixup &

-		    (TODO_TIMES | TODO_MODE_BASE | TODO_ACLS | TODO_FFLAGS)) {

-			fd = open(p->name,

-			    O_WRONLY | O_BINARY | O_NOFOLLOW | O_CLOEXEC);

+

+		/* We must strip trailing slashes from the path to avoid

+		   dereferencing symbolic links to directories */

+		c = p->name;

+		while (*c != '\0')

+			c++;

+		while (c != p->name && *(c - 1) == '/') {

+			c--;

+			*c = '\0';

+		}

+

+		if (p->fixup == 0)

+			goto skip_fixup_entry;

+		else {

+			fd = open(p->name, O_BINARY | O_NOFOLLOW | O_RDONLY

+#if defined(O_DIRECTORY)

+			    | O_DIRECTORY

+#endif

+			    | O_CLOEXEC);

+			/*

+		 `	 * If we don't support O_DIRECTORY,

+			 * or open() has failed, we must stat()

+			 * to verify that we are opening a directory

+			 */

+#if defined(O_DIRECTORY)

 			if (fd == -1) {

-				/* If we cannot lstat, skip entry */

-				if (lstat(p->name, &st) != 0)

+				if (lstat(p->name, &st) != 0 ||

+				    !S_ISDIR(st.st_mode)) {

 					goto skip_fixup_entry;

-				/*

-				 * If we deal with a symbolic link, mark

-				 * it in the fixup mode to ensure no

-				 * modifications are made to its target.

-				 */

-				if (S_ISLNK(st.st_mode)) {

-					p->mode &= ~S_IFMT;

-					p->mode |= S_IFLNK;

 				}

 			}

+#else

+#if HAVE_FSTAT

+			if (fd > 0 && (

+			    fstat(fd, &st) != 0 || !S_ISDIR(st.st_mode))) {

+				goto skip_fixup_entry;

+			} else

+#endif

+			if (lstat(p->name, &st) != 0 ||

+			    !S_ISDIR(st.st_mode)) {

+				goto skip_fixup_entry;

+			}

+#endif

 		}

 		if (p->fixup & TODO_TIMES) {

 			set_times(a, fd, p->mode, p->name,

@@ -2504,14 +2530,13 @@ _archive_write_disk_close(struct archive *_a)

 		if (p->fixup & TODO_MODE_BASE) {

 #ifdef HAVE_FCHMOD

 			if (fd >= 0)

-				fchmod(fd, p->mode);

+				fchmod(fd, p->mode & 07777);

 			else

 #endif

 #ifdef HAVE_LCHMOD

-			lchmod(p->name, p->mode);

+			lchmod(p->name, p->mode & 07777);

 #else

-			if (!S_ISLNK(p->mode))

-				chmod(p->name, p->mode);

+			chmod(p->name, p->mode & 07777);

 #endif

 		}

 		if (p->fixup & TODO_ACLS)

@@ -2664,7 +2689,6 @@ new_fixup(struct archive_write_disk *a, const char *pathname)

 	fe->next = a->fixup_list;

 	a->fixup_list = fe;

 	fe->fixup = 0;

-	fe->mode = 0;

 	fe->name = strdup(pathname);

 	return (fe);

 }

diff --git a/libarchive/test/test_write_disk_fixup.c b/libarchive/test/test_write_disk_fixup.c

index c399c984..b83b7307 100644

--- a/libarchive/test/test_write_disk_fixup.c

+++ b/libarchive/test/test_write_disk_fixup.c

@@ -47,26 +47,50 @@ DEFINE_TEST(test_write_disk_fixup)

 	/*

 	 * Create a file

 	 */

-	assertMakeFile("victim", 0600, "a");

+	assertMakeFile("file", 0600, "a");

+

+	/*

+	 * Create a directory

+	 */

+	assertMakeDir("dir", 0700);

 	/*

 	 * Create a directory and a symlink with the same name

 	 */

-	/* Directory: dir */

+	/* Directory: dir1 */

+        assert((ae = archive_entry_new()) != NULL);

+        archive_entry_copy_pathname(ae, "dir1/");

+        archive_entry_set_mode(ae, AE_IFDIR | 0555);

+	assertEqualIntA(ad, 0, archive_write_header(ad, ae));

+	assertEqualIntA(ad, 0, archive_write_finish_entry(ad));

+        archive_entry_free(ae);

+

+	/* Directory: dir2 */

         assert((ae = archive_entry_new()) != NULL);

-        archive_entry_copy_pathname(ae, "dir");

-        archive_entry_set_mode(ae, AE_IFDIR | 0606);

+        archive_entry_copy_pathname(ae, "dir2/");

+        archive_entry_set_mode(ae, AE_IFDIR | 0555);

 	assertEqualIntA(ad, 0, archive_write_header(ad, ae));

 	assertEqualIntA(ad, 0, archive_write_finish_entry(ad));

         archive_entry_free(ae);

-	/* Symbolic Link: dir -> foo */

+	/* Symbolic Link: dir1 -> dir */

+	assert((ae = archive_entry_new()) != NULL);

+	archive_entry_copy_pathname(ae, "dir1");

+	archive_entry_set_mode(ae, AE_IFLNK | 0777);

+	archive_entry_set_size(ae, 0);

+	archive_entry_copy_symlink(ae, "dir");

+	assertEqualIntA(ad, 0, r = archive_write_header(ad, ae));

+	if (r >= ARCHIVE_WARN)

+		assertEqualIntA(ad, 0, archive_write_finish_entry(ad));

+	archive_entry_free(ae);

+

+	/* Symbolic Link: dir2 -> file */

 	assert((ae = archive_entry_new()) != NULL);

-	archive_entry_copy_pathname(ae, "dir");

+	archive_entry_copy_pathname(ae, "dir2");

 	archive_entry_set_mode(ae, AE_IFLNK | 0777);

 	archive_entry_set_size(ae, 0);

-	archive_entry_copy_symlink(ae, "victim");

+	archive_entry_copy_symlink(ae, "file");

 	assertEqualIntA(ad, 0, r = archive_write_header(ad, ae));

 	if (r >= ARCHIVE_WARN)

 		assertEqualIntA(ad, 0, archive_write_finish_entry(ad));

@@ -75,7 +99,9 @@ DEFINE_TEST(test_write_disk_fixup)

 	assertEqualInt(ARCHIVE_OK, archive_write_free(ad));

 	/* Test the entries on disk. */

-	assertIsSymlink("dir", "victim", 0);

-	assertFileMode("victim", 0600);

+	assertIsSymlink("dir1", "dir", 0);

+	assertIsSymlink("dir2", "file", 0);

+	assertFileMode("dir", 0700);

+	assertFileMode("file", 0600);

 #endif

 }