[ADVAPI32][RPCRT4][UMPNPMGR][WINLOGON] Fix buffer size retrieval for MakeSelfRelativeSD() (#8395)

Based on Timo's observation in PR #8387.

And improve debug ERR strings for last-error values.
This commit is contained in:
Hermès Bélusca-Maïto
2025-10-08 15:45:36 +02:00
parent c76049ec8b
commit 8f3ac7ef58
4 changed files with 73 additions and 102 deletions
+19 -27
View File
@@ -369,6 +369,7 @@ CreatePnpInstallEventSecurity(
goto Quit;
}
/* Compute the size needed for the DACL and allocate it */
DaclSize = sizeof(ACL) +
sizeof(ACCESS_ALLOWED_ACE) + GetLengthSid(SystemSid) +
sizeof(ACCESS_ALLOWED_ACE) + GetLengthSid(AdminsSid);
@@ -379,7 +380,6 @@ CreatePnpInstallEventSecurity(
ErrCode = ERROR_OUTOFMEMORY;
goto Quit;
}
if (!InitializeAcl(Dacl, DaclSize, ACL_REVISION))
{
ErrCode = GetLastError();
@@ -428,20 +428,26 @@ CreatePnpInstallEventSecurity(
goto Quit;
}
if (!MakeSelfRelativeSD(&AbsoluteSd, NULL, &Size) && GetLastError() == ERROR_INSUFFICIENT_BUFFER)
/* Retrieve the size needed for the relative SD */
if (MakeSelfRelativeSD(&AbsoluteSd, NULL, &Size) ||
(GetLastError() != ERROR_INSUFFICIENT_BUFFER))
{
RelativeSd = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, Size);
if (RelativeSd == NULL)
{
ErrCode = ERROR_OUTOFMEMORY;
goto Quit;
}
ErrCode = GetLastError();
goto Quit;
}
if (!MakeSelfRelativeSD(&AbsoluteSd, RelativeSd, &Size))
{
ErrCode = GetLastError();
goto Quit;
}
/* Build the relative SD */
RelativeSd = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, Size);
if (RelativeSd == NULL)
{
ErrCode = ERROR_OUTOFMEMORY;
goto Quit;
}
if (!MakeSelfRelativeSD(&AbsoluteSd, RelativeSd, &Size))
{
ErrCode = GetLastError();
HeapFree(GetProcessHeap(), 0, RelativeSd);
goto Quit;
}
*EventSd = RelativeSd;
@@ -449,27 +455,13 @@ CreatePnpInstallEventSecurity(
Quit:
if (SystemSid)
{
FreeSid(SystemSid);
}
if (AdminsSid)
{
FreeSid(AdminsSid);
}
if (Dacl)
{
HeapFree(GetProcessHeap(), 0, Dacl);
}
if (ErrCode != ERROR_SUCCESS)
{
if (RelativeSd)
{
HeapFree(GetProcessHeap(), 0, RelativeSd);
}
}
return ErrCode;
}
+6 -5
View File
@@ -67,10 +67,11 @@ ConvertToSelfRelative(
PSECURITY_DESCRIPTOR RelativeSd;
DWORD DescriptorLength = 0;
/* Determine the size for our buffer to allocate */
if (!MakeSelfRelativeSD(AbsoluteSd, NULL, &DescriptorLength) && GetLastError() != ERROR_INSUFFICIENT_BUFFER)
/* Determine the size for allocating our buffer */
if (MakeSelfRelativeSD(AbsoluteSd, NULL, &DescriptorLength) ||
(GetLastError() != ERROR_INSUFFICIENT_BUFFER))
{
ERR("ConvertToSelfRelative(): Unexpected error code (error code %lu -- must be ERROR_INSUFFICIENT_BUFFER)\n", GetLastError());
ERR("ConvertToSelfRelative(): error %lu, expected ERROR_INSUFFICIENT_BUFFER\n", GetLastError());
return NULL;
}
@@ -80,14 +81,14 @@ ConvertToSelfRelative(
DescriptorLength);
if (RelativeSd == NULL)
{
ERR("ConvertToSelfRelative(): Failed to allocate buffer for relative SD!\n");
ERR("ConvertToSelfRelative(): Failed to allocate buffer for relative SD\n");
return NULL;
}
/* Convert the security descriptor now */
if (!MakeSelfRelativeSD(AbsoluteSd, RelativeSd, &DescriptorLength))
{
ERR("ConvertToSelfRelative(): Failed to convert the security descriptor to a self relative format (error code %lu)\n", GetLastError());
ERR("ConvertToSelfRelative(): Failed to convert the security descriptor to a self relative format (error %lu)\n", GetLastError());
RtlFreeHeap(RtlGetProcessHeap(), 0, RelativeSd);
return NULL;
}
+22 -28
View File
@@ -164,7 +164,7 @@ CreateDefaultProcessSecurityCommon(
TokenOwnerSize);
if (OwnerOfToken == NULL)
{
ERR("CreateDefaultProcessSecurityCommon(): Failed to allocate buffer for token owner!\n");
ERR("CreateDefaultProcessSecurityCommon(): Failed to allocate buffer for token owner\n");
return FALSE;
}
@@ -200,7 +200,7 @@ CreateDefaultProcessSecurityCommon(
PrimaryGroupSize);
if (PrimaryGroupOfToken == NULL)
{
ERR("CreateDefaultProcessSecurityCommon(): Failed to allocate buffer for primary group token!\n");
ERR("CreateDefaultProcessSecurityCommon(): Failed to allocate buffer for primary group token\n");
Success = FALSE;
goto Quit;
}
@@ -225,7 +225,7 @@ CreateDefaultProcessSecurityCommon(
0, 0, 0, 0, 0, 0, 0,
&SystemSid))
{
ERR("CreateDefaultProcessSecurityCommon(): Failed to create Local System SID (error code %d)\n", GetLastError());
ERR("CreateDefaultProcessSecurityCommon(): Failed to create Local System SID (error %lu)\n", GetLastError());
Success = FALSE;
goto Quit;
}
@@ -245,7 +245,7 @@ CreateDefaultProcessSecurityCommon(
DaclSize);
if (Dacl == NULL)
{
ERR("CreateDefaultProcessSecurityCommon(): Failed to allocate buffer for DACL!\n");
ERR("CreateDefaultProcessSecurityCommon(): Failed to allocate buffer for DACL\n");
Success = FALSE;
goto Quit;
}
@@ -253,7 +253,7 @@ CreateDefaultProcessSecurityCommon(
/* Initialize the DACL */
if (!InitializeAcl(Dacl, DaclSize, ACL_REVISION))
{
ERR("CreateDefaultProcessSecurityCommon(): Failed to initialize DACL (error code %d)\n", GetLastError());
ERR("CreateDefaultProcessSecurityCommon(): Failed to initialize DACL (error %lu)\n", GetLastError());
Success = FALSE;
goto Quit;
}
@@ -264,7 +264,7 @@ CreateDefaultProcessSecurityCommon(
GENERIC_ALL,
OwnerSid))
{
ERR("CreateDefaultProcessSecurityCommon(): Failed to set up ACE for owner (error code %d)\n", GetLastError());
ERR("CreateDefaultProcessSecurityCommon(): Failed to set up ACE for owner (error %lu)\n", GetLastError());
Success = FALSE;
goto Quit;
}
@@ -275,7 +275,7 @@ CreateDefaultProcessSecurityCommon(
GENERIC_ALL,
SystemSid))
{
ERR("CreateDefaultProcessSecurityCommon(): Failed to set up ACE for SYSTEM (error code %d)\n", GetLastError());
ERR("CreateDefaultProcessSecurityCommon(): Failed to set up ACE for SYSTEM (error %lu)\n", GetLastError());
Success = FALSE;
goto Quit;
}
@@ -283,7 +283,7 @@ CreateDefaultProcessSecurityCommon(
/* Initialize the descriptor in absolute format */
if (!InitializeSecurityDescriptor(&AbsoluteSd, SECURITY_DESCRIPTOR_REVISION))
{
ERR("CreateDefaultProcessSecurityCommon(): Failed to initialize absolute security descriptor (error code %d)\n", GetLastError());
ERR("CreateDefaultProcessSecurityCommon(): Failed to initialize absolute security descriptor (error %lu)\n", GetLastError());
Success = FALSE;
goto Quit;
}
@@ -291,7 +291,7 @@ CreateDefaultProcessSecurityCommon(
/* Set the DACL to the security descriptor */
if (!SetSecurityDescriptorDacl(&AbsoluteSd, TRUE, Dacl, FALSE))
{
ERR("CreateDefaultProcessSecurityCommon(): Failed to set up DACL to absolute security descriptor (error code %d)\n", GetLastError());
ERR("CreateDefaultProcessSecurityCommon(): Failed to set up DACL to absolute security descriptor (error %lu)\n", GetLastError());
Success = FALSE;
goto Quit;
}
@@ -299,7 +299,7 @@ CreateDefaultProcessSecurityCommon(
/* Set the owner for this descriptor */
if (!SetSecurityDescriptorOwner(&AbsoluteSd, OwnerSid, FALSE))
{
ERR("CreateDefaultProcessSecurityCommon(): Failed to set up owner to absolute security descriptor (error code %d)\n", GetLastError());
ERR("CreateDefaultProcessSecurityCommon(): Failed to set up owner to absolute security descriptor (error %lu)\n", GetLastError());
Success = FALSE;
goto Quit;
}
@@ -307,7 +307,7 @@ CreateDefaultProcessSecurityCommon(
/* Set the primary group for this descriptor */
if (!SetSecurityDescriptorGroup(&AbsoluteSd, PrimaryGroupSid, FALSE))
{
ERR("CreateDefaultProcessSecurityCommon(): Failed to set up group to absolute security descriptor (error code %d)\n", GetLastError());
ERR("CreateDefaultProcessSecurityCommon(): Failed to set up group to absolute security descriptor (error %lu)\n", GetLastError());
Success = FALSE;
goto Quit;
}
@@ -318,9 +318,10 @@ CreateDefaultProcessSecurityCommon(
* to hold the descriptor in a converted self
* relative format.
*/
if (!MakeSelfRelativeSD(&AbsoluteSd, NULL, &RelativeSDSize) && GetLastError() != ERROR_INSUFFICIENT_BUFFER)
if (MakeSelfRelativeSD(&AbsoluteSd, NULL, &RelativeSDSize) ||
(GetLastError() != ERROR_INSUFFICIENT_BUFFER))
{
ERR("CreateDefaultProcessSecurityCommon(): Unexpected error code (error code %d -- must be ERROR_INSUFFICIENT_BUFFER)\n", GetLastError());
ERR("CreateDefaultProcessSecurityCommon(): error %lu, expected ERROR_INSUFFICIENT_BUFFER\n", GetLastError());
Success = FALSE;
goto Quit;
}
@@ -331,7 +332,7 @@ CreateDefaultProcessSecurityCommon(
RelativeSDSize);
if (RelativeSD == NULL)
{
ERR("CreateDefaultProcessSecurityCommon(): Failed to allocate buffer for self relative descriptor!\n");
ERR("CreateDefaultProcessSecurityCommon(): Failed to allocate relative SD\n");
Success = FALSE;
goto Quit;
}
@@ -339,7 +340,8 @@ CreateDefaultProcessSecurityCommon(
/* Convert to a self relative format now */
if (!MakeSelfRelativeSD(&AbsoluteSd, RelativeSD, &RelativeSDSize))
{
ERR("CreateDefaultProcessSecurityCommon(): Failed to allocate relative SD, buffer too smal (error code %d)\n", GetLastError());
ERR("CreateDefaultProcessSecurityCommon(): Failed to allocate relative SD (error %lu)\n", GetLastError());
RtlFreeHeap(RtlGetProcessHeap(), 0, RelativeSD);
Success = FALSE;
goto Quit;
}
@@ -362,14 +364,6 @@ Quit:
if (Dacl != NULL)
RtlFreeHeap(RtlGetProcessHeap(), 0, Dacl);
if (Success == FALSE)
{
if (RelativeSD != NULL)
{
RtlFreeHeap(RtlGetProcessHeap(), 0, RelativeSD);
}
}
return Success;
}
@@ -412,7 +406,7 @@ InsertProcessSecurityCommon(
DACL_SECURITY_INFORMATION | OWNER_SECURITY_INFORMATION | GROUP_SECURITY_INFORMATION,
ProcessSecurity))
{
ERR("InsertProcessSecurityCommon(): Failed to set security for process (error code %d)\n", GetLastError());
ERR("InsertProcessSecurityCommon(): Failed to set security for process (error %lu)\n", GetLastError());
return FALSE;
}
@@ -421,7 +415,7 @@ InsertProcessSecurityCommon(
DACL_SECURITY_INFORMATION | OWNER_SECURITY_INFORMATION | GROUP_SECURITY_INFORMATION,
ThreadSecurity))
{
ERR("InsertProcessSecurityCommon(): Failed to set security for thread (error code %d)\n", GetLastError());
ERR("InsertProcessSecurityCommon(): Failed to set security for thread (error %lu)\n", GetLastError());
return FALSE;
}
@@ -730,7 +724,7 @@ CreateProcessAsUserCommon(
*/
if (!CreateDefaultProcessSecurityCommon(hToken, &DefaultSd))
{
ERR("Failed to create common security descriptor for the token for new process!\n");
ERR("Failed to create common security descriptor for the token for new process\n");
Success = FALSE;
goto Quit;
}
@@ -865,7 +859,7 @@ CreateProcessAsUserCommon(
ProcessSd,
ThreadSd))
{
ERR("Failed to set new security information for process and thread!\n");
ERR("Failed to set new security information for process and thread\n");
NtClose(hTokenDup);
Success = FALSE;
goto Quit;
@@ -881,7 +875,7 @@ CreateProcessAsUserCommon(
* ourselves as job done. The newly created process will use
* the default security context at this point anyway.
*/
TRACE("No token supplied, the process will use default security context!\n");
TRACE("No token supplied, the process will use default security context\n");
Success = TRUE;
Quit:
+26 -42
View File
@@ -126,7 +126,8 @@ static void release_np_event(RpcConnection_np *connection, HANDLE event)
* are given a subset of rights to access the pipe,
* whereas admins are given full power.
*/
static DWORD rpcrt4_create_pipe_security(PSECURITY_DESCRIPTOR *SecDesc)
static DWORD
rpcrt4_create_pipe_security(PSECURITY_DESCRIPTOR *SecDesc)
{
DWORD ErrCode;
PACL Dacl;
@@ -142,8 +143,9 @@ static DWORD rpcrt4_create_pipe_security(PSECURITY_DESCRIPTOR *SecDesc)
0, 0, 0, 0, 0, 0, 0,
&EveryoneSid))
{
ERR("rpcrt4_create_pipe_security(): Failed to allocate Everyone SID (error code %d)\n", GetLastError());
return GetLastError();
ErrCode = GetLastError();
ERR("rpcrt4_create_pipe_security(): Failed to allocate Everyone SID (error %lu)\n", ErrCode);
return ErrCode;
}
if (!AllocateAndInitializeSid(&NtAuthority,
@@ -152,8 +154,8 @@ static DWORD rpcrt4_create_pipe_security(PSECURITY_DESCRIPTOR *SecDesc)
0, 0, 0, 0, 0, 0, 0,
&AnonymousSid))
{
ERR("rpcrt4_create_pipe_security(): Failed to allocate Anonymous SID (error code %d)\n", GetLastError());
ErrCode = GetLastError();
ERR("rpcrt4_create_pipe_security(): Failed to allocate Anonymous SID (error %lu)\n", ErrCode);
goto Quit;
}
@@ -164,23 +166,23 @@ static DWORD rpcrt4_create_pipe_security(PSECURITY_DESCRIPTOR *SecDesc)
0, 0, 0, 0, 0, 0,
&AdminsSid))
{
ERR("rpcrt4_create_pipe_security(): Failed to allocate Admins SID (error code %d)\n", GetLastError());
ErrCode = GetLastError();
ERR("rpcrt4_create_pipe_security(): Failed to allocate Admins SID (error %lu)\n", ErrCode);
goto Quit;
}
AbsSD = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(SECURITY_DESCRIPTOR));
if (AbsSD == NULL)
{
ERR("rpcrt4_create_pipe_security(): Failed to allocate absolute SD!\n");
ErrCode = ERROR_OUTOFMEMORY;
ERR("rpcrt4_create_pipe_security(): Failed to allocate absolute SD\n");
goto Quit;
}
if (!InitializeSecurityDescriptor(AbsSD, SECURITY_DESCRIPTOR_REVISION))
{
ERR("rpcrt4_create_pipe_security(): Failed to create absolute SD (error code %d)\n", GetLastError());
ErrCode = GetLastError();
ERR("rpcrt4_create_pipe_security(): Failed to create absolute SD (error %lu)\n", ErrCode);
goto Quit;
}
@@ -189,19 +191,18 @@ static DWORD rpcrt4_create_pipe_security(PSECURITY_DESCRIPTOR *SecDesc)
sizeof(ACCESS_ALLOWED_ACE) + RtlLengthSid(AnonymousSid) +
sizeof(ACCESS_ALLOWED_ACE) + RtlLengthSid(AdminsSid);
Dacl = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, DaclSize);
if (Dacl == NULL)
{
ERR("rpcrt4_create_pipe_security(): Failed to allocate DACL!\n");
ErrCode = ERROR_OUTOFMEMORY;
ERR("rpcrt4_create_pipe_security(): Failed to allocate DACL\n");
goto Quit;
}
if (!InitializeAcl(Dacl, DaclSize, ACL_REVISION))
{
ERR("rpcrt4_create_pipe_security(): Failed to create DACL (error code %d)\n", GetLastError());
ErrCode = GetLastError();
ERR("rpcrt4_create_pipe_security(): Failed to create DACL (error %lu)\n", ErrCode);
goto Quit;
}
@@ -210,8 +211,8 @@ static DWORD rpcrt4_create_pipe_security(PSECURITY_DESCRIPTOR *SecDesc)
GENERIC_READ | GENERIC_WRITE | SYNCHRONIZE | READ_CONTROL,
EveryoneSid))
{
ERR("rpcrt4_create_pipe_security(): Failed to set up ACE for Everyone SID (error code %d)\n", GetLastError());
ErrCode = GetLastError();
ERR("rpcrt4_create_pipe_security(): Failed to set up ACE for Everyone SID (error %lu)\n", ErrCode);
goto Quit;
}
@@ -220,8 +221,8 @@ static DWORD rpcrt4_create_pipe_security(PSECURITY_DESCRIPTOR *SecDesc)
GENERIC_READ | GENERIC_WRITE | SYNCHRONIZE | READ_CONTROL,
AnonymousSid))
{
ERR("rpcrt4_create_pipe_security(): Failed to set up ACE for Anonymous SID (error code %d)\n", GetLastError());
ErrCode = GetLastError();
ERR("rpcrt4_create_pipe_security(): Failed to set up ACE for Anonymous SID (error %lu)\n", ErrCode);
goto Quit;
}
@@ -230,91 +231,74 @@ static DWORD rpcrt4_create_pipe_security(PSECURITY_DESCRIPTOR *SecDesc)
GENERIC_ALL,
AdminsSid))
{
ERR("rpcrt4_create_pipe_security(): Failed to set up ACE for Admins SID (error code %d)\n", GetLastError());
ErrCode = GetLastError();
ERR("rpcrt4_create_pipe_security(): Failed to set up ACE for Admins SID (error %lu)\n", ErrCode);
goto Quit;
}
if (!SetSecurityDescriptorDacl(AbsSD, TRUE, Dacl, FALSE))
{
ERR("rpcrt4_create_pipe_security(): Failed to set DACL to absolute SD (error code %d)\n", GetLastError());
ErrCode = GetLastError();
ERR("rpcrt4_create_pipe_security(): Failed to set DACL to absolute SD (error %lu)\n", ErrCode);
goto Quit;
}
if (!SetSecurityDescriptorOwner(AbsSD, AdminsSid, FALSE))
{
ERR("rpcrt4_create_pipe_security(): Failed to set SD owner (error code %d)\n", GetLastError());
ErrCode = GetLastError();
ERR("rpcrt4_create_pipe_security(): Failed to set SD owner (error %lu)\n", ErrCode);
goto Quit;
}
if (!SetSecurityDescriptorGroup(AbsSD, AdminsSid, FALSE))
{
ERR("rpcrt4_create_pipe_security(): Failed to set SD group (error code %d)\n", GetLastError());
ErrCode = GetLastError();
ERR("rpcrt4_create_pipe_security(): Failed to set SD group (error %lu)\n", ErrCode);
goto Quit;
}
if (!MakeSelfRelativeSD(AbsSD, NULL, &RelSDSize) && GetLastError() != ERROR_INSUFFICIENT_BUFFER)
if (MakeSelfRelativeSD(AbsSD, NULL, &RelSDSize) ||
(GetLastError() != ERROR_INSUFFICIENT_BUFFER))
{
ERR("rpcrt4_create_pipe_security(): Unexpected error code (error code %d -- must be ERROR_INSUFFICIENT_BUFFER)\n", GetLastError());
ErrCode = GetLastError();
ERR("rpcrt4_create_pipe_security(): error %lu, expected ERROR_INSUFFICIENT_BUFFER\n", ErrCode);
goto Quit;
}
RelSD = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, RelSDSize);
if (RelSD == NULL)
{
ERR("rpcrt4_create_pipe_security(): Failed to allocate relative SD!\n");
ErrCode = ERROR_OUTOFMEMORY;
ERR("rpcrt4_create_pipe_security(): Failed to allocate relative SD\n");
goto Quit;
}
if (!MakeSelfRelativeSD(AbsSD, RelSD, &RelSDSize) && GetLastError() == ERROR_INSUFFICIENT_BUFFER)
if (!MakeSelfRelativeSD(AbsSD, RelSD, &RelSDSize))
{
ERR("rpcrt4_create_pipe_security(): Failed to allocate relative SD, buffer too smal (expected size %lu)\n", RelSDSize);
ErrCode = ERROR_INSUFFICIENT_BUFFER;
ErrCode = GetLastError();
ERR("rpcrt4_create_pipe_security(): Failed to allocate relative SD (error %lu)\n", ErrCode);
HeapFree(GetProcessHeap(), 0, RelSD);
goto Quit;
}
TRACE("rpcrt4_create_pipe_security(): Success!\n");
*SecDesc = RelSD;
ErrCode = ERROR_SUCCESS;
Quit:
if (ErrCode != ERROR_SUCCESS)
{
if (RelSD != NULL)
{
HeapFree(GetProcessHeap(), 0, RelSD);
}
}
if (EveryoneSid != NULL)
{
FreeSid(EveryoneSid);
}
if (AnonymousSid != NULL)
{
FreeSid(AnonymousSid);
}
if (AdminsSid != NULL)
{
FreeSid(AdminsSid);
}
if (Dacl != NULL)
{
HeapFree(GetProcessHeap(), 0, Dacl);
}
if (AbsSD != NULL)
{
HeapFree(GetProcessHeap(), 0, AbsSD);
}
return ErrCode;
}
@@ -335,7 +319,7 @@ static RPC_STATUS rpcrt4_conn_create_pipe(RpcConnection *conn)
ErrCode = rpcrt4_create_pipe_security(&PipeSecDesc);
if (ErrCode != ERROR_SUCCESS)
{
ERR("rpcrt4_conn_create_pipe(): Pipe security descriptor creation failed!\n");
ERR("rpcrt4_conn_create_pipe(): Pipe security descriptor creation failed\n");
return RPC_S_CANT_CREATE_ENDPOINT;
}