Dmitry Timoshkov <[email protected]> writes: >> Alexandre Julliard <[email protected]> wrote: >> >> > > While investigating how to fix the file section access tests in kernel32 >> > > I've >> > > found that some places in Wine deliberately create objects with access >> > > rights >> > > set to 0, that leads to creation of potentially not accessible objects. >> > >> > Many of these are deliberate. You'll need test cases to show that you >> > can require more permissions. >> >> In the most cases these patches just add the access rights appropriate for >> particular calls instead of assuming some default ones, that should be a good >> thing to do from a security point of view. Test cases are needed, but only >> to figure out what actual default permissions are provided for 0 access, and >> for file sections the test already exists and shows that defaults access is 0 >> and a not accessible object as a result. Creating objects with access rights >> set to 0 should not be used, and considered a bad practice in general IMO. > > I still would like to see your opinion on the above. > > I see that all these patches are marked now as 'Needs tests'. What exactly > needs > tests for cases > 1. when a file is being opened for reading use access GENERIC_READ instead of > 0? > 2. symblic link object opened for deletion use access DELETE instead of 0? > 3. mapping opject created with GENERIC_READ|GENERIC_WRITE (just like in > another > code patch a couple of lines earlier)? > 4. registry key objects are opened with particular access right like > KEY_QUERY_VALUE > for reading or DELETE for deletion instead of 0? > > The cases listed above are at least a non-consistency if not the bugs IMO, > requring > the tests for them looks strange to me.
Requiring extra access rights can break things, especially in such low-level functions. It's not just a harmless cleanup, so you need to either show an app that requires it, or provide a test case that demonstrates that the extra accesses are required on Windows too. -- Alexandre Julliard [email protected]
