[Bug 20317] New: Uninitialised memory reference in SetEntriesInAclW()
http://bugs.winehq.org/show_bug.cgi?id=20317 Summary: Uninitialised memory reference in SetEntriesInAclW() Product: Wine Version: 1.1.31 Platform: PC OS/Version: Linux Status: NEW Keywords: download, patch, source Severity: normal Priority: P2 Component: advapi32 AssignedTo: wine-bugs(a)winehq.org ReportedBy: dank(a)kegel.com Once you are past bug 20303 and bug 20315, the commands cd dlls/advapi32/tests /usr/local/valgrind-10896/bin/valgrind --trace-children=yes --track-origins=yes --workaround-gcc296-bugs=yes ~/wine-git/wine advapi32_test.exe.so security.c produce the valgrind warning Conditional jump or move depends on uninitialised value(s) at RtlAllocateHeap (heap.c:1373) by HeapAlloc (heap.c:276) by GlobalAlloc (heap.c:361) by LocalAlloc (heap.c:961) by SetEntriesInAclW (security.c:3568) by test_SetEntriesInAcl (security.c:2583) Uninitialised value was created by a client request at mark_block_uninitialized (heap.c:187) by RtlAllocateHeap (heap.c:1429) by SetEntriesInAclW (security.c:3471) by test_SetEntriesInAcl (security.c:2583) (so the amount of memory being allocated is undefined!) It seems the ppsid memory block is not fully initialized, since the change --- a/dlls/advapi32/security.c +++ b/dlls/advapi32/security.c @@ -3468,7 +3468,7 @@ DWORD WINAPI SetEntriesInAclW( ULONG count, PEXPLICIT_ACCESSW pEntries, return ERROR_SUCCESS; /* allocate array of maximum sized sids allowed */ - ppsid = HeapAlloc(GetProcessHeap(), 0, count * (sizeof(SID *) + FIELD_OFFSET(SID, SubAuthority[SID_MAX_SUB_AUTHORITIES]))); + ppsid = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, count * (sizeof(SID *) + FIELD_OFFSET(SID, SubAuthority[SID_MAX_SUB_AUTHORITIES]))); makes the warning go away. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=20317 --- Comment #1 from Dan Kegel <dank(a)kegel.com> 2009-10-10 22:22:23 --- Created an attachment (id=24023) --> (http://bugs.winehq.org/attachment.cgi?id=24023) Quick patch to work around missing initialization -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=20317 Dan Kegel <dank(a)kegel.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |robertshearman(a)gmail.com --- Comment #2 from Dan Kegel <dank(a)kegel.com> 2009-10-20 03:35:17 --- Still there today. cc'ing Rob, who wrote this bit about two years ago (not sure he's still interested, though.) (BTW, you also need a simple patch from John Reiser to get current valgrind to not complain on every heap allocation, see https://bugs.kde.org/show_bug.cgi?id=205541#c1 ) -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=20317 --- Comment #3 from Rob Shearman <robertshearman(a)gmail.com> 2009-10-21 09:03:15 --- I'm guessing this is Valgrind warning for the same piece of code that I commented on when the patch was sent and not fixed: subject Re: advapi32: SetEntriesInAclW() should accept account name "CURRENT_USER". mailed-by gmail.com hide details 18 Aug 2009/8/16 Rein Klazes <wijn(a)online.nl>:
@@ -3510,7 +3511,8 @@ DWORD WINAPI SetEntriesInAclW( ULONG count, PEXPLICIT_ACCESSW pEntries, DWORD sid_size = FIELD_OFFSET(SID, SubAuthority[SID_MAX_SUB_AUTHORITIES]); DWORD domain_size = MAX_COMPUTERNAME_LENGTH + 1; SID_NAME_USE use; - if (!LookupAccountNameW(NULL, pEntries[i].Trustee.ptstrName, ppsid[i], &sid_size, NULL, &domain_size, &use)) + if ( strcmpW( pEntries[i].Trustee.ptstrName, CURRENT_USER ) &&
What will the memory pointed to by ppsid[i] be set to in this case? Hint: whatever the memory was last used for, not the SID of the current user which is what is desired.
+ !LookupAccountNameW(NULL, pEntries[i].Trustee.ptstrName, ppsid[i], &sid_size, NULL, &domain_size, &use)) { WARN("bad user name %s for trustee %d\n", debugstr_w(pEntries[i].Trustee.ptstrName), i); ret = ERROR_INVALID_PARAMETER;
"git revert b46e2ef9b1d1349d4caf7f7a31ea1b6c348875ca" should also fix the issue if the original author doesn't want to fix the code. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=20317 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution| |FIXED --- Comment #4 from Alexandre Julliard <julliard(a)winehq.org> 2009-10-22 10:41:18 --- Should be fixed by 104a0f5439c941c5a052de28c40fd6613cdf99c1. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=20317 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #5 from Alexandre Julliard <julliard(a)winehq.org> 2009-10-23 13:19:33 --- Closing bugs fixed in 1.1.32. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
participants (1)
-
wine-bugs＠winehq.org