Dmitry Timoshkov dmitry@baikal.ru writes:
Alexandre Julliard julliard@winehq.org 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
- when a file is being opened for reading use access GENERIC_READ instead of 0?
- symblic link object opened for deletion use access DELETE instead of 0?
- 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.