https://git.reactos.org/?p=reactos.git;a=commitdiff;h=d3164a0f557bdc2cc9dfd…
commit d3164a0f557bdc2cc9dfd86612131f6cf180d462
Author: Joachim Henze <joachim.henze(a)reactos.org>
AuthorDate: Sun Jun 11 20:53:25 2023 +0200
Commit: GitHub <noreply(a)github.com>
CommitDate: Sun Jun 11 20:53:25 2023 +0200
[SHELL32] Strip leftover in CDefView::FillFileMenu() (#5330)
In 0.4.14-dev-955-g 1cf564c25f532c32f9fae891f17a70e62d5c1c14
Katayama experimented with populating explorers file-menu when no object is selected.
Later we found out, that none of the new entries introduced by that commit really
made sense and even created duplicates. So the commit was reverted by
0.4.15-dev-6039-g 0fa4edebd9125b9ca7a4256edbea7675e2152c7e 'Revert CDefView::FillFileMenu (#5278)' CORE-18429
But it seems that not all parts were properly reverted back then,
maybe because 6 lines of new code were written between the two lines in the meantime.
---
dll/win32/shell32/CDefView.cpp | 2 --
1 file changed, 2 deletions(-)
diff --git a/dll/win32/shell32/CDefView.cpp b/dll/win32/shell32/CDefView.cpp
index 880afa6e457..ebcfa767090 100644
--- a/dll/win32/shell32/CDefView.cpp
+++ b/dll/win32/shell32/CDefView.cpp
@@ -1348,8 +1348,6 @@ HRESULT CDefView::FillFileMenu()
DeleteMenu(hFileMenu, i, MF_BYPOSITION);
}
- m_cidl = m_ListView.GetSelectedCount();
-
/* In case we still have this left over, clean it up! */
if (m_pFileMenu)
{
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=0f9be539856b392b99dae…
commit 0f9be539856b392b99dae3d8bdecedb0b9de44fd
Author: George Bișoc <george.bisoc(a)reactos.org>
AuthorDate: Sun Jun 11 00:00:03 2023 +0200
Commit: George Bișoc <george.bisoc(a)reactos.org>
CommitDate: Sun Jun 11 18:14:02 2023 +0200
[WIN32K:NTUSER] Fix an unintialized user's token variable case
And remove the "!NT_SUCCESS(Status)" check which is excessive, the expected
status will always be STATUS_BUFFER_TOO_SMALL anyway. This should fix
some compilation warnings spotted by GCC. Courtesy goes to Hermes for letting
me know of these warnings.
---
win32ss/user/ntuser/security.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/win32ss/user/ntuser/security.c b/win32ss/user/ntuser/security.c
index 276d01bfe16..2ed4e1b66a0 100644
--- a/win32ss/user/ntuser/security.c
+++ b/win32ss/user/ntuser/security.c
@@ -2,7 +2,7 @@
* PROJECT: ReactOS Win32k subsystem
* LICENSE: GPL-2.0-or-later (https://spdx.org/licenses/GPL-2.0-or-later)
* PURPOSE: Security infrastructure of NTUSER component of Win32k
- * COPYRIGHT: Copyright 2022 George Bișoc <george.bisoc(a)reactos.org>
+ * COPYRIGHT: Copyright 2022-2023 George Bișoc <george.bisoc(a)reactos.org>
*/
/* INCLUDES ******************************************************************/
@@ -170,7 +170,7 @@ IntQueryUserSecurityIdentification(
_Out_ PTOKEN_USER *User)
{
NTSTATUS Status;
- PTOKEN_USER UserToken;
+ PTOKEN_USER UserToken = NULL;
HANDLE Token;
ULONG BufferLength;
@@ -196,7 +196,7 @@ IntQueryUserSecurityIdentification(
NULL,
0,
&BufferLength);
- if (!NT_SUCCESS(Status) && Status == STATUS_BUFFER_TOO_SMALL)
+ if (Status == STATUS_BUFFER_TOO_SMALL)
{
/*
* Allocate some memory for the buffer
@@ -212,6 +212,12 @@ IntQueryUserSecurityIdentification(
return STATUS_NO_MEMORY;
}
}
+ else if (!NT_SUCCESS(Status))
+ {
+ ERR("IntQueryUserSecurityIdentification(): Failed to query the necessary length for the buffer (Status 0x%08lx)!\n", Status);
+ ZwClose(Token);
+ return Status;
+ }
/* Query the user now as we have plenty of space to hold it */
Status = ZwQueryInformationToken(Token,
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=114bc2b96e48e991e5cff…
commit 114bc2b96e48e991e5cff01bee9c12f2f31f2dd5
Author: Oleg Dubinskiy <oleg.dubinskij30(a)gmail.com>
AuthorDate: Sat Jun 18 20:35:17 2022 +0200
Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
CommitDate: Sun Jun 11 13:44:56 2023 +0200
[NTGDI][NTUSER] Load DirectX graphics driver at system startup (#4551)
CORE-18221
Load the DirectX graphics kernel driver (dxg.sys) by win32k at WINSRV
initialization time, in NtUserInitialize(). Keep it always loaded in
memory, as on Windows, instead of loading it only by DirectX dlls.
This fixes the problem of acessing this driver: we need only to call
DxDdEnableDirectDraw() and do other stuff when DirectDraw/Direct3D is
required by anything. In other cases, it is called from win32k PDEV
functions when changing display mode (as in Windows). Since it's used
by other things too, it needs to be always loaded.
Otherwise, if it's not loaded, its APIs are not accessible when needed,
and execution fails.
For example, it fixes display mode change problem in VMWare, when a
new mode fails to be applied. Indeed, when it manages DirectDraw stuff,
it calls DXG routines, and therefore fails if dxg.sys isn't loaded
in memory at this moment.
- Implement InitializeGreCSRSS() initialization routine, that initializes
supplemental NTGDI/GRE data once CSRSS and WINSRV are loaded:
* Call DxDdStartupDxGraphics() inside it, which loads dxg.sys.
* Additionally, move fonts and language ID initialization there, from
win32k!DriverEntry. Confirmed by analysis on Windows.
- Call InitializeGreCSRSS() in NtUserInitialize() main initialization routine
(called by WINSRV initialization).
Moved to NTGDI from previously NTUSER place:
Co-authored-by: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
---
win32ss/gdi/ntgdi/init.c | 28 +++++++++++++++++++++++++---
win32ss/gdi/ntgdi/misc.h | 2 ++
win32ss/user/ntuser/main.c | 10 ----------
win32ss/user/ntuser/ntuser.c | 9 +++------
4 files changed, 30 insertions(+), 19 deletions(-)
diff --git a/win32ss/gdi/ntgdi/init.c b/win32ss/gdi/ntgdi/init.c
index 35bab0f6029..44afa1219df 100644
--- a/win32ss/gdi/ntgdi/init.c
+++ b/win32ss/gdi/ntgdi/init.c
@@ -7,10 +7,9 @@
*/
#include <win32k.h>
+DBG_DEFAULT_CHANNEL(UserMisc);
-#define NDEBUG
-#include <debug.h>
-#include <kdros.h>
+USHORT gusLanguageID;
BOOL NTAPI GDI_CleanupForProcess(struct _EPROCESS *Process);
@@ -77,6 +76,29 @@ GdiThreadDestroy(PETHREAD Thread)
}
+BOOL
+InitializeGreCSRSS(VOID)
+{
+ /* Initialize DirectX graphics driver */
+ if (DxDdStartupDxGraphics(0, NULL, 0, NULL, NULL, gpepCSRSS) != STATUS_SUCCESS)
+ {
+ ERR("Unable to initialize DirectX graphics\n");
+ return FALSE;
+ }
+
+ /* Get global language ID */
+ gusLanguageID = UserGetLanguageID();
+
+ /* Initialize FreeType library */
+ if (!InitFontSupport())
+ {
+ ERR("Unable to initialize font support\n");
+ return FALSE;
+ }
+
+ return TRUE;
+}
+
/*
* @implemented
*/
diff --git a/win32ss/gdi/ntgdi/misc.h b/win32ss/gdi/ntgdi/misc.h
index 6757acce4c0..06fa17b8255 100644
--- a/win32ss/gdi/ntgdi/misc.h
+++ b/win32ss/gdi/ntgdi/misc.h
@@ -23,7 +23,9 @@ extern BOOL APIENTRY IntEngLeave(PINTENG_ENTER_LEAVE EnterLeave);
extern HGDIOBJ StockObjects[];
extern USHORT gusLanguageID;
+BOOL InitializeGreCSRSS(VOID);
USHORT FASTCALL UserGetLanguageID(VOID);
+
PVOID APIENTRY HackSecureVirtualMemory(IN PVOID,IN SIZE_T,IN ULONG,OUT PVOID *);
VOID APIENTRY HackUnsecureVirtualMemory(IN PVOID);
diff --git a/win32ss/user/ntuser/main.c b/win32ss/user/ntuser/main.c
index bdb36d45236..235e8aee696 100644
--- a/win32ss/user/ntuser/main.c
+++ b/win32ss/user/ntuser/main.c
@@ -26,7 +26,6 @@ NTSTATUS GdiThreadDestroy(PETHREAD Thread);
PSERVERINFO gpsi = NULL; // Global User Server Information.
-USHORT gusLanguageID;
PPROCESSINFO ppiScrnSaver;
PPROCESSINFO gppiList = NULL;
@@ -1051,15 +1050,6 @@ DriverEntry(
NT_ROF(InitTimerImpl());
NT_ROF(InitDCEImpl());
- gusLanguageID = UserGetLanguageID();
-
- /* Initialize FreeType library */
- if (!InitFontSupport())
- {
- DPRINT1("Unable to initialize font support\n");
- return Status;
- }
-
return STATUS_SUCCESS;
}
diff --git a/win32ss/user/ntuser/ntuser.c b/win32ss/user/ntuser/ntuser.c
index fd2f5f97e93..6643495606d 100644
--- a/win32ss/user/ntuser/ntuser.c
+++ b/win32ss/user/ntuser/ntuser.c
@@ -198,12 +198,9 @@ NtUserInitialize(
// Initialize Power Request List (use hPowerRequestEvent).
// Initialize Media Change (use hMediaRequestEvent).
-// InitializeGreCSRSS();
-// {
-// Startup DxGraphics.
-// calls ** UserGetLanguageID() and sets it **.
-// Enables Fonts drivers, Initialize Font table & Stock Fonts.
-// }
+ /* Initialize various GDI stuff (DirectX, fonts, language ID etc.) */
+ if (!InitializeGreCSRSS())
+ return STATUS_UNSUCCESSFUL;
/* Initialize USER */
Status = UserInitialize();
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=29a706fc5ad0a1b169688…
commit 29a706fc5ad0a1b169688b86d19f12ef1552c07a
Author: Sophie Lemos <slemos(a)fastmail.com>
AuthorDate: Fri May 26 17:42:36 2023 +0100
Commit: Stanislav Motylkov <x86corez(a)gmail.com>
CommitDate: Sun Jun 11 13:13:11 2023 +0300
[NTOS:PNP] Fix bug causing all devices be considered as already existing
We should compare against DeviceObject as DeviceInstance is never NULL.
Fix a resource leak as well. The bug CORE-18983 seems to lay somewhere
else though, I just stumbled upon this one while researching it.
Note there is a BSOD in the PnP manager on reboot after the driver
installation failure, but it seems it was uncovered by the fix
as opposed to caused by it.
---
ntoskrnl/io/pnpmgr/plugplay.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/ntoskrnl/io/pnpmgr/plugplay.c b/ntoskrnl/io/pnpmgr/plugplay.c
index 46585458b84..461328c8651 100644
--- a/ntoskrnl/io/pnpmgr/plugplay.c
+++ b/ntoskrnl/io/pnpmgr/plugplay.c
@@ -218,15 +218,14 @@ IopInitializeDevice(
/* Leave, if the device already exists */
DeviceObject = IopGetDeviceObjectFromDeviceInstance(&DeviceInstance);
- if (DeviceInstance.Buffer != NULL)
+ if (DeviceObject != NULL)
{
DPRINT1("Device %wZ already exists!\n", &DeviceInstance);
+ ObDereferenceObject(DeviceObject);
Status = STATUS_SUCCESS;
goto done;
}
- ObDereferenceObject(DeviceObject);
-
DPRINT("Device %wZ does not exist!\n", &DeviceInstance);
/* Create a device node for the device instance */
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=fa388640ca3391a708b7c…
commit fa388640ca3391a708b7c7a10907efda86556d88
Author: Andrei Miloiu <miloiuandrei(a)gmail.com>
AuthorDate: Sat Jun 10 23:03:23 2023 +0300
Commit: GitHub <noreply(a)github.com>
CommitDate: Sat Jun 10 20:03:23 2023 +0000
[GETUNAME] Update Romanian translation (#5323)
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=59e74584ac5c48e9edb2b…
commit 59e74584ac5c48e9edb2bf494c2224bd5ca6fa90
Author: George Bișoc <george.bisoc(a)reactos.org>
AuthorDate: Tue Jun 6 19:06:11 2023 +0200
Commit: George Bișoc <george.bisoc(a)reactos.org>
CommitDate: Fri Jun 9 11:53:56 2023 +0200
[NTOS:SE] Refactor SeTokenCanImpersonate
- Refactor most of the code, since there's quite some stuff that don't make much sense.
For instance ImpersonationLevel is basically the requested impersonation level a
server asks for. PsImpersonateClient doesn't explicitly say that SecurityAnonymous
and SecurityIdentification are not allowed. If the server was to give such levels
it simply means it doesn't want to impersonate the client.
Another thing that doesn't make much sense is that we check if the client is
associated with an anonymous token, then avoid impersonating regular anonymous
tokens that weren't created by the system. Only system can create such tokens
and an anonymous token basically means a token with hidden security info.
- Check that the server is within the same client logon session.
- If the server is granted the SeImpersonatePrivilege privilege, allow impersonation
regardless of the conditions we want to check for.
- Update the documentation and code comments.
---
ntoskrnl/se/token.c | 127 ++++++++++++++++++++++++++++++++++------------------
1 file changed, 83 insertions(+), 44 deletions(-)
diff --git a/ntoskrnl/se/token.c b/ntoskrnl/se/token.c
index bdb360e8672..dae5a411dff 100644
--- a/ntoskrnl/se/token.c
+++ b/ntoskrnl/se/token.c
@@ -2158,22 +2158,47 @@ SeTokenIsWriteRestricted(
/**
* @brief
- * Ensures that client impersonation can occur by checking if the token
- * we're going to assign as the impersonation token can be actually impersonated
- * in the first place. The routine is used primarily by PsImpersonateClient.
+ * Determines whether the server is allowed to impersonate on behalf
+ * of a client or not. For further details, see Remarks.
*
* @param[in] ProcessToken
- * Token from a process.
+ * A pointer to the primary access token of the server process
+ * that requests impersonation of the client target.
*
* @param[in] TokenToImpersonate
- * Token that we are going to impersonate.
+ * A pointer to an access token that represents a client that is to
+ * be impersonated.
*
* @param[in] ImpersonationLevel
- * Security impersonation level grade.
+ * The requested impersonation level.
*
* @return
* Returns TRUE if the conditions checked are met for token impersonation,
* FALSE otherwise.
+ *
+ * @remarks
+ * The server has to meet the following criteria in order to impersonate
+ * a client, that is:
+ *
+ * - The server must not impersonate a client beyond the level that
+ * the client imposed on itself.
+ *
+ * - The server must be authenticated on the same logon session of
+ * the target client.
+ *
+ * - IF NOT then the server's user ID has to match to that of the
+ * target client.
+ *
+ * - The server must not be restricted in order to impersonate a
+ * client that is not restricted.
+ *
+ * If the associated access token that represents the security properties
+ * of the server is granted the SeImpersonatePrivilege privilege the server
+ * is given immediate impersonation, regardless of the conditions above.
+ * If the client in question is associated with an anonymous token then
+ * the server is given immediate impersonation. Or if the server simply
+ * doesn't ask for impersonation but instead it wants to get the security
+ * identification of a client, the server is given immediate impersonation.
*/
BOOLEAN
NTAPI
@@ -2186,73 +2211,87 @@ SeTokenCanImpersonate(
PAGED_CODE();
/*
- * SecurityAnonymous and SecurityIdentification levels do not
- * allow impersonation.
+ * The server may want to obtain identification details of a client
+ * instead of impersonating so just give the server a pass.
*/
- if (ImpersonationLevel == SecurityAnonymous ||
- ImpersonationLevel == SecurityIdentification)
+ if (ImpersonationLevel < SecurityIdentification)
{
- return FALSE;
+ DPRINT("The server doesn't ask for impersonation\n");
+ return TRUE;
}
/* Time to lock our tokens */
SepAcquireTokenLockShared(ProcessToken);
SepAcquireTokenLockShared(TokenToImpersonate);
- /* What kind of authentication ID does the token have? */
+ /*
+ * As the name implies, an anonymous token has invisible security
+ * identification details. By the general rule these tokens do not
+ * pose a danger in terms of power escalation so give the server a pass.
+ */
if (RtlEqualLuid(&TokenToImpersonate->AuthenticationId,
&SeAnonymousAuthenticationId))
{
- /*
- * OK, it looks like the token has an anonymous
- * authentication. Is that token created by the system?
- */
- if (TokenToImpersonate->TokenSource.SourceName != SeSystemTokenSource.SourceName &&
- !RtlEqualLuid(&TokenToImpersonate->TokenSource.SourceIdentifier, &SeSystemTokenSource.SourceIdentifier))
- {
- /* It isn't, we can't impersonate regular tokens */
- DPRINT("SeTokenCanImpersonate(): Token has an anonymous authentication ID, can't impersonate!\n");
- CanImpersonate = FALSE;
- goto Quit;
- }
+ DPRINT("The token to impersonate has an anonymous authentication ID, allow impersonation either way\n");
+ CanImpersonate = TRUE;
+ goto Quit;
}
- /* Are the SID values from both tokens equal? */
- if (!RtlEqualSid(ProcessToken->UserAndGroups->Sid,
- TokenToImpersonate->UserAndGroups->Sid))
+ /* Allow impersonation for the process server if it's granted the impersonation privilege */
+ if ((ProcessToken->TokenFlags & TOKEN_HAS_IMPERSONATE_PRIVILEGE) != 0)
{
- /* They aren't, bail out */
- DPRINT("SeTokenCanImpersonate(): Tokens SIDs are not equal!\n");
- CanImpersonate = FALSE;
+ DPRINT("The process is granted the impersonation privilege, allow impersonation\n");
+ CanImpersonate = TRUE;
goto Quit;
}
/*
- * Make sure the tokens aren't diverged in terms of
- * restrictions, that is, one token is restricted
- * but the other one isn't.
+ * Deny impersonation for the server if it wants to impersonate a client
+ * beyond what the impersonation level originally permits.
*/
- if (SeTokenIsRestricted(ProcessToken) !=
- SeTokenIsRestricted(TokenToImpersonate))
+ if (ImpersonationLevel > TokenToImpersonate->ImpersonationLevel)
{
- /*
- * One token is restricted so we cannot
- * continue further at this point, bail out.
- */
- DPRINT("SeTokenCanImpersonate(): One token is restricted, can't continue!\n");
+ DPRINT1("Cannot impersonate a client above the permitted impersonation level!\n");
CanImpersonate = FALSE;
goto Quit;
}
+ /* Is the server authenticated on the same client originating session? */
+ if (!RtlEqualLuid(&ProcessToken->AuthenticationId,
+ &TokenToImpersonate->OriginatingLogonSession))
+ {
+ /* It's not, check that at least both the server and client are the same user */
+ if (!RtlEqualSid(ProcessToken->UserAndGroups[0].Sid,
+ TokenToImpersonate->UserAndGroups[0].Sid))
+ {
+ DPRINT1("Server and client aren't the same user!\n");
+ CanImpersonate = FALSE;
+ goto Quit;
+ }
+
+ /*
+ * Make sure the tokens haven't diverged in terms of restrictions
+ * that is one token is restricted but the other one isn't. If that
+ * would have been the case then the server would have impersonated
+ * a less restricted client thus potentially triggering an elevation,
+ * which is not what we want.
+ */
+ if (SeTokenIsRestricted(ProcessToken) !=
+ SeTokenIsRestricted(TokenToImpersonate))
+ {
+ DPRINT1("Attempting to impersonate a less restricted client token, bail out!\n");
+ CanImpersonate = FALSE;
+ goto Quit;
+ }
+ }
+
/* If we've reached that far then we can impersonate! */
- DPRINT("SeTokenCanImpersonate(): We can impersonate.\n");
+ DPRINT("We can impersonate\n");
CanImpersonate = TRUE;
Quit:
- /* We're done, unlock the tokens now */
- SepReleaseTokenLock(ProcessToken);
SepReleaseTokenLock(TokenToImpersonate);
-
+ SepReleaseTokenLock(ProcessToken);
return CanImpersonate;
}