Bug#802702: CVE-2011-5325: busybox: Directory traversal via crafted tar file which contains a symlink pointing outside of the current directory

[Chris Lamb <lamby@debian.org>]

tags 802702 + patch
forwarded 802702 https://bugs.busybox.net/show_bug.cgi?id=8411#c2
thanks

Patch attached & sent upstream.


Regards,

-- 
      ,''`.
     : :'  :     Chris Lamb
     `. `'`      lamby@debian.org / chris-lamb.co.uk
       `-

diff --git a/archival/libarchive/data_extract_all.c b/archival/libarchive/data_extract_all.c
index 45776dc..534c7fc 100644
--- a/archival/libarchive/data_extract_all.c
+++ b/archival/libarchive/data_extract_all.c
@@ -142,7 +142,40 @@ void FAST_FUNC data_extract_all(archive_handle_t *archive_handle)
 		break;
 	case S_IFLNK:
 		/* Symlink */
-//TODO: what if file_header->link_target == NULL (say, corrupted tarball?)
+
+		/* To avoid a directory traversal attack via symlinks, for
+		 * certain link targets first write placeholder regular files
+		 * containing the intended target and mark to replace them
+		 * later.
+		 *
+		 * For example, consider a .tar created via:
+		 *
+		 *  $ tar cvf bug.tar anything.txt
+		 *  $ ln -s /tmp symlink
+		 *  $ tar --append -f bug.tar symlink
+		 *  $ rm symlink
+		 *  $ mkdir symlink
+		 *  $ tar --append -f bug.tar symlink/evil.py
+		 *
+		 * This will result in an archive that contains:
+		 *
+		 *  $ tar --list -f bug.tar
+		 *  anything.txt
+		 *  symlink [-> /tmp]
+		 *  symlink/evil.py
+		 *
+		 * Untarring bug.tar would otherwise place evil.py in '/tmp'.
+		*/
+		if ((!strncmp(file_header->link_target, "/", 1))
+		 || (strstr(file_header->link_target, "../"))) {
+			dst_fd = xopen(file_header->name, O_WRONLY | O_CREAT | O_EXCL);
+			xwrite(dst_fd, file_header->link_target,
+					strlen(file_header->link_target));
+			close(dst_fd);
+			llist_add_to_end(&archive_handle->symlink_placeholders,
+					strdup(file_header->name));
+			break;
+		}
 		res = symlink(file_header->link_target, file_header->name);
 		if ((res == -1)
 		 && !(archive_handle->ah_flags & ARCHIVE_EXTRACT_QUIET)
diff --git a/archival/tar.c b/archival/tar.c
index bd61abd..49fc93f 100644
--- a/archival/tar.c
+++ b/archival/tar.c
@@ -22,24 +22,6 @@
  *
  * Licensed under GPLv2 or later, see file LICENSE in this source tree.
  */
-/* TODO: security with -C DESTDIR option can be enhanced.
- * Consider tar file created via:
- * $ tar cvf bug.tar anything.txt
- * $ ln -s /tmp symlink
- * $ tar --append -f bug.tar symlink
- * $ rm symlink
- * $ mkdir symlink
- * $ tar --append -f bug.tar symlink/evil.py
- *
- * This will result in an archive which contains:
- * $ tar --list -f bug.tar
- * anything.txt
- * symlink
- * symlink/evil.py
- *
- * Untarring it puts evil.py in '/tmp' even if the -C DESTDIR is given.
- * This doesn't feel right, and IIRC GNU tar doesn't do that.
- */
 
 //config:config TAR
 //config:	bool "tar"
@@ -309,6 +291,20 @@ static void chksum_and_xwrite(int fd, struct tar_header_t* hp)
 	xwrite(fd, hp, sizeof(*hp));
 }
 
+static void replace_symlink_placeholders(llist_t *list) {
+	while (list) {
+		char *linkpath = list->data;
+		char *target = xmalloc_xopen_read_close(linkpath, NULL);
+		if (unlink(linkpath) == -1)
+			bb_error_msg_and_die("can't remove placeholder %s",
+				linkpath);
+		if (symlink(target, linkpath) == -1)
+			bb_error_msg_and_die("can't create link: %s -> %s",
+				linkpath, target);
+		list = list->link;
+	}
+}
+
 #if ENABLE_FEATURE_TAR_GNU_EXTENSIONS
 static void writeLongname(int fd, int type, const char *name, int dir)
 {
@@ -1205,6 +1201,8 @@ int tar_main(int argc UNUSED_PARAM, char **argv)
 	while (get_header_tar(tar_handle) == EXIT_SUCCESS)
 		bb_got_signal = EXIT_SUCCESS; /* saw at least one header, good */
 
+	replace_symlink_placeholders(tar_handle->symlink_placeholders);
+
 	/* Check that every file that should have been extracted was */
 	while (tar_handle->accept) {
 		if (!find_list_entry(tar_handle->reject, tar_handle->accept->data)
diff --git a/include/bb_archive.h b/include/bb_archive.h
index b82cfd8..bcaed7f 100644
--- a/include/bb_archive.h
+++ b/include/bb_archive.h
@@ -64,6 +64,9 @@ typedef struct archive_handle_t {
 	/* Currently processed file's header */
 	file_header_t *file_header;
 
+	/* List of symlink placeholders */
+	llist_t *symlink_placeholders;
+
 	/* Process the header component, e.g. tar -t */
 	void FAST_FUNC (*action_header)(const file_header_t *);
 
@@ -182,6 +185,7 @@ char get_header_ar(archive_handle_t *archive_handle) FAST_FUNC;
 char get_header_cpio(archive_handle_t *archive_handle) FAST_FUNC;
 char get_header_tar(archive_handle_t *archive_handle) FAST_FUNC;
 char get_header_tar_gz(archive_handle_t *archive_handle) FAST_FUNC;
+char get_header_tar_xz(archive_handle_t *archive_handle) FAST_FUNC;
 char get_header_tar_bz2(archive_handle_t *archive_handle) FAST_FUNC;
 char get_header_tar_lzma(archive_handle_t *archive_handle) FAST_FUNC;