Fix faulty memory access in Util's User custom actions
Generally, clean up the handling of getting the domain from a server name by centralizing and simplifying it behind an improved GetDomainFromServerName() based on the buggy GetServerName(). Fixes 8576
Rob Mensching committed
Jul 14, 2024 at 23:58 UTC
6bd80b51b42686ce5665140d0ab7c64bd35204d9
5 files changed
+95
-120
src/ext/Util/ca/precomp.h
+2
-1
@@ -15,7 +15,7 @@
15
16
#include <msxml2.h>
17
#include <Iads.h>
18
-#include <activeds.h>
18
+#include <activeds.h>
19
#include <lm.h> // NetApi32.lib
20
#include <Ntsecapi.h>
21
#include <Dsgetdc.h>
@@ -50,5 +50,6 @@
50
#include "scauser.h"
51
#include "scasmb.h"
52
#include "scasmbexec.h"
53
+#include "utilca.h"
54
55
#include "..\..\caDecor.h"
src/ext/Util/ca/scaexec.cpp
+21
-79
@@ -613,8 +613,7 @@ static HRESULT RemoveUserInternal(
613
LPWSTR pwz = NULL;
614
LPWSTR pwzGroup = NULL;
615
LPWSTR pwzGroupDomain = NULL;
616
- LPCWSTR wz = NULL;
617
- PDOMAIN_CONTROLLER_INFOW pDomainControllerInfo = NULL;
616
+ LPWSTR pwzDomainName = NULL;
617
618
//
619
// Remove the logon as service privilege.
@@ -644,30 +643,10 @@ static HRESULT RemoveUserInternal(
643
//
644
if (!(SCAU_DONT_CREATE_USER & iAttributes))
645
{
647
- if (wzDomain && *wzDomain)
648
- {
649
- er = ::DsGetDcNameW(NULL, (LPCWSTR)wzDomain, NULL, NULL, NULL, &pDomainControllerInfo);
650
- if (RPC_S_SERVER_UNAVAILABLE == er)
651
- {
652
- // MSDN says, if we get the above error code, try again with the "DS_FORCE_REDISCOVERY" flag
653
- er = ::DsGetDcNameW(NULL, (LPCWSTR)wzDomain, NULL, NULL, DS_FORCE_REDISCOVERY, &pDomainControllerInfo);
654
- }
655
- if (ERROR_SUCCESS == er)
656
- {
657
- if (2 <= wcslen(pDomainControllerInfo->DomainControllerName))
658
- {
659
- wz = pDomainControllerInfo->DomainControllerName + 2; // Add 2 so that we don't get the \\ prefix.
660
- // Pass the entire string if it is too short
661
- // to have a \\ prefix.
662
- }
663
- }
664
- else
665
- {
666
- wz = wzDomain;
667
- }
668
- }
646
+ hr = GetDomainFromServerName(&pwzDomainName, wzDomain, 0);
647
+ ExitOnFailure(hr, "Failed to get domain to remove user from server name: %ls", wzDomain);
648
670
- er = ::NetUserDel(wz, wzName);
649
+ er = ::NetUserDel(pwzDomainName, wzName);
650
if (NERR_UserNotFound == er)
651
{
652
er = NERR_Success;
@@ -707,52 +686,13 @@ static HRESULT RemoveUserInternal(
686
}
687
688
LExit:
710
- if (pDomainControllerInfo)
711
- {
712
- ::NetApiBufferFree(static_cast<LPVOID>(pDomainControllerInfo));
713
- }
689
+ ReleaseStr(pwzDomainName);
690
+ ReleaseStr(pwzGroupDomain);
691
+ ReleaseStr(pwzGroup);
692
693
return hr;
694
}
695
718
-static void GetServerName(LPWSTR pwzDomain, LPWSTR* ppwzServerName)
719
-{
720
- DWORD er = ERROR_SUCCESS;
721
- PDOMAIN_CONTROLLER_INFOW pDomainControllerInfo = NULL;
722
-
723
- if (pwzDomain && *pwzDomain)
724
- {
725
- er = ::DsGetDcNameW(NULL, (LPCWSTR)pwzDomain, NULL, NULL, NULL, &pDomainControllerInfo);
726
- if (RPC_S_SERVER_UNAVAILABLE == er)
727
- {
728
- // MSDN says, if we get the above error code, try again with the "DS_FORCE_REDISCOVERY" flag
729
- er = ::DsGetDcNameW(NULL, (LPCWSTR)pwzDomain, NULL, NULL, DS_FORCE_REDISCOVERY, &pDomainControllerInfo);
730
- }
731
-
732
- if (ERROR_SUCCESS == er && pDomainControllerInfo->DomainControllerName)
733
- {
734
- // Skip the \\ prefix if present.
735
- if ('\\' == *pDomainControllerInfo->DomainControllerName && '\\' == *pDomainControllerInfo->DomainControllerName + 1)
736
- {
737
- *ppwzServerName = pDomainControllerInfo->DomainControllerName + 2;
738
- }
739
- else
740
- {
741
- *ppwzServerName = pDomainControllerInfo->DomainControllerName;
742
- }
743
- }
744
- else
745
- {
746
- *ppwzServerName = pwzDomain;
747
- }
748
- }
749
-
750
- if (pDomainControllerInfo)
751
- {
752
- ::NetApiBufferFree((LPVOID)pDomainControllerInfo);
753
- }
754
-}
755
-
696
/********************************************************************
697
CreateUser - CUSTOM ACTION ENTRY POINT for creating users
698
@@ -776,6 +716,7 @@ extern "C" UINT __stdcall CreateUser(
716
LPWSTR pwzPassword = NULL;
717
LPWSTR pwzGroup = NULL;
718
LPWSTR pwzGroupDomain = NULL;
719
+ LPWSTR pwzDomainName = NULL;
720
int iAttributes = 0;
721
BOOL fInitializedCom = FALSE;
722
@@ -786,7 +727,6 @@ extern "C" UINT __stdcall CreateUser(
727
USER_INFO_1 userInfo1;
728
USER_INFO_1* pUserInfo1 = NULL;
729
DWORD dw;
789
- LPWSTR pwzServerName = NULL;
730
731
hr = WcaInitialize(hInstall, "CreateUser");
732
ExitOnFailure(hr, "failed to initialize");
@@ -845,9 +785,10 @@ extern "C" UINT __stdcall CreateUser(
785
//
786
// Create the User
787
//
848
- GetServerName(pwzDomain, &pwzServerName);
788
+ hr = GetDomainFromServerName(&pwzDomainName, pwzDomain, 0);
789
+ ExitOnFailure(hr, "Failed to get domain from server name: %ls", pwzDomain);
790
850
- er = ::NetUserAdd(pwzServerName, 1, reinterpret_cast<LPBYTE>(pUserInfo1), &dw);
791
+ er = ::NetUserAdd(pwzDomainName, 1, reinterpret_cast<LPBYTE>(pUserInfo1), &dw);
792
if (NERR_UserExists == er)
793
{
794
if (SCAU_FAIL_IF_EXISTS & iAttributes)
@@ -862,7 +803,7 @@ extern "C" UINT __stdcall CreateUser(
803
if (SCAU_UPDATE_IF_EXISTS & iAttributes)
804
{
805
pUserInfo1 = NULL;
865
- er = ::NetUserGetInfo(pwzServerName, pwzName, 1, reinterpret_cast<LPBYTE*>(&pUserInfo1));
806
+ er = ::NetUserGetInfo(pwzDomainName, pwzName, 1, reinterpret_cast<LPBYTE*>(&pUserInfo1));
807
if (ERROR_SUCCESS == er)
808
{
809
// There is no rollback scheduled if the key is empty.
@@ -922,28 +863,28 @@ extern "C" UINT __stdcall CreateUser(
863
864
if (ERROR_SUCCESS == er)
865
{
925
- hr = SetUserPassword(pwzServerName, pwzName, pwzPassword);
866
+ hr = SetUserPassword(pwzDomainName, pwzName, pwzPassword);
867
if (FAILED(hr))
868
{
928
- WcaLogError(hr, "failed to set user password for user %ls\\%ls, continuing anyway.", pwzServerName, pwzName);
869
+ WcaLogError(hr, "failed to set user password for user %ls\\%ls, continuing anyway.", pwzDomainName, pwzName);
870
hr = S_OK;
871
}
872
873
if (SCAU_REMOVE_COMMENT & iAttributes)
874
{
934
- hr = SetUserComment(pwzServerName, pwzName, L"");
875
+ hr = SetUserComment(pwzDomainName, pwzName, L"");
876
if (FAILED(hr))
877
{
937
- WcaLogError(hr, "failed to clear user comment for user %ls\\%ls, continuing anyway.", pwzServerName, pwzName);
878
+ WcaLogError(hr, "failed to clear user comment for user %ls\\%ls, continuing anyway.", pwzDomainName, pwzName);
879
hr = S_OK;
880
}
881
}
882
else if (pwzComment && *pwzComment)
883
{
943
- hr = SetUserComment(pwzServerName, pwzName, pwzComment);
884
+ hr = SetUserComment(pwzDomainName, pwzName, pwzComment);
885
if (FAILED(hr))
886
{
946
- WcaLogError(hr, "failed to set user comment to %ls for user %ls\\%ls, continuing anyway.", pwzComment, pwzServerName, pwzName);
887
+ WcaLogError(hr, "failed to set user comment to %ls for user %ls\\%ls, continuing anyway.", pwzComment, pwzDomainName, pwzName);
888
hr = S_OK;
889
}
890
}
@@ -952,10 +893,10 @@ extern "C" UINT __stdcall CreateUser(
893
894
ApplyAttributes(iAttributes, &flags);
895
955
- hr = SetUserFlags(pwzServerName, pwzName, flags);
896
+ hr = SetUserFlags(pwzDomainName, pwzName, flags);
897
if (FAILED(hr))
898
{
958
- WcaLogError(hr, "failed to set user flags for user %ls\\%ls, continuing anyway.", pwzServerName, pwzName);
899
+ WcaLogError(hr, "failed to set user flags for user %ls\\%ls, continuing anyway.", pwzDomainName, pwzName);
900
hr = S_OK;
901
}
902
}
@@ -1018,6 +959,7 @@ LExit:
959
ReleaseStr(pwzPassword);
960
ReleaseStr(pwzGroup);
961
ReleaseStr(pwzGroupDomain);
962
+ ReleaseStr(pwzDomainName)
963
964
if (fInitializedCom)
965
{
src/ext/Util/ca/scauser.cpp
+8
-40
@@ -487,7 +487,7 @@ HRESULT ScaUserExecute(
487
{
488
HRESULT hr = S_OK;
489
DWORD er = 0;
490
- PDOMAIN_CONTROLLER_INFOW pDomainControllerInfo = NULL;
490
+ LPWSTR pwzDomainName = NULL;
491
492
LPWSTR pwzBaseScriptKey = NULL;
493
DWORD cScriptKey = 0;
@@ -518,36 +518,11 @@ HRESULT ScaUserExecute(
518
ExitOnFailure(hr, "Failed to add user comment to custom action data: %ls", psu->wzComment);
519
520
// Check to see if the user already exists since we have to be very careful when adding
521
- // and removing users. Note: MSDN says that it is safe to call these APIs from any
522
- // user, so we should be safe calling it during immediate mode.
523
- er = ::NetApiBufferAllocate(sizeof(USER_INFO_0), reinterpret_cast<LPVOID*>(&pUserInfo));
524
- hr = HRESULT_FROM_WIN32(er);
525
- ExitOnFailure(hr, "Failed to allocate memory to check existence of user: %ls", psu->wzName);
526
-
527
- LPCWSTR wzDomain = psu->wzDomain;
528
- if (wzDomain && *wzDomain)
529
- {
530
- er = ::DsGetDcNameW(NULL, wzDomain, NULL, NULL, NULL, &pDomainControllerInfo);
531
- if (RPC_S_SERVER_UNAVAILABLE == er)
532
- {
533
- // MSDN says, if we get the above error code, try again with the "DS_FORCE_REDISCOVERY" flag
534
- er = ::DsGetDcNameW(NULL, wzDomain, NULL, NULL, DS_FORCE_REDISCOVERY, &pDomainControllerInfo);
535
- }
536
- if (ERROR_SUCCESS == er && pDomainControllerInfo->DomainControllerName)
537
- {
538
- // If the \\ prefix on the queried domain was present, skip it.
539
- if ('\\' == *pDomainControllerInfo->DomainControllerName && '\\' == *pDomainControllerInfo->DomainControllerName + 1)
540
- {
541
- wzDomain = pDomainControllerInfo->DomainControllerName + 2;
542
- }
543
- else
544
- {
545
- wzDomain = pDomainControllerInfo->DomainControllerName;
546
- }
547
- }
548
- }
521
+ // and removing users.
522
+ hr = GetDomainFromServerName(&pwzDomainName, psu->wzDomain, 0);
523
+ ExitOnFailure(hr, "Failed to get domain from server name: %ls", psu->wzDomain);
524
550
- er = ::NetUserGetInfo(wzDomain, psu->wzName, 0, reinterpret_cast<LPBYTE*>(pUserInfo));
525
+ er = ::NetUserGetInfo(pwzDomainName, psu->wzName, 0, reinterpret_cast<LPBYTE*>(&pUserInfo));
526
if (NERR_Success == er)
527
{
528
ueUserExists = USER_EXISTS_YES;
@@ -560,7 +535,7 @@ HRESULT ScaUserExecute(
535
{
536
ueUserExists = USER_EXISTS_INDETERMINATE;
537
hr = HRESULT_FROM_WIN32(er);
563
- WcaLog(LOGMSG_VERBOSE, "Failed to check existence of domain: %ls, user: %ls (error code 0x%x) - continuing", wzDomain, psu->wzName, hr);
538
+ WcaLog(LOGMSG_VERBOSE, "Failed to check existence of domain: %ls, user: %ls (error code 0x%x) - continuing", pwzDomainName, psu->wzName, hr);
539
hr = S_OK;
540
er = ERROR_SUCCESS;
541
}
@@ -685,11 +660,6 @@ HRESULT ScaUserExecute(
660
::NetApiBufferFree(static_cast<LPVOID>(pUserInfo));
661
pUserInfo = NULL;
662
}
688
- if (pDomainControllerInfo)
689
- {
690
- ::NetApiBufferFree(static_cast<LPVOID>(pDomainControllerInfo));
691
- pDomainControllerInfo = NULL;
692
- }
663
}
664
665
LExit:
@@ -697,14 +667,12 @@ LExit:
667
ReleaseStr(pwzScriptKey);
668
ReleaseStr(pwzActionData);
669
ReleaseStr(pwzRollbackData);
670
+ ReleaseStr(pwzDomainName);
671
+
672
if (pUserInfo)
673
{
674
::NetApiBufferFree(static_cast<LPVOID>(pUserInfo));
675
}
704
- if (pDomainControllerInfo)
705
- {
706
- ::NetApiBufferFree(static_cast<LPVOID>(pDomainControllerInfo));
707
- }
676
677
return hr;
678
}
src/ext/Util/ca/utilca.cpp
+56
@@ -1,3 +1,59 @@
1
// Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the Microsoft Reciprocal License. See LICENSE.TXT file in the project root for full license information.
2
3
#include "precomp.h"
4
+
5
+HRESULT GetDomainFromServerName(
6
+ __deref_out_z LPWSTR* ppwzDomainName,
7
+ __in_z LPCWSTR wzServerName,
8
+ __in DWORD dwFlags
9
+ )
10
+{
11
+ HRESULT hr = S_OK;
12
+ DWORD er = ERROR_SUCCESS;
13
+ PDOMAIN_CONTROLLER_INFOW pDomainControllerInfo = NULL;
14
+ LPCWSTR wz = wzServerName ? wzServerName : L""; // initialize the domain to the provided server name (or empty string).
15
+
16
+ // If the server name was not empty, try to get the domain name out of it.
17
+ if (*wz)
18
+ {
19
+ er = ::DsGetDcNameW(NULL, wz, NULL, NULL, dwFlags, &pDomainControllerInfo);
20
+ if (RPC_S_SERVER_UNAVAILABLE == er)
21
+ {
22
+ // MSDN says, if we get the above error code, try again with the "DS_FORCE_REDISCOVERY" flag.
23
+ er = ::DsGetDcNameW(NULL, wz, NULL, NULL, dwFlags | DS_FORCE_REDISCOVERY, &pDomainControllerInfo);
24
+ }
25
+ ExitOnWin32Error(er, hr, "Could not get domain name from server name: %ls", wz);
26
+
27
+ if (pDomainControllerInfo->DomainControllerName)
28
+ {
29
+ // Skip the \\ prefix if present.
30
+ if ('\\' == *pDomainControllerInfo->DomainControllerName && '\\' == *(pDomainControllerInfo->DomainControllerName + 1))
31
+ {
32
+ wz = pDomainControllerInfo->DomainControllerName + 2;
33
+ }
34
+ else
35
+ {
36
+ wz = pDomainControllerInfo->DomainControllerName;
37
+ }
38
+ }
39
+ }
40
+
41
+LExit:
42
+ // Note: we overwrite the error code here as failure to contact domain controller above is not a fatal error.
43
+ if (wz && *wz)
44
+ {
45
+ hr = StrAllocString(ppwzDomainName, wz, 0);
46
+ }
47
+ else // return NULL the server name ended up empty.
48
+ {
49
+ ReleaseNullStr(*ppwzDomainName);
50
+ hr = S_OK;
51
+ }
52
+
53
+ if (pDomainControllerInfo)
54
+ {
55
+ ::NetApiBufferFree((LPVOID)pDomainControllerInfo);
56
+ }
57
+
58
+ return hr;
59
+}
src/ext/Util/ca/utilca.h
new
+8
@@ -0,0 +1,8 @@
1
+#pragma once
2
+// Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the Microsoft Reciprocal License. See LICENSE.TXT file in the project root for full license information.
3
+
4
+HRESULT GetDomainFromServerName(
5
+ __deref_out_z LPWSTR* ppwzDomainName,
6
+ __in_z LPCWSTR wzServerName,
7
+ __in DWORD dwFlags
8
+ );