From 497801fb8000671b375d81d48b48fc293bd431f9 Mon Sep 17 00:00:00 2001 From: Martin Matuska Date: Fri, 27 Aug 2021 12:51:01 +0200 Subject: [PATCH] libarchive: import bugfix from upstream Reworked bugfix for upstream issue #1566: Do not follow symlinks when processing the fixup list (cherry picked from commit c577bdfce6b4451ab897bfe5013543e78a7f9b62) --- .../libarchive/archive_write_disk_posix.c | 62 +++++++++++++------ .../libarchive/test/test_write_disk_fixup.c | 44 ++++++++++--- 2 files changed, 78 insertions(+), 28 deletions(-) diff --git a/contrib/libarchive/libarchive/archive_write_disk_posix.c b/contrib/libarchive/libarchive/archive_write_disk_posix.c index bcd152d9454..a554679bfd1 100644 --- a/contrib/libarchive/libarchive/archive_write_disk_posix.c +++ b/contrib/libarchive/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/contrib/libarchive/libarchive/test/test_write_disk_fixup.c b/contrib/libarchive/libarchive/test/test_write_disk_fixup.c index c399c9842e4..b83b7307929 100644 --- a/contrib/libarchive/libarchive/test/test_write_disk_fixup.c +++ b/contrib/libarchive/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 } -- 2.45.0