[PATCH v5] kernel32: Correct ReplaceFileW behaviour

[Brock York]

On Sat, Sep 08, 2018 at 10:45:16AM +0800, Dmitry Timoshkov wrote:
> Brock York <twunknown at gmail.com> wrote:
> 
> >      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);
> > @@ -1788,6 +1789,19 @@ BOOL WINAPI ReplaceFileW(LPCWSTR lpReplacedFileName, LPCWSTR lpReplacementFileNa
> >          goto fail;
> >      }
> >  
> > +    /* Replacement should fail if replaced is READ_ONLY */
> > +    if (!GetFileInformationByHandle(hReplaced, &file_info))
> > +    {
> > +	    error = GetLastError();
> > +	    goto fail;
> > +    }
> > +
> > +    if (file_info.dwFileAttributes & FILE_ATTRIBUTE_READONLY)
> > +    {
> > +        error = ERROR_ACCESS_DENIED;
> > +        goto fail;
> > +    }
> 
> There is an existing test for NtCreateFile that it's supposed to fail if
> the file has read-only attribute. Probably NtOpenFile with WRITE|DELETE
> access supposed to behave in a similar way. Adding such workarounds to
> high level API like ReplaceFile only clutters the code with not appropriate
> hacks.
> 
> -- 
> Dmitry.

Hello Dmitry

I had written a test to check that exact case before I implemented this
patch but it turns out that I had screwed up my test program and was
trying to NtOpenFile the wrong file...
Which made me then think that the ACCESS_DENIED was coming from the
ReplaceFileW call itself. 

The MSDN documentation for ReplaceFileW states that the replaced
file is opened with GENERIC_READ, DELETE and SYNCHRONISE.
But the next sentence after mentions that the user must have write access,
is this implying that the GENERIC_WRITE flag in passed in the like
original author of ReplaceFileW for wine had implemented or is the MSDN
probably wrong? ? If so the fix is easy modifying the check from NtOpenFile
to handle the access_denied case which you were correct that when WRITE | DELETE
is passed as the access flags to open a read only file NtOpenFile correctly returns
ACCESS_DENIED

Thanks for the good pickup and feedback.

Warm regards
Brock