CVE-2015-2304: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: Absolute path traversal vulnerability in bsdcpio in libarchive 3.1.2 and earlier allows remote attackers to write to arbitrary files via a full pathname in an archive.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
Published: 2015-03-15T19:59Z
Repository: https://github.com/libarchive/libarchive
Language: C
Files changed: cpio/bsdcpio.1, cpio/cpio.c, libarchive/archive.h, libarchive/archive_write_disk.3, libarchive/archive_write_disk_posix.c, libarchive/test/test_write_disk_secure.c
Commit date: 2015-03-04 18:44:03 +0100
Commit message: Add ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS option

This fixes a directory traversal in the cpio tool.

Diff: diff --git a/cpio/bsdcpio.1 b/cpio/bsdcpio.1
index f966aa06..e52546e6 100644
--- a/cpio/bsdcpio.1
+++ b/cpio/bsdcpio.1
@@ -154,11 +154,12 @@ Read archive from
 Input mode.
 See above for description.
 .It Fl Fl insecure
 (i and p mode only)
 Disable security checks during extraction or copying.
-This allows extraction via symbolic links and path names containing
+This allows extraction via symbolic links, absolute paths,
+and path names containing
 .Sq ..
 in the name.
 .It Fl J , Fl Fl xz
 (o mode only)
 Compress the file with xz-compatible compression before writing it.
diff --git a/cpio/cpio.c b/cpio/cpio.c
index 0acde119..b267e9b8 100644
--- a/cpio/cpio.c
+++ b/cpio/cpio.c
@@ -169,10 +169,11 @@ main(int argc, char *argv[])
 	cpio->compress = '\0';
 	cpio->extract_flags = ARCHIVE_EXTRACT_NO_AUTODIR;
 	cpio->extract_flags |= ARCHIVE_EXTRACT_NO_OVERWRITE_NEWER;
 	cpio->extract_flags |= ARCHIVE_EXTRACT_SECURE_SYMLINKS;
 	cpio->extract_flags |= ARCHIVE_EXTRACT_SECURE_NODOTDOT;
+	cpio->extract_flags |= ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS;
 	cpio->extract_flags |= ARCHIVE_EXTRACT_PERM;
 	cpio->extract_flags |= ARCHIVE_EXTRACT_FFLAGS;
 	cpio->extract_flags |= ARCHIVE_EXTRACT_ACL;
 #if !defined(_WIN32) && !defined(__CYGWIN__)
 	if (geteuid() == 0)
@@ -254,10 +255,11 @@ main(int argc, char *argv[])
 			cpio->compress = opt;
 			break;
 		case OPTION_INSECURE:
 			cpio->extract_flags &= ~ARCHIVE_EXTRACT_SECURE_SYMLINKS;
 			cpio->extract_flags &= ~ARCHIVE_EXTRACT_SECURE_NODOTDOT;
+			cpio->extract_flags &= ~ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS;
 			break;
 		case 'L': /* GNU cpio */
 			cpio->option_follow_links = 1;
 			break;
 		case 'l': /* POSIX 1997 */
