From 5e646b890fb3c59ef6f94221ef8ef9ae62a8a9d6 Mon Sep 17 00:00:00 2001 From: Martin Matuska Date: Sat, 21 Aug 2021 01:39:31 +0200 Subject: [PATCH] Fix extracting hardlinks to symlinks On platforms that support the linkat(2) function we can safely write hardlinks to symlinks as linkat(2) does not follow symlinks by default. Fixes #1044 --- CMakeLists.txt | 1 + build/cmake/config.h.in | 3 ++ configure.ac | 2 +- libarchive/archive_write_disk_posix.c | 36 ++++++++++++++-- libarchive/config_freebsd.h | 1 + libarchive/test/test_write_disk_hardlink.c | 49 +++++++++++++++++++++- 6 files changed, 86 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d6b9b2f96..6dfdf966b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1352,6 +1352,7 @@ CHECK_FUNCTION_EXISTS_GLIBC(lchflags HAVE_LCHFLAGS) CHECK_FUNCTION_EXISTS_GLIBC(lchmod HAVE_LCHMOD) CHECK_FUNCTION_EXISTS_GLIBC(lchown HAVE_LCHOWN) CHECK_FUNCTION_EXISTS_GLIBC(link HAVE_LINK) +CHECK_FUNCTION_EXISTS_GLIBC(linkat HAVE_LINKAT) CHECK_FUNCTION_EXISTS_GLIBC(localtime_r HAVE_LOCALTIME_R) CHECK_FUNCTION_EXISTS_GLIBC(lstat HAVE_LSTAT) CHECK_FUNCTION_EXISTS_GLIBC(lutimes HAVE_LUTIMES) diff --git a/build/cmake/config.h.in b/build/cmake/config.h.in index ff629f9ce..5ddd2f338 100644 --- a/build/cmake/config.h.in +++ b/build/cmake/config.h.in @@ -744,6 +744,9 @@ typedef uint64_t uintmax_t; /* Define to 1 if you have the `link' function. */ #cmakedefine HAVE_LINK 1 +/* Define to 1 if you have the `linkat' function. */ +#cmakedefine HAVE_LINKAT 1 + /* Define to 1 if you have the header file. */ #cmakedefine HAVE_LINUX_FIEMAP_H 1 diff --git a/configure.ac b/configure.ac index 2c3838efa..337aa8e20 100644 --- a/configure.ac +++ b/configure.ac @@ -656,7 +656,7 @@ AC_CHECK_FUNCS([fstat fstatat fstatfs fstatvfs ftruncate]) AC_CHECK_FUNCS([futimens futimes futimesat]) AC_CHECK_FUNCS([geteuid getpid getgrgid_r getgrnam_r]) AC_CHECK_FUNCS([getpwnam_r getpwuid_r getvfsbyname gmtime_r]) -AC_CHECK_FUNCS([lchflags lchmod lchown link localtime_r lstat lutimes]) +AC_CHECK_FUNCS([lchflags lchmod lchown link linkat localtime_r lstat lutimes]) AC_CHECK_FUNCS([mbrtowc memmove memset]) AC_CHECK_FUNCS([mkdir mkfifo mknod mkstemp]) AC_CHECK_FUNCS([nl_langinfo openat pipe poll posix_spawnp readlink readlinkat]) diff --git a/libarchive/archive_write_disk_posix.c b/libarchive/archive_write_disk_posix.c index 7e32fca92..83a1f6d16 100644 --- a/libarchive/archive_write_disk_posix.c +++ b/libarchive/archive_write_disk_posix.c @@ -360,7 +360,7 @@ static int la_mktemp(struct archive_write_disk *); static void fsobj_error(int *, struct archive_string *, int, const char *, const char *); static int check_symlinks_fsobj(char *, int *, struct archive_string *, - int); + int, int); 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 *, @@ -2263,7 +2263,7 @@ create_filesystem_object(struct archive_write_disk *a) return (EPERM); } r = check_symlinks_fsobj(linkname_copy, &error_number, - &error_string, a->flags); + &error_string, a->flags, 1); if (r != ARCHIVE_OK) { archive_set_error(&a->archive, error_number, "%s", error_string.s); @@ -2284,7 +2284,12 @@ create_filesystem_object(struct archive_write_disk *a) */ if (a->flags & ARCHIVE_EXTRACT_SAFE_WRITES) unlink(a->name); +#ifdef HAVE_LINKAT + r = linkat(AT_FDCWD, linkname, AT_FDCWD, a->name, + 0) ? errno : 0; +#else r = link(linkname, a->name) ? errno : 0; +#endif /* * New cpio and pax formats allow hardlink entries * to carry data, so we may have to open the file @@ -2675,7 +2680,7 @@ fsobj_error(int *a_eno, struct archive_string *a_estr, */ static int check_symlinks_fsobj(char *path, int *a_eno, struct archive_string *a_estr, - int flags) + int flags, int extracting_hardlink) { #if !defined(HAVE_LSTAT) && \ !(defined(HAVE_OPENAT) && defined(HAVE_FSTATAT) && defined(HAVE_UNLINKAT)) @@ -2684,6 +2689,7 @@ check_symlinks_fsobj(char *path, int *a_eno, struct archive_string *a_estr, (void)error_number; /* UNUSED */ (void)error_string; /* UNUSED */ (void)flags; /* UNUSED */ + (void)extracting_hardlink; /* UNUSED */ return (ARCHIVE_OK); #else int res = ARCHIVE_OK; @@ -2805,6 +2811,28 @@ check_symlinks_fsobj(char *path, int *a_eno, struct archive_string *a_estr, head = tail + 1; } } else if (S_ISLNK(st.st_mode)) { + if (last && extracting_hardlink) { +#ifdef HAVE_LINKAT + /* + * Hardlinks to symlinks are safe to write + * if linkat() is supported as it does not + * follow symlinks. + */ + res = ARCHIVE_OK; +#else + /* + * We return ARCHIVE_FAILED here as we are + * not able to safely write hardlinks + * to symlinks. + */ + tail[0] = c; + fsobj_error(a_eno, a_estr, errno, + "Cannot write hardlink to symlink ", + path); + res = ARCHIVE_FAILED; +#endif + break; + } else if (last) { /* * Last element is symlink; remove it @@ -2971,7 +2999,7 @@ check_symlinks(struct archive_write_disk *a) int rc; archive_string_init(&error_string); rc = check_symlinks_fsobj(a->name, &error_number, &error_string, - a->flags); + a->flags, 0); if (rc != ARCHIVE_OK) { archive_set_error(&a->archive, error_number, "%s", error_string.s); diff --git a/libarchive/config_freebsd.h b/libarchive/config_freebsd.h index a484618b4..ac651f00e 100644 --- a/libarchive/config_freebsd.h +++ b/libarchive/config_freebsd.h @@ -138,6 +138,7 @@ #define HAVE_LIBZ 1 #define HAVE_LIMITS_H 1 #define HAVE_LINK 1 +#define HAVE_LINKAT 1 #define HAVE_LOCALE_H 1 #define HAVE_LOCALTIME_R 1 #define HAVE_LONG_LONG_INT 1 diff --git a/libarchive/test/test_write_disk_hardlink.c b/libarchive/test/test_write_disk_hardlink.c index f80821c7e..184f77a00 100644 --- a/libarchive/test/test_write_disk_hardlink.c +++ b/libarchive/test/test_write_disk_hardlink.c @@ -49,6 +49,9 @@ DEFINE_TEST(test_write_disk_hardlink) static const char data[]="abcdefghijklmnopqrstuvwxyz"; struct archive *ad; struct archive_entry *ae; +#ifdef HAVE_LINKAT + int can_symlink; +#endif int r; /* Force the umask to something predictable. */ @@ -147,7 +150,7 @@ DEFINE_TEST(test_write_disk_hardlink) archive_entry_free(ae); /* - * Finally, try a new-cpio-like approach, where the initial + * Third, try a new-cpio-like approach, where the initial * regular file is empty and the hardlink has the data. */ @@ -174,6 +177,41 @@ DEFINE_TEST(test_write_disk_hardlink) assertEqualIntA(ad, 0, archive_write_finish_entry(ad)); } archive_entry_free(ae); + +#ifdef HAVE_LINKAT + /* Finally, try creating a hard link to a dangling symlink */ + can_symlink = canSymlink(); + if (can_symlink) { + /* Symbolic link: link5a -> foo */ + assert((ae = archive_entry_new()) != NULL); + archive_entry_copy_pathname(ae, "link5a"); + archive_entry_set_mode(ae, AE_IFLNK | 0642); + archive_entry_unset_size(ae); + archive_entry_copy_symlink(ae, "foo"); + assertEqualIntA(ad, 0, r = archive_write_header(ad, ae)); + if (r >= ARCHIVE_WARN) { + assertEqualInt(ARCHIVE_WARN, + archive_write_data(ad, data, sizeof(data))); + assertEqualIntA(ad, 0, archive_write_finish_entry(ad)); + } + archive_entry_free(ae); + + + /* Link. Size of zero means this doesn't carry data. */ + assert((ae = archive_entry_new()) != NULL); + archive_entry_copy_pathname(ae, "link5b"); + archive_entry_set_mode(ae, S_IFREG | 0642); + archive_entry_set_size(ae, 0); + archive_entry_copy_hardlink(ae, "link5a"); + assertEqualIntA(ad, 0, r = archive_write_header(ad, ae)); + if (r >= ARCHIVE_WARN) { + assertEqualInt(ARCHIVE_WARN, + archive_write_data(ad, data, sizeof(data))); + assertEqualIntA(ad, 0, archive_write_finish_entry(ad)); + } + archive_entry_free(ae); + } +#endif assertEqualInt(0, archive_write_free(ad)); /* Test the entries on disk. */ @@ -211,5 +249,14 @@ DEFINE_TEST(test_write_disk_hardlink) assertFileNLinks("link4a", 2); assertFileSize("link4a", sizeof(data)); assertIsHardlink("link4a", "link4b"); + +#ifdef HAVE_LINKAT + if (can_symlink) { + /* Test #5 */ + assertIsSymlink("link5a", "foo", 0); + assertFileNLinks("link5a", 2); + assertIsHardlink("link5a", "link5b"); + } +#endif #endif } -- 2.47.2