Re: [PATCH 2/3] ntdll: Improve invalid parameter handling in NtAccessCheck. (try 2)
Qian Hong <qhong(a)codeweavers.com> writes:
@@ -1586,7 +1586,16 @@ NtAccessCheck( SecurityDescriptor, ClientToken, DesiredAccess, GenericMapping, PrivilegeSet, ReturnLength, GrantedAccess, AccessStatus);
- if (!PrivilegeSet || !ReturnLength) + if (!ReturnLength) + return STATUS_ACCESS_VIOLATION; + + if (*ReturnLength == 0) + { + *ReturnLength = sizeof(PRIVILEGE_SET);
Shouldn't this depend on the number of privileges to be returned? -- Alexandre Julliard julliard(a)winehq.org
On Wed, Feb 17, 2016 at 11:02 PM, Alexandre Julliard <julliard(a)winehq.org> wrote:
Shouldn't this depend on the number of privileges to be returned?
Thanks for review! Yes. In theory this should depend on the number of privileges to be returned. In practice, Windows requires a minimal value of sizeof(PRIVILEGE_SET) (size equal to 20) not matter if PrivilegeCount is 0 (size equal to 8) or PrivilegeCount is 1 (size equal to 20). I can't find a test case where Windows returns a PrivilegeCount larger than 1. Wine's current implementation also does not return any value larger than 1. I agree that it is not safe and maintainable to hardcode like this, so I changed my implementation to MAX( 20, real_size_needed ), this will match Windows' minimal requirement, also runs safely on Wine. The real world app I want to fix relies on the exact last error code when ReturnLength and PrivSet are both 0/NULL, there are also some other programmers rely on this way to fetch required length according to MSDN comments. I didn't sent the NtAccessCheck version of test this time, my patch v2 show that NtAccessCheck behaviors the same to AccessCheck, I'll resent that patch after current patch committed in order to avoid further changing and rebasing. Thanks! -- Regards, Qian Hong - http://www.codeweavers.com
participants (2)
-
Alexandre Julliard -
Qian Hong