diff --git a/libarchive/archive.h b/libarchive/archive.h
index 1f0fc387..ef635ac9 100644
--- a/libarchive/archive.h
+++ b/libarchive/archive.h
@@ -647,10 +647,12 @@ __LA_DECL int archive_read_set_passphrase_callback(struct archive *,
 /* This has no effect except on Mac OS v10.6 or later. */
 #define	ARCHIVE_EXTRACT_NO_HFS_COMPRESSION	(0x4000)
 /* Default: Do not use HFS+ compression if it was not compressed. */
 /* This has no effect except on Mac OS v10.6 or later. */
 #define	ARCHIVE_EXTRACT_HFS_COMPRESSION_FORCED	(0x8000)
+/* Default: Do not reject entries with absolute paths */
+#define ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS (0x10000)
 
 __LA_DECL int archive_read_extract(struct archive *, struct archive_entry *,
 		     int flags);
 __LA_DECL int archive_read_extract2(struct archive *, struct archive_entry *,
 		     struct archive * /* dest */);
diff --git a/libarchive/archive_write_disk.3 b/libarchive/archive_write_disk.3
index fa925cc5..a2e7afaa 100644
--- a/libarchive/archive_write_disk.3
+++ b/libarchive/archive_write_disk.3
@@ -175,10 +175,13 @@ Refuse to extract a path that contains a
 element anywhere within it.
 The default is to not refuse such paths.
 Note that paths ending in
 .Pa ..
 always cause an error, regardless of this flag.
+.It Cm ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS
+Refuse to extract an absolute path.
+The default is to not refuse such paths.
 .It Cm ARCHIVE_EXTRACT_SPARSE
 Scan data for blocks of NUL bytes and try to recreate them with holes.
 This results in sparse files, independent of whether the archive format
 supports or uses them.
 .El
diff --git a/libarchive/archive_write_disk_posix.c b/libarchive/archive_write_disk_posix.c
index ab3bdace..c1290eb3 100644
--- a/libarchive/archive_write_disk_posix.c
+++ b/libarchive/archive_write_disk_posix.c
@@ -2507,12 +2507,13 @@ cleanup_pathname_win(struct archive_write_disk *a)
 #endif
 
 /*
  * Canonicalize the pathname.  In particular, this strips duplicate
  * '/' characters, '.' elements, and trailing '/'.  It also raises an
- * error for an empty path, a trailing '..' or (if _SECURE_NODOTDOT is
- * set) any '..' in the path.
+ * error for an empty path, a trailing '..', (if _SECURE_NODOTDOT is
+ * set) any '..' in the path or (if ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS
+ * is set) if the path is absolute.
  */
 static int
 cleanup_pathname(struct archive_write_disk *a)
 {
 	char *dest, *src;
@@ -2527,12 +2528,19 @@ cleanup_pathname(struct archive_write_disk *a)
 
 #if defined(__CYGWIN__)
 	cleanup_pathname_win(a);
 #endif
 	/* Skip leading '/'. */
-	if (*src == '/')
+	if (*src == '/') {
+		if (a->flags & ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS) {
+			archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
+			                  "Path is absolute");
+			return (ARCHIVE_FAILED);
+		}
+
 		separator = *src++;
+	}
 
 	/* Scan the pathname one element at a time. */
 	for (;;) {
 		/* src points to first char after '/' */
 		if (src[0] == '\0') {
diff --git a/libarchive/test/test_write_disk_secure.c b/libarchive/test/test_write_disk_secure.c
index 31c5bfd7..2c942068 100644
--- a/libarchive/test/test_write_disk_secure.c
+++ b/libarchive/test/test_write_disk_secure.c
@@ -176,10 +176,33 @@ DEFINE_TEST(test_write_disk_secure)
 	/* Verify link was replaced. */
 	assertEqualInt(0, lstat("link_to_dir5", &st));
 	assert(S_ISDIR(st.st_mode));
 	archive_entry_free(ae);
 
+	/*
+	 * Without security checks, we should be able to
+	 * extract an absolute path.
+	 */
+	assert((ae = archive_entry_new()) != NULL);
+	archive_entry_copy_pathname(ae, "/tmp/libarchive_test-test_write_disk_secure-absolute_path.tmp");
+	archive_entry_set_mode(ae, S_IFREG | 0777);
+	assert(0 == archive_write_header(a, ae));
+	assert(0 == archive_write_finish_entry(a));
+	assertFileExists("/tmp/libarchive_test-test_write_disk_secure-absolute_path.tmp");
+	assert(0 == unlink("/tmp/libarchive_test-test_write_disk_secure-absolute_path.tmp"));
+
+	/* But 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_path.tmp");
+	archive_entry_set_mode(ae, S_IFREG | 0777);
+	archive_write_disk_set_options(a, ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS);
+	failure("Extracting an absolute path should fail here.");
+	assertEqualInt(ARCHIVE_FAILED, archive_write_header(a, ae));
+	archive_entry_free(ae);
+	assert(0 == archive_write_finish_entry(a));
+	assertFileNotExists("/tmp/libarchive_test-test_write_disk_secure-absolute_path.tmp");
+
 	assertEqualInt(ARCHIVE_OK, archive_write_free(a));
 
 	/* Test the entries on disk. */
 	assert(0 == lstat("dir", &st));
 	failure("dir: st.st_mode=%o", st.st_mode);

Vulnerable code: This allows extraction via symbolic links and path names containing
 * error for an empty path, a trailing '..' or (if _SECURE_NODOTDOT is
 * set) any '..' in the path.
	if (*src == '/')
Fixed code: This allows extraction via symbolic links, absolute paths,
and path names containing
	cpio->extract_flags |= ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS;
			cpio->extract_flags &= ~ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS;
/* Default: Do not reject entries with absolute paths */
#define ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS (0x10000)
.It Cm ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS
Refuse to extract an absolute path.
The default is to not refuse such paths.
 * error for an empty path, a trailing '..', (if _SECURE_NODOTDOT is
 * set) any '..' in the path or (if ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS
 * is set) if the path is absolute.
	if (*src == '/') {
		if (a->flags & ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS) {
			archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
			                  "Path is absolute");
			return (ARCHIVE_FAILED);
		}

	}
	/*
	 * Without security checks, we should be able to
	 * extract an absolute path.
	 */
	assert((ae = archive_entry_new()) != NULL);
	archive_entry_copy_pathname(ae, "/tmp/libarchive_test-test_write_disk_secure-absolute_path.tmp");
	archive_entry_set_mode(ae, S_IFREG | 0777);
	assert(0 == archive_write_header(a, ae));
	assert(0 == archive_write_finish_entry(a));
	assertFileExists("/tmp/libarchive_test-test_write_disk_secure-absolute_path.tmp");
	assert(0 == unlink("/tmp/libarchive_test-test_write_disk_secure-absolute_path.tmp"));

	/* But 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_path.tmp");
	archive_entry_set_mode(ae, S_IFREG | 0777);
	archive_write_disk_set_options(a, ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS);
	failure("Extracting an absolute path should fail here.");
	assertEqualInt(ARCHIVE_FAILED, archive_write_header(a, ae));
	archive_entry_free(ae);
	assert(0 == archive_write_finish_entry(a));
	assertFileNotExists("/tmp/libarchive_test-test_write_disk_secure-absolute_path.tmp");