Implement ARCHIVE_EXTRACT_SAFE_WRITES on Windows

As Windows does not support atomic rename/_wrename
we have to unlink the file right before calling _wrename
leaving a short unsafe window where a different file with the
same name can be created and make the rename operation fail.

2 files changed, 157 insertions, 62 deletions

diff --git a/libarchive/archive_util.c b/libarchive/archive_util.c
index dc34adcc..288a4428 100644
--- a/libarchive/archive_util.c
+++ b/libarchive/archive_util.c
@@ -218,8 +218,8 @@ __archive_errx(int retvalue, const char *msg)
  * Also Windows version of mktemp family including _mktemp_s
  * are not secure.
  */
-int
-__archive_mktemp(const char *tmpdir)
+static int
+__archive_mktempx(const char *tmpdir, wchar_t *template)
 {
 	static const wchar_t prefix[] = L"libarchive_";
 	static const wchar_t suffix[] = L"XXXXXXXXXX";
@@ -243,64 +243,76 @@ __archive_mktemp(const char *tmpdir)
 	hProv = (HCRYPTPROV)NULL;
 	fd = -1;
 	ws = NULL;
-	archive_string_init(&temp_name);
 
-	/* Get a temporary directory. */
-	if (tmpdir == NULL) {
-		size_t l;
-		wchar_t *tmp;
+	if (template == NULL) {
+		archive_string_init(&temp_name);
 
-		l = GetTempPathW(0, NULL);
-		if (l == 0) {
-			la_dosmaperr(GetLastError());
-			goto exit_tmpfile;
-		}
-		tmp = malloc(l*sizeof(wchar_t));
-		if (tmp == NULL) {
-			errno = ENOMEM;
-			goto exit_tmpfile;
-		}
-		GetTempPathW((DWORD)l, tmp);
-		archive_wstrcpy(&temp_name, tmp);
-		free(tmp);
-	} else {
-		if (archive_wstring_append_from_mbs(&temp_name, tmpdir,
-		    strlen(tmpdir)) < 0)
-			goto exit_tmpfile;
-		if (temp_name.s[temp_name.length-1] != L'/')
-			archive_wstrappend_wchar(&temp_name, L'/');
-	}
+		/* Get a temporary directory. */
+		if (tmpdir == NULL) {
+			size_t l;
+			wchar_t *tmp;
 
-	/* Check if temp_name is a directory. */
-	attr = GetFileAttributesW(temp_name.s);
-	if (attr == (DWORD)-1) {
-		if (GetLastError() != ERROR_FILE_NOT_FOUND) {
-			la_dosmaperr(GetLastError());
-			goto exit_tmpfile;
-		}
-		ws = __la_win_permissive_name_w(temp_name.s);
-		if (ws == NULL) {
-			errno = EINVAL;
-			goto exit_tmpfile;
+			l = GetTempPathW(0, NULL);
+			if (l == 0) {
+				la_dosmaperr(GetLastError());
+				goto exit_tmpfile;
+			}
+			tmp = malloc(l*sizeof(wchar_t));
+			if (tmp == NULL) {
+				errno = ENOMEM;
+				goto exit_tmpfile;
+			}
+			GetTempPathW((DWORD)l, tmp);
+			archive_wstrcpy(&temp_name, tmp);
+			free(tmp);
+		} else {
+			if (archive_wstring_append_from_mbs(&temp_name, tmpdir,
+			    strlen(tmpdir)) < 0)
+				goto exit_tmpfile;
+			if (temp_name.s[temp_name.length-1] != L'/')
+				archive_wstrappend_wchar(&temp_name, L'/');
 		}
-		attr = GetFileAttributesW(ws);
+
+		/* Check if temp_name is a directory. */
+		attr = GetFileAttributesW(temp_name.s);
 		if (attr == (DWORD)-1) {
-			la_dosmaperr(GetLastError());
+			if (GetLastError() != ERROR_FILE_NOT_FOUND) {
+				la_dosmaperr(GetLastError());
+				goto exit_tmpfile;
+			}
+			ws = __la_win_permissive_name_w(temp_name.s);
+			if (ws == NULL) {
+				errno = EINVAL;
+				goto exit_tmpfile;
+			}
+			attr = GetFileAttributesW(ws);
+			if (attr == (DWORD)-1) {
+				la_dosmaperr(GetLastError());
+				goto exit_tmpfile;
+			}
+		}
+		if (!(attr & FILE_ATTRIBUTE_DIRECTORY)) {
+			errno = ENOTDIR;
 			goto exit_tmpfile;
 		}
-	}
-	if (!(attr & FILE_ATTRIBUTE_DIRECTORY)) {
-		errno = ENOTDIR;
-		goto exit_tmpfile;
-	}
 
-	/*
-	 * Create a temporary file.
-	 */
-	archive_wstrcat(&temp_name, prefix);
-	archive_wstrcat(&temp_name, suffix);
-	ep = temp_name.s + archive_strlen(&temp_name);
-	xp = ep - wcslen(suffix);
+		/*
+		 * Create a temporary file.
+		 */
+		archive_wstrcat(&temp_name, prefix);
+		archive_wstrcat(&temp_name, suffix);
+		ep = temp_name.s + archive_strlen(&temp_name);
+		xp = ep - wcslen(suffix);
+		template = temp_name.s;
+	} else {
+		xp = wcschr(template, L'X');
+		if (xp == NULL)	/* No X, programming error */
+			abort();
+		for (ep = xp; *ep == L'X'; ep++)
+			continue;
+		if (*ep)	/* X followed by non X, programming error */
+			abort();
+	}
 
 	if (!CryptAcquireContext(&hProv, NULL, NULL, PROV_RSA_FULL,
 		CRYPT_VERIFYCONTEXT)) {
@@ -323,20 +335,24 @@ __archive_mktemp(const char *tmpdir)
 			*p = num[((DWORD)*p) % (sizeof(num)/sizeof(num[0]))];
 
 		free(ws);
-		ws = __la_win_permissive_name_w(temp_name.s);
+		ws = __la_win_permissive_name_w(template);
 		if (ws == NULL) {
 			errno = EINVAL;
 			goto exit_tmpfile;
 		}
-		/* Specifies FILE_FLAG_DELETE_ON_CLOSE flag is to
-		 * delete this temporary file immediately when this
-		 * file closed. */
+		if (template == temp_name.s) {
+			attr = FILE_ATTRIBUTE_TEMPORARY |
+			       FILE_FLAG_DELETE_ON_CLOSE;
+		} else {
+			/* mkstemp */
+			attr = FILE_ATTRIBUTE_NORMAL;
+		}
 		h = CreateFileW(ws,
 		    GENERIC_READ | GENERIC_WRITE | DELETE,
 		    0,/* Not share */
 		    NULL,
 		    CREATE_NEW,/* Create a new file only */
-		    FILE_ATTRIBUTE_TEMPORARY | FILE_FLAG_DELETE_ON_CLOSE,
+		    attr,
 		    NULL);
 		if (h == INVALID_HANDLE_VALUE) {
 			/* The same file already exists. retry with
@@ -358,10 +374,23 @@ exit_tmpfile:
 	if (hProv != (HCRYPTPROV)NULL)
 		CryptReleaseContext(hProv, 0);
 	free(ws);
-	archive_wstring_free(&temp_name);
+	if (template == temp_name.s)
+		archive_wstring_free(&temp_name);
 	return (fd);
 }
 
+int
+__archive_mktemp(const char *tmpdir)
+{
+	return __archive_mktempx(tmpdir, NULL);
+}
+
+int
+__archive_mkstemp(wchar_t *template)
+{
+	return __archive_mktempx(NULL, template);
+}
+
 #else
 
 static int
diff --git a/libarchive/archive_write_disk_windows.c b/libarchive/archive_write_disk_windows.c
index 8b947304..adf89bc5 100644
--- a/libarchive/archive_write_disk_windows.c
+++ b/libarchive/archive_write_disk_windows.c
@@ -165,6 +165,8 @@ struct archive_write_disk {
 	struct archive_entry	*entry; /* Entry being extracted. */
 	wchar_t			*name; /* Name of entry, possibly edited. */
 	struct archive_wstring	 _name_data; /* backing store for 'name' */
+	wchar_t			*tmpname; /* Temporary name */
+	struct archive_wstring	_tmpname_data; /* backing store for 'tmpname' */
 	/* Tasks remaining for this object. */
 	int			 todo;
 	/* Tasks deferred until end-of-archive. */
@@ -215,6 +217,7 @@ static int	cleanup_pathname(struct archive_write_disk *);
 static int	create_dir(struct archive_write_disk *, wchar_t *);
 static int	create_parent_dir(struct archive_write_disk *, wchar_t *);
 static int	la_chmod(const wchar_t *, mode_t);
+static int	la_mktemp(struct archive_write_disk *);
 static int	older(BY_HANDLE_FILE_INFORMATION *, struct archive_entry *);
 static int	permissive_name_w(struct archive_write_disk *);
 static int	restore_entry(struct archive_write_disk *);
@@ -534,6 +537,28 @@ exit_chmode:
 	return (ret);
 }
 
+static int
+la_mktemp(struct archive_write_disk *a)
+{
+	int fd;
+	mode_t mode;
+
+	archive_wstring_empty(&(a->_tmpname_data));
+	archive_wstrcpy(&(a->_tmpname_data), a->name);
+	archive_wstrcat(&(a->_tmpname_data), L".XXXXXX");
+	a->tmpname = a->_tmpname_data.s;
+
+	fd = __archive_mkstemp(a->tmpname);
+
+	mode = a->mode & 0777 & ~a->user_umask;
+	if (la_chmod(a->tmpname, mode) == -1) {
+		la_dosmaperr(GetLastError());
+		_close(fd);
+		return -1;
+	}
+	return (fd);
+}
+
 static void *
 la_GetFunctionKernel32(const char *name)
 {
@@ -1252,6 +1277,16 @@ _archive_write_disk_finish_entry(struct archive *_a)
 	if (a->fh != INVALID_HANDLE_VALUE) {
 		CloseHandle(a->fh);
 		a->fh = INVALID_HANDLE_VALUE;
+		if (a->tmpname) {
+			/* Windows does not support atomic rename */
+			disk_unlink(a->name);
+			if (_wrename(a->tmpname, a->name) != 0) {
+				archive_set_error(&a->archive, errno,
+				    "rename failed");
+				ret = ARCHIVE_FATAL;
+			}
+			a->tmpname = NULL;
+		}
 	}
 	/* If there's an entry, we can release it now. */
 	archive_entry_free(a->entry);
@@ -1530,26 +1565,40 @@ restore_entry(struct archive_write_disk *a)
 		}
 
 		if (!S_ISDIR(st_mode)) {
-			/* Edge case: a directory symlink pointing to a file */
 			if (a->flags &
 			    ARCHIVE_EXTRACT_CLEAR_NOCHANGE_FFLAGS) {
 				(void)clear_nochange_fflags(a);
 			}
 			if (dirlnk) {
+				/* Edge case: dir symlink pointing to a file */
 				if (disk_rmdir(a->name) != 0) {
 					archive_set_error(&a->archive, errno,
 					    "Can't unlink directory symlink");
 					return (ARCHIVE_FAILED);
 				}
+			} else if ((a->flags & ARCHIVE_EXTRACT_SAFE_WRITES) &&
+				S_ISREG(st_mode)) {
+				int fd = la_mktemp(a);
+
+				if (fd == -1)
+					return (ARCHIVE_FAILED);
+				a->fh = (HANDLE)_get_osfhandle(fd);
+				if (a->fh == INVALID_HANDLE_VALUE)
+					return (ARCHIVE_FAILED);
+
+				a->pst = NULL;
+				en = 0;
+
 			} else if (disk_unlink(a->name) != 0) {
 				/* A non-dir is in the way, unlink it. */
 				archive_set_error(&a->archive, errno,
 				    "Can't unlink already-existing object");
 				return (ARCHIVE_FAILED);
+				a->pst = NULL;
+				/* Try again. */
+				en = create_filesystem_object(a);
 			}
-			a->pst = NULL;
-			/* Try again. */
-			en = create_filesystem_object(a);
+
 		} else if (!S_ISDIR(a->mode)) {
 			/* A dir is in the way of a non-dir, rmdir it. */
 			if (a->flags & ARCHIVE_EXTRACT_CLEAR_NOCHANGE_FFLAGS)
@@ -1601,6 +1650,7 @@ create_filesystem_object(struct archive_write_disk *a)
 	wchar_t *fullname;
 	mode_t final_mode, mode;
 	int r;
+	DWORD attrs = 0;
 
 	/* We identify hard/symlinks according to the link names. */
 	/* Since link(2) and symlink(2) don't handle modes, we're done here. */
@@ -1650,6 +1700,20 @@ create_filesystem_object(struct archive_write_disk *a)
 	}
 	linkname = archive_entry_symlink_w(a->entry);
 	if (linkname != NULL) {
+		/*
+		 * Unlinking and linking here is really not atomic,
+		 * but doing it right, would require us to construct
+		 * an mktemplink() function, and then use rename(2).
+		 *
+		 * The original link may be a directory symlink.
+		 */
+		attrs = GetFileAttributesW(a->name);
+		if (attrs != INVALID_FILE_ATTRIBUTES) {
+			if (attrs & FILE_ATTRIBUTE_DIRECTORY)
+				disk_rmdir(a->name);
+			else
+				disk_unlink(a->name);
+		}
 #if HAVE_SYMLINK
 		return symlink(linkname, a->name) ? errno : 0;
 #else
@@ -1686,6 +1750,7 @@ create_filesystem_object(struct archive_write_disk *a)
 		/* POSIX requires that we fall through here. */
 		/* FALLTHROUGH */
 	case AE_IFREG:
+		a->tmpname = NULL;
 		fullname = a->name;
 		/* O_WRONLY | O_CREAT | O_EXCL */
 		a->fh = CreateFileW(fullname, GENERIC_WRITE, 0, NULL,
@@ -1842,6 +1907,7 @@ _archive_write_disk_free(struct archive *_a)
 	archive_write_disk_set_user_lookup(&a->archive, NULL, NULL, NULL);
 	archive_entry_free(a->entry);
 	archive_wstring_free(&a->_name_data);
+	archive_wstring_free(&a->_tmpname_data);
 	archive_string_free(&a->archive.error_string);
 	archive_wstring_free(&a->path_safe);
 	a->archive.magic = 0;