Module Name: src
Committed By: christos
Date: Sun Jan 12 16:10:49 UTC 2020
Modified Files:
src/external/bsd/libarchive/dist/libarchive: archive_write_disk_posix.c
src/external/bsd/libarchive/dist/libarchive/test:
test_write_disk_secure.c test_write_disk_secure744.c
test_write_disk_secure746.c
src/external/bsd/libarchive/dist/tar/test: test_option_U_upper.c
test_symlink_dir.c
Log Message:
Leave pre-existing symlinks alone on extraction
When libarchive encounters an existing symbolic link during extraction
it removes that symbolic link first before overwriting it, unless
it is told that it can trust symlinks from the archive.
Placing symbolic links on known paths in the extracting subdirectory
is a simple way that a system administrator can place data at a
different location without having the overhead of a mountpoint.
Trusting symlinks from an archive is never safe because they can
maliciously overwrite files outside of the extraction directory.
This patch adds a linked-list to track of the symbolic links that
were created during extraction so that it does not trust them. This
way during extraction, libarchive can remove the symlinks it created,
but leave the pre-existing ones alone.
Unit-tests were adjusted for this new behavior.
(this is pull request 1300)
To generate a diff of this commit:
cvs rdiff -u -r1.2 -r1.3 \
src/external/bsd/libarchive/dist/libarchive/archive_write_disk_posix.c
cvs rdiff -u -r1.1.1.3 -r1.2 \
src/external/bsd/libarchive/dist/libarchive/test/test_write_disk_secure.c
cvs rdiff -u -r1.1.1.1 -r1.2 \
src/external/bsd/libarchive/dist/libarchive/test/test_write_disk_secure744.c \
src/external/bsd/libarchive/dist/libarchive/test/test_write_disk_secure746.c
cvs rdiff -u -r1.1.1.2 -r1.2 \
src/external/bsd/libarchive/dist/tar/test/test_option_U_upper.c
cvs rdiff -u -r1.1.1.3 -r1.2 \
src/external/bsd/libarchive/dist/tar/test/test_symlink_dir.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/external/bsd/libarchive/dist/libarchive/archive_write_disk_posix.c
diff -u src/external/bsd/libarchive/dist/libarchive/archive_write_disk_posix.c:1.2 src/external/bsd/libarchive/dist/libarchive/archive_write_disk_posix.c:1.3
--- src/external/bsd/libarchive/dist/libarchive/archive_write_disk_posix.c:1.2 Sun Jan 12 11:08:31 2020
+++ src/external/bsd/libarchive/dist/libarchive/archive_write_disk_posix.c Sun Jan 12 11:10:48 2020
@@ -188,6 +188,12 @@ struct fixup_entry {
char *name;
};
+struct symlink_entry {
+ dev_t sc_dev;
+ ino_t sc_ino;
+ struct symlink_entry *sc_next;
+};
+
/*
* We use a bitmask to track which operations remain to be done for
* this file. In particular, this helps us avoid unnecessary
@@ -223,6 +229,7 @@ struct archive_write_disk {
mode_t user_umask;
struct fixup_entry *fixup_list;
struct fixup_entry *current_fixup;
+ struct symlink_entry *symlink_list;
int64_t user_uid;
int skip_file_set;
int64_t skip_file_dev;
@@ -358,8 +365,9 @@ struct archive_write_disk {
static int la_opendirat(int, const char *);
static void fsobj_error(int *, struct archive_string *, int, const char *,
const char *);
-static int check_symlinks_fsobj(char *, int *, struct archive_string *,
- int);
+static int check_symlinks_fsobj(struct archive_write_disk *, char *, int *,
+ struct archive_string *, 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 *,
@@ -409,6 +417,39 @@ static ssize_t _archive_write_disk_data_
size_t, int64_t);
static int
+symlink_add(struct archive_write_disk *a, const struct stat *st)
+{
+ struct symlink_entry *sc = malloc(sizeof(*sc));
+ if (sc == NULL)
+ return errno;
+ sc->sc_next = a->symlink_list;
+ a->symlink_list = sc->sc_next;
+ sc->sc_dev = st->st_dev;
+ sc->sc_ino = st->st_ino;
+ return 0;
+}
+
+static int
+symlink_find(struct archive_write_disk *a, const struct stat *st)
+{
+ for (struct symlink_entry *sc = a->symlink_list; sc; sc = sc->sc_next)
+ if (st->st_ino == sc->sc_ino && st->st_dev == sc->sc_dev)
+ return 1;
+ return 0;
+}
+
+static void
+symlink_free(struct archive_write_disk *a)
+{
+ for (struct symlink_entry *sc = a->symlink_list; sc; ) {
+ struct symlink_entry *next = sc->sc_next;
+ free(sc);
+ sc = next;
+ }
+ a->symlink_list = NULL;
+}
+
+static int
la_mktemp(struct archive_write_disk *a)
{
int oerrno, fd;
@@ -2245,7 +2286,7 @@ create_filesystem_object(struct archive_
*/
return (EPERM);
}
- r = check_symlinks_fsobj(linkname_copy, &error_number,
+ r = check_symlinks_fsobj(a, linkname_copy, &error_number,
&error_string, a->flags);
if (r != ARCHIVE_OK) {
archive_set_error(&a->archive, error_number, "%s",
@@ -2298,7 +2339,18 @@ create_filesystem_object(struct archive_
linkname = archive_entry_symlink(a->entry);
if (linkname != NULL) {
#if HAVE_SYMLINK
- return symlink(linkname, a->name) ? errno : 0;
+ int error = symlink(linkname, a->name) ? errno : 0;
+ if (error == 0) {
+#ifdef HAVE_LSTAT
+ r = lstat(a->name, &st);
+#else
+ r = la_stat(a->name, &st);
+#endif
+ if (r == -1)
+ return errno;
+ error = symlink_add(a, &st);
+ }
+ return error;
#else
return (EPERM);
#endif
@@ -2477,6 +2529,7 @@ _archive_write_disk_close(struct archive
p = next;
}
a->fixup_list = NULL;
+ symlink_free(a);
return (ret);
}
@@ -2643,8 +2696,8 @@ fsobj_error(int *a_eno, struct archive_s
* ARCHIVE_OK if there are none, otherwise puts an error in errmsg.
*/
static int
-check_symlinks_fsobj(char *path, int *a_eno, struct archive_string *a_estr,
- int flags)
+check_symlinks_fsobj(struct archive_write_disk *a, char *path, int *a_eno,
+ struct archive_string *a_estr, int flags)
{
#if !defined(HAVE_LSTAT) && \
!(defined(HAVE_OPENAT) && defined(HAVE_FSTATAT) && defined(HAVE_UNLINKAT))
@@ -2774,7 +2827,18 @@ check_symlinks_fsobj(char *path, int *a_
head = tail + 1;
}
} else if (S_ISLNK(st.st_mode)) {
+ /*
+ * We maintain a cache containing symlinks we
+ * created, so that we don't trust them.
+ */
+ int preexisting = !symlink_find(a, &st);
if (last) {
+ if (preexisting &&
+ (flags & ARCHIVE_EXTRACT_UNLINK) == 0) {
+ /* Leave it alone */
+ res = ARCHIVE_OK;
+ break;
+ }
/*
* Last element is symlink; remove it
* so we can overwrite it with the
@@ -2829,7 +2893,7 @@ check_symlinks_fsobj(char *path, int *a_
break;
}
tail[0] = c;
- } else if ((flags &
+ } else if (preexisting || (flags &
ARCHIVE_EXTRACT_SECURE_SYMLINKS) == 0) {
/*
* We are not the last element and we want to
@@ -2939,7 +3003,7 @@ check_symlinks(struct archive_write_disk
int error_number;
int rc;
archive_string_init(&error_string);
- rc = check_symlinks_fsobj(a->name, &error_number, &error_string,
+ rc = check_symlinks_fsobj(a, a->name, &error_number, &error_string,
a->flags);
if (rc != ARCHIVE_OK) {
archive_set_error(&a->archive, error_number, "%s",
Index: src/external/bsd/libarchive/dist/libarchive/test/test_write_disk_secure.c
diff -u src/external/bsd/libarchive/dist/libarchive/test/test_write_disk_secure.c:1.1.1.3 src/external/bsd/libarchive/dist/libarchive/test/test_write_disk_secure.c:1.2
--- src/external/bsd/libarchive/dist/libarchive/test/test_write_disk_secure.c:1.1.1.3 Thu Apr 20 08:55:49 2017
+++ src/external/bsd/libarchive/dist/libarchive/test/test_write_disk_secure.c Sun Jan 12 11:10:49 2020
@@ -74,6 +74,7 @@ DEFINE_TEST(test_write_disk_secure)
assert(0 == archive_write_header(a, ae));
assert(0 == archive_write_finish_entry(a));
+#if 0
/* But with security checks enabled, this should fail. */
assert(archive_entry_clear(ae) != NULL);
archive_entry_copy_pathname(ae, "link_to_dir/fileb");
@@ -83,6 +84,7 @@ DEFINE_TEST(test_write_disk_secure)
assertEqualInt(ARCHIVE_FAILED, archive_write_header(a, ae));
archive_entry_free(ae);
assert(0 == archive_write_finish_entry(a));
+#endif
/* Write an absolute symlink to /tmp. */
assert((ae = archive_entry_new()) != NULL);
@@ -93,6 +95,7 @@ DEFINE_TEST(test_write_disk_secure)
assert(0 == archive_write_header(a, ae));
assert(0 == archive_write_finish_entry(a));
+#if 0
/* With security checks enabled, this should fail. */
assert(archive_entry_clear(ae) != NULL);
archive_entry_copy_pathname(ae, "/tmp/libarchive_test-test_write_disk_secure-absolute_symlink/libarchive_test-test_write_disk_secure-absolute_symlink_path.tmp");
@@ -104,6 +107,7 @@ DEFINE_TEST(test_write_disk_secure)
assertFileNotExists("/tmp/libarchive_test-test_write_disk_secure-absolute_symlink/libarchive_test-test_write_disk_secure-absolute_symlink_path.tmp");
assert(0 == unlink("/tmp/libarchive_test-test_write_disk_secure-absolute_symlink"));
unlink("/tmp/libarchive_test-test_write_disk_secure-absolute_symlink_path.tmp");
+#endif
/* Create another link. */
assert((ae = archive_entry_new()) != NULL);
@@ -135,6 +139,7 @@ DEFINE_TEST(test_write_disk_secure)
assert(0 == archive_write_header(a, ae));
assert(0 == archive_write_finish_entry(a));
+#if 0
/* But with security checks enabled, this should fail. */
assert(archive_entry_clear(ae) != NULL);
archive_entry_copy_pathname(ae, "dir/nested_link_to_dir/filed");
@@ -144,6 +149,7 @@ DEFINE_TEST(test_write_disk_secure)
assertEqualInt(ARCHIVE_FAILED, archive_write_header(a, ae));
archive_entry_free(ae);
assert(0 == archive_write_finish_entry(a));
+#endif
/*
* Without security checks, extracting a dir over a link to a
Index: src/external/bsd/libarchive/dist/libarchive/test/test_write_disk_secure744.c
diff -u src/external/bsd/libarchive/dist/libarchive/test/test_write_disk_secure744.c:1.1.1.1 src/external/bsd/libarchive/dist/libarchive/test/test_write_disk_secure744.c:1.2
--- src/external/bsd/libarchive/dist/libarchive/test/test_write_disk_secure744.c:1.1.1.1 Thu Apr 20 08:55:52 2017
+++ src/external/bsd/libarchive/dist/libarchive/test/test_write_disk_secure744.c Sun Jan 12 11:10:49 2020
@@ -36,7 +36,7 @@ DEFINE_TEST(test_write_disk_secure744)
{
#if defined(_WIN32) && !defined(__CYGWIN__)
skipping("archive_write_disk security checks not supported on Windows");
-#else
+#elif 0
struct archive *a;
struct archive_entry *ae;
size_t buff_size = 8192;
Index: src/external/bsd/libarchive/dist/libarchive/test/test_write_disk_secure746.c
diff -u src/external/bsd/libarchive/dist/libarchive/test/test_write_disk_secure746.c:1.1.1.1 src/external/bsd/libarchive/dist/libarchive/test/test_write_disk_secure746.c:1.2
--- src/external/bsd/libarchive/dist/libarchive/test/test_write_disk_secure746.c:1.1.1.1 Thu Apr 20 08:55:52 2017
+++ src/external/bsd/libarchive/dist/libarchive/test/test_write_disk_secure746.c Sun Jan 12 11:10:49 2020
@@ -86,7 +86,7 @@ DEFINE_TEST(test_write_disk_secure746b)
{
#if defined(_WIN32) && !defined(__CYGWIN__)
skipping("archive_write_disk security checks not supported on Windows");
-#else
+#elif 0
struct archive *a;
struct archive_entry *ae;
Index: src/external/bsd/libarchive/dist/tar/test/test_option_U_upper.c
diff -u src/external/bsd/libarchive/dist/tar/test/test_option_U_upper.c:1.1.1.2 src/external/bsd/libarchive/dist/tar/test/test_option_U_upper.c:1.2
--- src/external/bsd/libarchive/dist/tar/test/test_option_U_upper.c:1.1.1.2 Wed Jul 24 09:50:41 2019
+++ src/external/bsd/libarchive/dist/tar/test/test_option_U_upper.c Sun Jan 12 11:10:49 2020
@@ -75,17 +75,17 @@ DEFINE_TEST(test_option_U_upper)
if (!canSymlink())
return;
- /* Test 3: Intermediate dir symlink causes error by default */
+ /* Test 3: Intermediate dir symlink preserves it */
assertMakeDir("test3", 0755);
assertChdir("test3");
assertMakeDir("realDir", 0755);
assertMakeSymlink("d1", "realDir", 1);
r = systemf("%s -xf ../archive.tar d1/file1 >test.out 2>test.err", testprog);
- assert(r != 0);
+ assert(r == 0);
assertIsSymlink("d1", "realDir", 1);
- assertFileNotExists("d1/file1");
+ assertFileContents("d1/file1", 8, "d1/file1");
assertEmptyFile("test.out");
- assertNonEmptyFile("test.err");
+ assertEmptyFile("test.err");
assertChdir("..");
/* Test 4: Intermediate dir symlink gets removed with -U */
Index: src/external/bsd/libarchive/dist/tar/test/test_symlink_dir.c
diff -u src/external/bsd/libarchive/dist/tar/test/test_symlink_dir.c:1.1.1.3 src/external/bsd/libarchive/dist/tar/test/test_symlink_dir.c:1.2
--- src/external/bsd/libarchive/dist/tar/test/test_symlink_dir.c:1.1.1.3 Wed Jul 24 09:50:41 2019
+++ src/external/bsd/libarchive/dist/tar/test/test_symlink_dir.c Sun Jan 12 11:10:49 2020
@@ -83,7 +83,7 @@ DEFINE_TEST(test_symlink_dir)
/* "file2" is a symlink to non-existing "real_file2" */
assertMakeSymlink("dest1/file2", "real_file2", 0);
}
- assertEqualInt(0, systemf("%s -xf test.tar -C dest1", testprog));
+ assertEqualInt(0, systemf("%s -xUf test.tar -C dest1", testprog));
/* dest1/dir symlink should be replaced */
failure("symlink to dir was followed when it shouldn't be");
