Attachment 203232 Details for Bug 236870 – libarchive directory traversal patch

[patch] libarchive directory traversal patch

la-fix.patch (text/plain), 3.76 KB, created by Andrew "RhodiumToad" Gierth on 2019-03-29 05:56:58 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew "RhodiumToad" Gierth

Created: 2019-03-29 05:56:58 UTC

Size: 3.76 KB

patch

obsolete

>Index: libarchive/archive_read_disk_posix.c
>===================================================================
>--- libarchive/archive_read_disk_posix.c	(revision 344980)
>+++ libarchive/archive_read_disk_posix.c	(working copy)
>@@ -1295,10 +1295,22 @@
> 	if (t->visit_type != TREE_REGULAR || !t->descend)
> 		return (ARCHIVE_OK);
> 
>+	/*
>+	 * We must not treat "." as a physical dir, because if we do then we
>+	 * will try and ascend out of it by opening ".." which is (a) wrong
>+	 * and (b) causes spurious permissions errors if ".." is not readable
>+	 * by us. Instead, treat it as if it were a symlink. (This uses an
>+	 * extra fd, but it can only happen once at the top level of a
>+	 * traverse.) But we can't necessarily assume t->st is valid for "."
>+	 * (though t->lst is), which complicates the logic here a little.
>+	 */
> 	if (tree_current_is_physical_dir(t)) {
> 		tree_push(t, t->basename, t->current_filesystem_id,
> 		    t->lst.st_dev, t->lst.st_ino, &t->restore_time);
>-		t->stack->flags |= isDir;
>+		if (strcmp(t->basename, ".") != 0)
>+			t->stack->flags |= isDir;
>+		else
>+			t->stack->flags |= isDirLink;
> 	} else if (tree_current_is_dir(t)) {
> 		tree_push(t, t->basename, t->current_filesystem_id,
> 		    t->st.st_dev, t->st.st_ino, &t->restore_time);
>Index: libarchive/test/test_read_disk_directory_traversals.c
>===================================================================
>--- libarchive/test/test_read_disk_directory_traversals.c	(revision 344980)
>+++ libarchive/test/test_read_disk_directory_traversals.c	(working copy)
>@@ -1567,6 +1567,73 @@
> 	archive_entry_free(ae);
> }
> 
>+static void
>+test_parent(void)
>+{
>+	struct archive *a;
>+	struct archive_entry *ae;
>+	const void *p;
>+	size_t size;
>+	int64_t offset;
>+	int file_count;
>+
>+	assertMakeDir("lock", 0311);
>+	assertMakeDir("lock/dir1", 0755);
>+	assertMakeFile("lock/dir1/f1", 0644, "0123456789");
>+	assertChdir("lock/dir1");
>+
>+	assert((ae = archive_entry_new()) != NULL);
>+	assert((a = archive_read_disk_new()) != NULL);
>+
>+	/*
>+	 * Test1: Traverse .
>+	 */
>+	failure("Directory traversals should work as well");
>+	assertEqualIntA(a, ARCHIVE_OK, archive_read_disk_open(a, "."));
>+
>+	file_count = 2;
>+	while (file_count--) {
>+		archive_entry_clear(ae);
>+		assertEqualIntA(a, ARCHIVE_OK, archive_read_next_header2(a, ae));
>+		if (strcmp(archive_entry_pathname(ae), ".") == 0) {
>+			assertEqualInt(archive_entry_filetype(ae), AE_IFDIR);
>+		} else if (strcmp(archive_entry_pathname(ae), "./f1") == 0) {
>+			assertEqualInt(archive_entry_filetype(ae), AE_IFREG);
>+			assertEqualInt(archive_entry_size(ae), 10);
>+			assertEqualIntA(a, ARCHIVE_OK,
>+			    archive_read_data_block(a, &p, &size, &offset));
>+			assertEqualInt((int)size, 10);
>+			assertEqualInt((int)offset, 0);
>+			assertEqualMem(p, "0123456789", 10);
>+			assertEqualInt(ARCHIVE_EOF,
>+			    archive_read_data_block(a, &p, &size, &offset));
>+			assertEqualInt((int)size, 0);
>+			assertEqualInt((int)offset, 10);
>+		}
>+		if (archive_read_disk_can_descend(a)) {
>+			/* Descend into the current object */
>+			assertEqualIntA(a, ARCHIVE_OK,
>+			    archive_read_disk_descend(a));
>+		}
>+	}
>+	/*
>+	 * There is no entry. This will however fail if the directory traverse
>+	 * tries to ascend past the initial directory, since it lacks permission
>+	 * to do so.
>+	 */
>+	failure("There should be no entry and no error");
>+	assertEqualIntA(a, ARCHIVE_EOF, archive_read_next_header2(a, ae));
>+
>+	/* Close the disk object. */
>+	assertEqualInt(ARCHIVE_OK, archive_read_close(a));
>+
>+	assertChdir("../..");
>+
>+	/* Destroy the disk object. */
>+	assertEqualInt(ARCHIVE_OK, archive_read_free(a));
>+	archive_entry_free(ae);
>+}
>+
> DEFINE_TEST(test_read_disk_directory_traversals)
> {
> 	/* Basic test. */
>@@ -1583,4 +1650,6 @@
> 	test_callbacks();
> 	/* Test nodump. */
> 	test_nodump();
>+	/* Test parent overshoot. */
>+	test_parent();
> }

Actions: View | Diff

Attachments on bug 236870: 203232 | 203234