commit d88792ab9de45efd74909d34fb20c8e8405dbb70
parent 474e74032f21d6f7214fc080f32e07825be027ac
Author: Duncan McIntosh <dmcintosh@mozilla.com>
Date: Mon, 17 Nov 2025 22:23:35 +0200
Bug 2000411 - Use GetTempPathW instead of GetTempPath2W. r=cdupuis!
We want to avoid GetTempPath2W since it is only available on Windows 11
and some of the latest versions of Windows 10.
The only case that they're different is when running as SYSTEM. Most
users won't run the desktop launcher as SYSTEM; for those who do (please
don't), we now hold a handle to the file during the execution to prevent
renaming or symbolic link issues. This should prevent any theoretical
issues that could have happened with GetTempPath2W.
Differential Revision: https://phabricator.services.mozilla.com/D272905
Diffstat:
5 files changed, 69 insertions(+), 4 deletions(-)
diff --git a/browser/app/desktop-launcher/file_sink.cpp b/browser/app/desktop-launcher/file_sink.cpp
@@ -9,8 +9,10 @@
// Open the download receiver
bool FileSink::open(std::wstring& filename) {
+ mFilename.assign(filename);
// Note: this only succeeds if the filename does not exist.
- fileHandle.own(CreateFileW(filename.c_str(), GENERIC_WRITE, 0, nullptr,
+ fileHandle.own(CreateFileW(filename.c_str(), GENERIC_WRITE,
+ FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr,
CREATE_NEW, FILE_ATTRIBUTE_NORMAL, nullptr));
if (fileHandle.get() == INVALID_HANDLE_VALUE) {
return false;
@@ -31,3 +33,18 @@ bool FileSink::accept(char* buf, int bytesToWrite) {
}
return true;
}
+
+bool FileSink::freeze() {
+ // Exchange our write-only handle for a read-only one. This allows us to
+ // prevent renaming or deleting the file under our nose, while also being
+ // able to actually run it.
+ HANDLE readOnlyHandle = CreateFileW(
+ mFilename.c_str(), GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE,
+ nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr);
+ if (readOnlyHandle == INVALID_HANDLE_VALUE) {
+ return false;
+ }
+
+ fileHandle.own(readOnlyHandle);
+ return true;
+}
diff --git a/browser/app/desktop-launcher/file_sink.h b/browser/app/desktop-launcher/file_sink.h
@@ -16,9 +16,12 @@ class FileSink : public DataSink {
bool open(std::wstring& filename);
// Send data to the download receiver
bool accept(char* buf, int bytesToWrite) override;
+ // Make our handle read-only so we can run the file
+ bool freeze();
private:
nsAutoHandle fileHandle;
+ std::wstring mFilename;
};
#endif
diff --git a/browser/app/desktop-launcher/main.cpp b/browser/app/desktop-launcher/main.cpp
@@ -62,8 +62,12 @@ int wmain() {
bool download_completed = false;
// If that doesn't work, we try to download the installer.
std::optional<std::wstring> tempfileName = get_tempfile_name();
+ // If we do create a fileSink, it needs to live until the
+ // ExecuteAndWaitForIdle calls return to ensure that other processes cannot
+ // delete or rename it.
+ std::unique_ptr<FileSink> fileSink;
if (tempfileName.has_value()) {
- std::unique_ptr<FileSink> fileSink = std::make_unique<FileSink>();
+ fileSink = std::make_unique<FileSink>();
if (fileSink->open(tempfileName.value())) {
ErrCode rc = download_firefox(fileSink.get());
if (rc == ErrCode::OK) {
@@ -74,7 +78,8 @@ int wmain() {
}
// If the installer successfully downloaded, try to launch it
if (download_completed) {
- if (ExecuteAndWaitForIdle(tempfileName.value(), STUB_INSTALLER_ARGS)) {
+ if (fileSink->freeze() &&
+ ExecuteAndWaitForIdle(tempfileName.value(), STUB_INSTALLER_ARGS)) {
std::wcout << L"Firefox installer launched" << std::endl;
return 0;
}
diff --git a/browser/app/desktop-launcher/tempfile_name.cpp b/browser/app/desktop-launcher/tempfile_name.cpp
@@ -17,7 +17,7 @@ std::optional<std::wstring> get_tempfile_name() {
wchar_t pathBuffer[BUFFER_LEN];
wchar_t filenameBuffer[BUFFER_LEN];
UUID uuid;
- DWORD pathLen = GetTempPath2W(BUFFER_LEN, pathBuffer);
+ DWORD pathLen = GetTempPathW(BUFFER_LEN, pathBuffer);
if (pathLen > BUFFER_LEN || pathLen == 0) {
// Error getting path
return std::nullopt;
diff --git a/browser/app/desktop-launcher/tests/gtest/DesktopLauncherTest.cpp b/browser/app/desktop-launcher/tests/gtest/DesktopLauncherTest.cpp
@@ -11,6 +11,7 @@
#include "tempfile_name.h"
#include "download_firefox.h"
#include "data_sink.h"
+#include "file_sink.h"
static std::optional<std::wstring> get_value_from_key(
const wchar_t* key_path, const wchar_t* value_path) {
@@ -100,3 +101,42 @@ TEST_F(DesktopLauncherTest, TestGetObjectName) {
ASSERT_NE(std::wstring::npos, objectName.value().find(L"lang="));
ASSERT_NE(std::wstring::npos, objectName.value().find(L"product="));
}
+
+TEST_F(DesktopLauncherTest, TestTempFileNamesAreDifferent) {
+ std::wstring temp1 = get_tempfile_name().value();
+ std::wstring temp2 = get_tempfile_name().value();
+ ASSERT_NE(temp1, temp2);
+}
+
+TEST_F(DesktopLauncherTest, TestFileSinkFreeze) {
+ std::wstring path = get_tempfile_name().value();
+
+ auto delete_expecting_error = [](const wchar_t* path, DWORD expected) {
+ SetLastError(ERROR_SUCCESS);
+ BOOL result = DeleteFileW(path);
+ DWORD lastError = GetLastError();
+ ASSERT_EQ(result, expected == ERROR_SUCCESS);
+ ASSERT_EQ(lastError, expected);
+ };
+
+ char data[] = "important data";
+
+ {
+ FileSink sink;
+ delete_expecting_error(path.c_str(), ERROR_FILE_NOT_FOUND);
+ ASSERT_TRUE(sink.open(path));
+ delete_expecting_error(path.c_str(), ERROR_SHARING_VIOLATION);
+ ASSERT_TRUE(sink.accept(data, sizeof(data) - 1));
+ delete_expecting_error(path.c_str(), ERROR_SHARING_VIOLATION);
+ ASSERT_TRUE(sink.freeze());
+ delete_expecting_error(path.c_str(), ERROR_SHARING_VIOLATION);
+ ASSERT_FALSE(sink.accept(data, sizeof(data) - 1));
+
+ // TODO: Automatically test that the path can be run with ShellExecuteEx
+ // or similar.
+ //
+ // To test this manually, run the launcher without Firefox installed. You
+ // should see a prompt to install Firefox; if so, then it was executable.
+ }
+ delete_expecting_error(path.c_str(), ERROR_SUCCESS);
+}
