From e31716860239db16e1be65da6296b205bf228a75 Mon Sep 17 00:00:00 2001 From: Ondrej Dubaj Date: Mon, 6 Sep 2021 17:28:27 +0200 Subject: [PATCH] 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(). Resolves: #1996634 --- libarchive-3.5.2-symlink-fix.patch | 193 +++++++++++++++++++++++++++++ libarchive.spec | 2 + 2 files changed, 195 insertions(+) create mode 100644 libarchive-3.5.2-symlink-fix.patch diff --git a/libarchive-3.5.2-symlink-fix.patch b/libarchive-3.5.2-symlink-fix.patch new file mode 100644 index 0000000..7ce10f5 --- /dev/null +++ b/libarchive-3.5.2-symlink-fix.patch @@ -0,0 +1,193 @@ +commit 8a1bd5c18e896f0411a991240ce0d772bb02c840 +Author: Martin Matuska +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 + } diff --git a/libarchive.spec b/libarchive.spec index 4039c82..655e5a1 100644 --- a/libarchive.spec +++ b/libarchive.spec @@ -10,6 +10,7 @@ URL: https://www.libarchive.org/ Source0: https://libarchive.org/downloads/%{name}-%{version}.tar.gz Patch1: openssl3-rmd160failure.patch +Patch2: libarchive-3.5.2-symlink-fix.patch BuildRequires: automake BuildRequires: bison @@ -215,6 +216,7 @@ run_testsuite %changelog * Mon Aug 23 2021 Ondrej Dubaj - 3.5.2-1 - Rebased to version 3.5.2 (#1996634) +- Fixed symlink handling * Mon Aug 09 2021 Mohan Boddu - 3.5.1-7 - Rebuilt for IMA sigs, glibc 2.34, aarch64 flags