commit 5ede16e21ae75c664b72b6cf762b8f9d00f49c87
Author:     Jacob Welsh <jfw>
AuthorDate: Thu Apr 18 21:36:55 2024 +0000
Commit:     Jacob Welsh <jfw>
CommitDate: Thu Apr 18 21:36:55 2024 +0000

    busybox/archival: add notes on possible symlink attacks & prevention

diff --git a/base/busybox/archival/libarchive/data_extract_all.c b/base/busybox/archival/libarchive/data_extract_all.c
index fc5fbc1..ef3510e 100644
--- a/base/busybox/archival/libarchive/data_extract_all.c
+++ b/base/busybox/archival/libarchive/data_extract_all.c
@@ -26,6 +26,7 @@ void FAST_FUNC data_extract_all(archive_handle_t *archive_handle)
 	}
 #endif
 
+	/* TODO since we're doing a parent stat, check for symlink attacks */
 	if (archive_handle->ah_flags & (ARCHIVE_CREATE_LEADING_DIRS | ARCHIVE_RESTORE_DATE)) {
 		slash = strrchr(file_header->name, '/');
 		if (slash) {
@@ -33,7 +34,7 @@ void FAST_FUNC data_extract_all(archive_handle_t *archive_handle)
 			*slash = '\0';
 			if (stat(file_header->name, &parent_stat) == -1) {
 				if (errno == ENOENT && (archive_handle->ah_flags & ARCHIVE_CREATE_LEADING_DIRS)) {
-					/* XXX doesn't preserve parent timestamps */
+					/* XXX doesn't preserve parent timestamps (or perhaps check for symlinks) */
 					bb_make_directory(file_header->name, -1, FILEUTILS_RECUR);
 				}
 			} else if (archive_handle->ah_flags & ARCHIVE_RESTORE_DATE) {
@@ -125,6 +126,7 @@ void FAST_FUNC data_extract_all(archive_handle_t *archive_handle)
 		char *dst_name;
 		int flags = O_WRONLY | O_CREAT | O_EXCL;
 		if (archive_handle->ah_flags & ARCHIVE_O_TRUNC)
+			/* TODO symlink attack safety (eg tar file first lists a symlink, then a regular file of the same name, causing us to truncate & overwrite an arbitrary file) */
 			flags = O_WRONLY | O_CREAT | O_TRUNC;
 		dst_name = file_header->name;
 #ifdef ARCHIVE_REPLACE_VIA_RENAME
diff --git a/base/busybox/archival/tar.c b/base/busybox/archival/tar.c
index 8901594..9e6571e 100644
--- a/base/busybox/archival/tar.c
+++ b/base/busybox/archival/tar.c
@@ -39,6 +39,8 @@
  *
  * 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.
+ *
+ * XXX actually this doesn't seem to have anything to do with -C DESTDIR; we're always vulnerable.
  */
 
 //config:config TAR