commit f829452e38ccd6814c1fae271bc42b00e5f41260
Author:     Jacob Welsh <jfw>
AuthorDate: Sat Apr 27 01:15:58 2024 +0000
Commit:     Jacob Welsh <jfw>
CommitDate: Sat Apr 27 01:27:11 2024 +0000

    busybox/archival: BSD/POSIX compatibility for directory unlink error handling and hard link creation.

diff --git a/base/busybox/archival/libarchive/data_extract_all.c b/base/busybox/archival/libarchive/data_extract_all.c
index cf8da99..4c401b2 100644
--- a/base/busybox/archival/libarchive/data_extract_all.c
+++ b/base/busybox/archival/libarchive/data_extract_all.c
@@ -6,6 +6,23 @@
 #include "libbb.h"
 #include "bb_archive.h"
 
+/* Emulate Linux behavior for systems with the POSIX behavior that unlinking a directory returns the ambiguous EPERM (this includes BSDs).
+ * Even worse, UFS on Solaris reportedly allows root to unlink non-empty directories, leading to orphaned files. Dealing with that would seem to require an lstat pre-check, which could still fail as a race condition.
+ * TODO see if this is applicable elsewhere in BB.
+ */
+static int bb_unlink(char const *path)
+{
+	int ret = unlink(path);
+	if (ret != 0 && errno == EPERM) {
+		if (is_directory(path, 0)) {
+			errno = EISDIR;
+		} else {
+			errno = EPERM; /* needs restoring after the is_directory call */
+		}
+	}
+	return ret;
+}
+
 /* Like bb_copyfd_exact_size but without death on error. */
 static int copyfd_exact_size(int src, int dst, off_t size)
 {
@@ -235,10 +252,8 @@ void FAST_FUNC data_extract_all(archive_handle_t *archive_handle)
 				goto ret;
 		}
 		/* Proceed with deleting */
-		/* Note 1: here and later we assume the Linux behavior that unlink on a directory fails with EISDIR. BSDs and POSIX don't distinguish this from EPERM, while UFS on Solaris reportedly allows root to unlink even non-empty directories. Accomodating these would require some more complex sequence.
-		 * Note 2: skipping the unlink when S_ISDIR(file_header->mode), as formerly done here, is not a valid substitute because it fails to replace an existing non-directory file. Although it doesn't seem to directly open a symlink attack due to the earlier path check.
-		 */
-		if (unlink(file_header->name) == -1
+		/* Note: skipping the unlink when S_ISDIR(file_header->mode), as formerly done here, is not valid because it fails to replace an existing non-directory file. Although it doesn't seem to directly open a symlink attack due to the earlier path check. */
+		if (bb_unlink(file_header->name) == -1
 		 && errno != ENOENT
 		 && !(errno == EISDIR && S_ISDIR(file_header->mode))
 		) {
@@ -272,7 +287,7 @@ void FAST_FUNC data_extract_all(archive_handle_t *archive_handle)
 			 * In general though we're not bothering about such conditions, so security will depend on only one process creating entries based on untrusted data in the same directory at the same time.
 			 * This could perhaps be improved by walking the path using openat() with O_NOFOLLOW and doing the rest relative to the resulting fd.
 			 */
-			if (unlink(file_header->name) == -1
+			if (bb_unlink(file_header->name) == -1
 			 && errno != ENOENT
 			 && !(errno == EISDIR && S_ISDIR(file_header->mode))
 			) {
@@ -307,7 +322,9 @@ void FAST_FUNC data_extract_all(archive_handle_t *archive_handle)
 
 		/* We encode hard links as regular files of size 0 with a symlink */
 		if (file_header->link_target && file_header->size == 0) {
-			if (link(file_header->link_target, file_header->name) == -1) {
+			/* Using linkat() with no AT_SYMLINK_FOLLOW flag specifies more clearly that we want the Linux link() behavior, i.e. symlinks not followed. */
+			if (linkat(AT_FDCWD, file_header->link_target,
+						AT_FDCWD, file_header->name, 0) == -1) {
 				bb_perror_msg("can't create %slink from %s to %s", "hard",
 						file_header->name,
 						file_header->link_target);