[PATCH v2 4/4] kernel32: Open replaced file without GENERIC_WRITE in ReplaceFileW().

[Zhiyi Zhang]

As MSDN says, the replaced file is opened with the GENERIC_READ, DELETE,
and SYNCHRONIZE access rights. If GENERIC_WRITE is added, then it will
fail with previous file opens without FILE_SHARE_WRITE.

And then we also need to check if the replaced file has readonly attributes
now that GENERIC_WRITE is gone. It seems fine to add a check in kernel32,
otherwise we need add an extra server call to rename files and checking
the attribute in wineserver.

Wine-Bug:https://bugs.winehq.org/show_bug.cgi?id=33845
Signed-off-by: Zhiyi Zhang <zzhang(a)codeweavers.com>
---
 dlls/kernel32/file.c       | 19 +++++++++++++++++--
 dlls/kernel32/tests/file.c |  4 ++--
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/dlls/kernel32/file.c b/dlls/kernel32/file.c
index c66073b820..64c3e3e826 100644
--- a/dlls/kernel32/file.c
+++ b/dlls/kernel32/file.c
@@ -1746,6 +1746,7 @@ BOOL WINAPI ReplaceFileW(LPCWSTR lpReplacedFileName, LPCWSTR lpReplacementFileNa
     NTSTATUS status;
     IO_STATUS_BLOCK io;
     OBJECT_ATTRIBUTES attr;
+    FILE_BASIC_INFORMATION info;
 
     TRACE("%s %s %s 0x%08x %p %p\n", debugstr_w(lpReplacedFileName),
           debugstr_w(lpReplacementFileName), debugstr_w(lpBackupFileName),
@@ -1772,7 +1773,7 @@ BOOL WINAPI ReplaceFileW(LPCWSTR lpReplacedFileName, LPCWSTR lpReplacementFileNa
     attr.SecurityDescriptor = NULL;
     attr.SecurityQualityOfService = NULL;
 
-    /* Open the "replaced" file for reading and writing */
+    /* Open the "replaced" file for reading */
     if (!(RtlDosPathNameToNtPathName_U(lpReplacedFileName, &nt_replaced_name, NULL, NULL)))
     {
         error = ERROR_PATH_NOT_FOUND;
@@ -1780,7 +1781,7 @@ BOOL WINAPI ReplaceFileW(LPCWSTR lpReplacedFileName, LPCWSTR lpReplacementFileNa
     }
     replaced_flags = lpBackupFileName ? FILE_OPEN : FILE_OPEN_IF;
     attr.ObjectName = &nt_replaced_name;
-    status = NtOpenFile(&hReplaced, GENERIC_READ|GENERIC_WRITE|DELETE|SYNCHRONIZE,
+    status = NtOpenFile(&hReplaced, GENERIC_READ|DELETE|SYNCHRONIZE,
                         &attr, &io,
                         FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE,
                         FILE_SYNCHRONOUS_IO_NONALERT|FILE_NON_DIRECTORY_FILE);
@@ -1796,6 +1797,20 @@ BOOL WINAPI ReplaceFileW(LPCWSTR lpReplacedFileName, LPCWSTR lpReplacementFileNa
         goto fail;
     }
 
+    /* Replacement should fail if replaced is READ_ONLY */
+    status = NtQueryInformationFile(hReplaced, &io, &info, sizeof(info), FileBasicInformation);
+    if (status != STATUS_SUCCESS)
+    {
+        error = RtlNtStatusToDosError(status);
+        goto fail;
+    }
+
+    if (info.FileAttributes & FILE_ATTRIBUTE_READONLY)
+    {
+        error = ERROR_ACCESS_DENIED;
+        goto fail;
+    }
+
     /*
      * Open the replacement file for reading, writing, and deleting
      * (writing and deleting are needed when finished)
diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c
index 65ef4a6b43..4df31c15c7 100644
--- a/dlls/kernel32/tests/file.c
+++ b/dlls/kernel32/tests/file.c
@@ -3691,7 +3691,7 @@ static void test_ReplaceFileA(void)
      */
     SetLastError(0xdeadbeef);
     ret = pReplaceFileA(replaced, replacement, backup, 0, 0, 0);
-    todo_wine ok(ret == 0 && GetLastError() == ERROR_ACCESS_DENIED, "ReplaceFileA: unexpected error %d\n", GetLastError());
+    ok(ret == 0 && GetLastError() == ERROR_ACCESS_DENIED, "ReplaceFileA: unexpected error %d\n", GetLastError());
     /* make sure that the replacement file still exists */
     hReplacementFile = CreateFileA(replacement, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, 0);
     ok(hReplacementFile != INVALID_HANDLE_VALUE ||
@@ -3727,7 +3727,7 @@ static void test_ReplaceFileA(void)
        "unexpected error, replaced file should be able to be opened %d\n", GetLastError());
     /*Calling ReplaceFileA on an exe should succeed*/
     ret = pReplaceFileA(replaced, replacement, NULL, 0, 0, 0);
-    todo_wine ok(ret, "ReplaceFileA: unexpected error %d\n", GetLastError());
+    ok(ret, "ReplaceFileA: unexpected error %d\n", GetLastError());
     CloseHandle(hReplacedFile);
 
     /* replace file while replacement is opened */
-- 
2.19.1