Ros-diffs September 2024

ros-diffs@reactos.org

23 participants
118 discussions

[reactos] 01/01: [SHELL32] Store target FS attributes in .lnk header (#7302)

by Whindmar Saksit

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=bad0dd599179eb7212749… commit bad0dd599179eb7212749a0aaae1275fd20b570f Author: Whindmar Saksit <whindsaks(a)proton.me> AuthorDate: Thu Sep 12 19:28:15 2024 +0200 Commit: GitHub <noreply(a)github.com> CommitDate: Thu Sep 12 19:28:15 2024 +0200 [SHELL32] Store target FS attributes in .lnk header (#7302) --- dll/win32/shell32/CShellLink.cpp | 44 ++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/dll/win32/shell32/CShellLink.cpp b/dll/win32/shell32/CShellLink.cpp index ca1f6b1a3ef..d438fa204ff 100644 --- a/dll/win32/shell32/CShellLink.cpp +++ b/dll/win32/shell32/CShellLink.cpp @@ -882,6 +882,21 @@ HRESULT STDMETHODCALLTYPE CShellLink::Save(IStream *stm, BOOL fClearDirty) m_Header.dwSize = sizeof(m_Header); m_Header.clsid = CLSID_ShellLink; + /* Store target attributes */ + WIN32_FIND_DATAW wfd = {}; + WCHAR FsTarget[MAX_PATH]; + if (GetPath(FsTarget, _countof(FsTarget), NULL, 0) == S_OK && PathFileExistsW(FsTarget)) + { + HANDLE hFind = FindFirstFileW(FsTarget, &wfd); + if (hFind != INVALID_HANDLE_VALUE) + FindClose(hFind); + } + m_Header.dwFileAttributes = wfd.dwFileAttributes; + m_Header.ftCreationTime = wfd.ftCreationTime; + m_Header.ftLastAccessTime = wfd.ftLastAccessTime; + m_Header.ftLastWriteTime = wfd.ftLastWriteTime; + m_Header.nFileSizeLow = wfd.nFileSizeLow; + /* * Reset the flags: keep only the flags related to data blocks as they were * already set in accordance by the different mutator member functions. @@ -1077,7 +1092,7 @@ HRESULT STDMETHODCALLTYPE CShellLink::GetPath(LPSTR pszFile, INT cchMaxPath, WIN this, pszFile, cchMaxPath, pfd, fFlags, debugstr_w(m_sPath)); /* Allocate a temporary UNICODE buffer */ - pszFileW = (LPWSTR)HeapAlloc(GetProcessHeap(), 0, cchMaxPath * sizeof(WCHAR)); + pszFileW = (LPWSTR)HeapAlloc(GetProcessHeap(), 0, max(cchMaxPath, MAX_PATH) * sizeof(WCHAR)); if (!pszFileW) return E_OUTOFMEMORY; @@ -2690,31 +2705,24 @@ INT_PTR CALLBACK ExtendedShortcutProc(HWND hwndDlg, UINT uMsg, return FALSE; } -/************************************************************************** -* SH_GetTargetTypeByPath -* -* Function to get target type by passing full path to it -*/ -void SH_GetTargetTypeByPath(LPCWSTR lpcwFullPath, LPWSTR szBuf, UINT cchBuf) +static void GetTypeDescriptionByPath(PCWSTR pszFullPath, DWORD fAttributes, PWSTR szBuf, UINT cchBuf) { - LPCWSTR pwszExt; - BOOL fFolderTarget = PathIsDirectoryW(lpcwFullPath); - DWORD fAttribs = fFolderTarget ? FILE_ATTRIBUTE_DIRECTORY : 0; + if (fAttributes == INVALID_FILE_ATTRIBUTES && !PathFileExistsAndAttributesW(pszFullPath, &fAttributes)) + fAttributes = 0; - /* Get file information */ SHFILEINFOW fi; - if (!SHGetFileInfoW(lpcwFullPath, fAttribs, &fi, sizeof(fi), SHGFI_TYPENAME | SHGFI_USEFILEATTRIBUTES)) + if (!SHGetFileInfoW(pszFullPath, fAttributes, &fi, sizeof(fi), SHGFI_TYPENAME | SHGFI_USEFILEATTRIBUTES)) { - ERR("SHGetFileInfoW failed for %ls (%lu)\n", lpcwFullPath, GetLastError()); - fi.szTypeName[0] = L'\0'; - fi.hIcon = NULL; + ERR("SHGetFileInfoW failed for %ls (%lu)\n", pszFullPath, GetLastError()); + fi.szTypeName[0] = UNICODE_NULL; } - pwszExt = fFolderTarget ? L"" : PathFindExtensionW(lpcwFullPath); + BOOL fFolder = (fAttributes & FILE_ATTRIBUTE_DIRECTORY); + LPCWSTR pwszExt = fFolder ? L"" : PathFindExtensionW(pszFullPath); if (pwszExt[0]) { if (!fi.szTypeName[0]) - StringCchPrintfW(szBuf, cchBuf,L"%s ", pwszExt + 1); + StringCchPrintfW(szBuf, cchBuf, L"%s", pwszExt + 1); else StringCchPrintfW(szBuf, cchBuf, L"%s (%s)", fi.szTypeName, pwszExt); } @@ -2764,7 +2772,7 @@ BOOL CShellLink::OnInitDialog(HWND hwndDlg, HWND hwndFocus, LPARAM lParam) if (m_sPath) { WCHAR buf[MAX_PATH]; - SH_GetTargetTypeByPath(m_sPath, buf, _countof(buf)); + GetTypeDescriptionByPath(m_sPath, m_Header.dwFileAttributes, buf, _countof(buf)); SetDlgItemTextW(hwndDlg, IDC_SHORTCUT_TYPE_EDIT, buf); }

3 months, 2 weeks

[reactos] 06/06: [KMTESTS:SE] Fix failing tests.

by Thomas Faber

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=44bdafa17e2b870c570ea… commit 44bdafa17e2b870c570ea15f3bdb56decfd7aaa5 Author: Thomas Faber <thomas.faber(a)reactos.org> AuthorDate: Sat May 27 22:37:47 2023 -0400 Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org> CommitDate: Thu Sep 12 17:44:13 2024 +0300 [KMTESTS:SE] Fix failing tests. --- modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c b/modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c index f58bbc3b17e..950cb78e35c 100644 --- a/modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c +++ b/modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c @@ -328,6 +328,10 @@ START_TEST(SeQueryInfoToken) // Testing SeFreePrivileges // //----------------------------------------------------------------// + // FIXME: KernelMode will automatically get all access granted without + // getting Privileges filled in. For UserMode, Privileges will only get + // filled if either WRITE_OWNER or ACCESS_SYSTEM_SECURITY is requested + // and granted. So this doesn't really test SeFreePrivileges. Privileges = NULL; Checker = SeAccessCheck( AccessState->SecurityDescriptor, @@ -342,7 +346,7 @@ START_TEST(SeQueryInfoToken) &Status ); ok(Checker, "Checker is NULL\n"); - ok((Privileges != NULL), "Privileges is NULL\n"); + ok(Privileges == NULL, "Privileges is not NULL\n"); if (Privileges) { trace("AuxData->PrivilegesUsed->PrivilegeCount = %d ; Privileges->PrivilegeCount = %d\n", @@ -390,6 +394,8 @@ START_TEST(SeQueryInfoToken) // Call SeFreePrivileges again + // FIXME: See other SeAccessCheck call above, we're not really testing + // SeFreePrivileges here. Privileges = NULL; Checker = SeAccessCheck( AccessState->SecurityDescriptor, @@ -404,7 +410,7 @@ START_TEST(SeQueryInfoToken) &Status ); ok(Checker, "Checker is NULL\n"); - ok((Privileges != NULL), "Privileges is NULL\n"); + ok(Privileges == NULL, "Privileges is not NULL\n"); if (Privileges) { trace("AuxData->PrivilegesUsed->PrivilegeCount = %d ; Privileges->PrivilegeCount = %d\n",

3 months, 2 weeks

[reactos] 05/06: [NTOS:SE] Mark output parameters as such.

by Thomas Faber

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=bf6af0f52eba23b2ca667… commit bf6af0f52eba23b2ca6677813792baa80c6d1f82 Author: Thomas Faber <thomas.faber(a)reactos.org> AuthorDate: Sat May 27 22:29:16 2023 -0400 Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org> CommitDate: Thu Sep 12 17:44:13 2024 +0300 [NTOS:SE] Mark output parameters as such. --- ntoskrnl/se/access.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/ntoskrnl/se/access.c b/ntoskrnl/se/access.c index d1ecd5bd571..7a9eedeba66 100644 --- a/ntoskrnl/se/access.c +++ b/ntoskrnl/se/access.c @@ -23,10 +23,10 @@ * @param[in] Process * Valid process object where subject context is to be captured. * - * @param[in,out] AccessState + * @param[out] AccessState * An initialized returned parameter to an access state. * - * @param[in] AuxData + * @param[out] AuxData * Auxiliary security data for access state. * * @param[in] Access @@ -43,8 +43,8 @@ NTAPI SeCreateAccessStateEx( _In_ PETHREAD Thread, _In_ PEPROCESS Process, - _Inout_ PACCESS_STATE AccessState, - _In_ PAUX_ACCESS_DATA AuxData, + _Out_ PACCESS_STATE AccessState, + _Out_ __drv_aliasesMem PAUX_ACCESS_DATA AuxData, _In_ ACCESS_MASK Access, _In_ PGENERIC_MAPPING GenericMapping) { @@ -101,10 +101,10 @@ SeCreateAccessStateEx( * @brief * Creates an access state. * - * @param[in,out] AccessState + * @param[out] AccessState * An initialized returned parameter to an access state. * - * @param[in] AuxData + * @param[out] AuxData * Auxiliary security data for access state. * * @param[in] Access @@ -119,8 +119,8 @@ SeCreateAccessStateEx( NTSTATUS NTAPI SeCreateAccessState( - _Inout_ PACCESS_STATE AccessState, - _In_ PAUX_ACCESS_DATA AuxData, + _Out_ PACCESS_STATE AccessState, + _Out_ __drv_aliasesMem PAUX_ACCESS_DATA AuxData, _In_ ACCESS_MASK Access, _In_ PGENERIC_MAPPING GenericMapping) {

3 months, 2 weeks

[reactos] 04/06: [NDK] Match AUX_ACCESS_DATA definition with publicly available version.

by Thomas Faber

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=156053cafd3d325ac5d6e… commit 156053cafd3d325ac5d6e6c341dae55bbb99b88c Author: Thomas Faber <thomas.faber(a)reactos.org> AuthorDate: Sat May 27 18:24:29 2023 -0400 Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org> CommitDate: Thu Sep 12 17:44:13 2024 +0300 [NDK] Match AUX_ACCESS_DATA definition with publicly available version. Looks like public symbols contain this structure starting with Win7, so we can deduce what it looked like in Win2003. Note that our previous definition was missing a second ULONG at the end, which can be seen in the SeQueryInfoToken kmtest -- if you allocated only sizeof(AUX_ACCESS_DATA), the test would crash with a 4 byte buffer overflow. --- .../rostests/kmtests/ntos_se/SeQueryInfoToken.c | 32 +++++++++++----------- ntoskrnl/ob/obhandle.c | 6 ++-- ntoskrnl/se/access.c | 4 +-- ntoskrnl/se/priv.c | 14 +++++----- sdk/include/ndk/setypes.h | 19 +++++++++++-- 5 files changed, 45 insertions(+), 30 deletions(-) diff --git a/modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c b/modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c index 2ddf864d086..f58bbc3b17e 100644 --- a/modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c +++ b/modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c @@ -279,7 +279,7 @@ START_TEST(SeQueryInfoToken) // Testing SeAppendPrivileges // //----------------------------------------------------------------// - InitialPrivilegeCount = AuxData->PrivilegeSet->PrivilegeCount; + InitialPrivilegeCount = AuxData->PrivilegesUsed->PrivilegeCount; trace("Initial privilege count = %lu\n", InitialPrivilegeCount); // Testing SeAppendPrivileges. Must change PrivilegeCount to 2 (1 + 1) @@ -291,7 +291,7 @@ START_TEST(SeQueryInfoToken) Status = SeAppendPrivileges(AccessState, NewPrivilegeSet); ok(Status == STATUS_SUCCESS, "SeAppendPrivileges failed\n"); - ok_eq_ulong(AuxData->PrivilegeSet->PrivilegeCount, InitialPrivilegeCount + 1); + ok_eq_ulong(AuxData->PrivilegesUsed->PrivilegeCount, InitialPrivilegeCount + 1); ExFreePoolWithTag(NewPrivilegeSet, 'QSmK'); //----------------------------------------------------------------// @@ -305,7 +305,7 @@ START_TEST(SeQueryInfoToken) Status = SeAppendPrivileges(AccessState, NewPrivilegeSet); ok(Status == STATUS_SUCCESS, "SeAppendPrivileges failed\n"); - ok_eq_ulong(AuxData->PrivilegeSet->PrivilegeCount, InitialPrivilegeCount + 5); + ok_eq_ulong(AuxData->PrivilegesUsed->PrivilegeCount, InitialPrivilegeCount + 5); ExFreePoolWithTag(NewPrivilegeSet, 'QSmK'); //----------------------------------------------------------------// @@ -313,14 +313,14 @@ START_TEST(SeQueryInfoToken) //----------------------------------------------------------------// // KPROCESSOR_MODE is set to KernelMode ===> Always return TRUE - ok(SePrivilegeCheck(AuxData->PrivilegeSet, &(AccessState->SubjectSecurityContext), KernelMode), "SePrivilegeCheck failed with KernelMode mode arg\n"); + ok(SePrivilegeCheck(AuxData->PrivilegesUsed, &(AccessState->SubjectSecurityContext), KernelMode), "SePrivilegeCheck failed with KernelMode mode arg\n"); // and call it again - ok(SePrivilegeCheck(AuxData->PrivilegeSet, &(AccessState->SubjectSecurityContext), KernelMode), "SePrivilegeCheck failed with KernelMode mode arg\n"); + ok(SePrivilegeCheck(AuxData->PrivilegesUsed, &(AccessState->SubjectSecurityContext), KernelMode), "SePrivilegeCheck failed with KernelMode mode arg\n"); //----------------------------------------------------------------// // KPROCESSOR_MODE is set to UserMode. Expect false - ok(!SePrivilegeCheck(AuxData->PrivilegeSet, &(AccessState->SubjectSecurityContext), UserMode), "SePrivilegeCheck unexpected success with UserMode arg\n"); + ok(!SePrivilegeCheck(AuxData->PrivilegesUsed, &(AccessState->SubjectSecurityContext), UserMode), "SePrivilegeCheck unexpected success with UserMode arg\n"); //----------------------------------------------------------------// @@ -345,8 +345,8 @@ START_TEST(SeQueryInfoToken) ok((Privileges != NULL), "Privileges is NULL\n"); if (Privileges) { - trace("AuxData->PrivilegeSet->PrivilegeCount = %d ; Privileges->PrivilegeCount = %d\n", - AuxData->PrivilegeSet->PrivilegeCount, Privileges->PrivilegeCount); + trace("AuxData->PrivilegesUsed->PrivilegeCount = %d ; Privileges->PrivilegeCount = %d\n", + AuxData->PrivilegesUsed->PrivilegeCount, Privileges->PrivilegeCount); } if (Privileges) SeFreePrivileges(Privileges); @@ -375,16 +375,16 @@ START_TEST(SeQueryInfoToken) NewPrivilegeSet->PrivilegeCount = 14; ok((SeAppendPrivileges(AccessState, NewPrivilegeSet)) == STATUS_SUCCESS, "SeAppendPrivileges failed\n"); - ok_eq_ulong(AuxData->PrivilegeSet->PrivilegeCount, InitialPrivilegeCount + 19); + ok_eq_ulong(AuxData->PrivilegesUsed->PrivilegeCount, InitialPrivilegeCount + 19); ExFreePoolWithTag(NewPrivilegeSet, 'QSmK'); - for (i = 0; i < AuxData->PrivilegeSet->PrivilegeCount; i++) + for (i = 0; i < AuxData->PrivilegesUsed->PrivilegeCount; i++) { - AuxData->PrivilegeSet->Privilege[i].Attributes = TPrivileges->Privileges[i].Attributes; - AuxData->PrivilegeSet->Privilege[i].Luid = TPrivileges->Privileges[i].Luid; + AuxData->PrivilegesUsed->Privilege[i].Attributes = TPrivileges->Privileges[i].Attributes; + AuxData->PrivilegesUsed->Privilege[i].Luid = TPrivileges->Privileges[i].Luid; } - //trace("AccessState->privCount = %u\n\n", ((PAUX_ACCESS_DATA)(AccessState->AuxData))->PrivilegeSet->PrivilegeCount); + //trace("AccessState->privCount = %u\n\n", ((PAUX_ACCESS_DATA)(AccessState->AuxData))->PrivilegesUsed->PrivilegeCount); - ok(SePrivilegeCheck(AuxData->PrivilegeSet, &(AccessState->SubjectSecurityContext), UserMode), "SePrivilegeCheck fails in UserMode, but I wish it will success\n"); + ok(SePrivilegeCheck(AuxData->PrivilegesUsed, &(AccessState->SubjectSecurityContext), UserMode), "SePrivilegeCheck fails in UserMode, but I wish it will success\n"); } } @@ -407,8 +407,8 @@ START_TEST(SeQueryInfoToken) ok((Privileges != NULL), "Privileges is NULL\n"); if (Privileges) { - trace("AuxData->PrivilegeSet->PrivilegeCount = %d ; Privileges->PrivilegeCount = %d\n", - AuxData->PrivilegeSet->PrivilegeCount, Privileges->PrivilegeCount); + trace("AuxData->PrivilegesUsed->PrivilegeCount = %d ; Privileges->PrivilegeCount = %d\n", + AuxData->PrivilegesUsed->PrivilegeCount, Privileges->PrivilegeCount); } if (Privileges) SeFreePrivileges(Privileges); diff --git a/ntoskrnl/ob/obhandle.c b/ntoskrnl/ob/obhandle.c index 28a2fc77e36..530e32fbfaf 100644 --- a/ntoskrnl/ob/obhandle.c +++ b/ntoskrnl/ob/obhandle.c @@ -1647,8 +1647,8 @@ ObpCreateHandle(IN OB_OPEN_REASON OpenReason, if (OpenReason == ObCreateHandle) { /* Check if we need to audit the privileges */ - if ((AuxData->PrivilegeSet) && - (AuxData->PrivilegeSet->PrivilegeCount)) + if ((AuxData->PrivilegesUsed) && + (AuxData->PrivilegesUsed->PrivilegeCount)) { /* Do the audit */ #if 0 @@ -1656,7 +1656,7 @@ ObpCreateHandle(IN OB_OPEN_REASON OpenReason, &AccessState-> SubjectSecurityContext, GrantedAccess, - AuxData->PrivilegeSet, + AuxData->PrivilegesUsed, TRUE, ExGetPreviousMode()); #endif diff --git a/ntoskrnl/se/access.c b/ntoskrnl/se/access.c index d9eacc550c7..d1ecd5bd571 100644 --- a/ntoskrnl/se/access.c +++ b/ntoskrnl/se/access.c @@ -88,7 +88,7 @@ SeCreateAccessStateEx( } /* Set the Auxiliary Data */ - AuxData->PrivilegeSet = (PPRIVILEGE_SET)((ULONG_PTR)AccessState + + AuxData->PrivilegesUsed = (PPRIVILEGE_SET)((ULONG_PTR)AccessState + FIELD_OFFSET(ACCESS_STATE, Privileges)); if (GenericMapping) AuxData->GenericMapping = *GenericMapping; @@ -158,7 +158,7 @@ SeDeleteAccessState( /* Deallocate Privileges */ if (AccessState->PrivilegesAllocated) - ExFreePoolWithTag(AuxData->PrivilegeSet, TAG_PRIVILEGE_SET); + ExFreePoolWithTag(AuxData->PrivilegesUsed, TAG_PRIVILEGE_SET); /* Deallocate Name and Type Name */ if (AccessState->ObjectName.Buffer) diff --git a/ntoskrnl/se/priv.c b/ntoskrnl/se/priv.c index a147d2bd2b7..c7f284d293d 100644 --- a/ntoskrnl/se/priv.c +++ b/ntoskrnl/se/priv.c @@ -601,9 +601,9 @@ SeAppendPrivileges( /* Calculate the size of the old privilege set */ OldPrivilegeSetSize = sizeof(PRIVILEGE_SET) + - (AuxData->PrivilegeSet->PrivilegeCount - 1) * sizeof(LUID_AND_ATTRIBUTES); + (AuxData->PrivilegesUsed->PrivilegeCount - 1) * sizeof(LUID_AND_ATTRIBUTES); - if (AuxData->PrivilegeSet->PrivilegeCount + + if (AuxData->PrivilegesUsed->PrivilegeCount + Privileges->PrivilegeCount > INITIAL_PRIVILEGE_COUNT) { /* Calculate the size of the new privilege set */ @@ -619,7 +619,7 @@ SeAppendPrivileges( /* Copy original privileges from the acess state */ RtlCopyMemory(PrivilegeSet, - AuxData->PrivilegeSet, + AuxData->PrivilegesUsed, OldPrivilegeSetSize); /* Append privileges from the privilege set*/ @@ -632,23 +632,23 @@ SeAppendPrivileges( /* Free the old privilege set if it was allocated */ if (AccessState->PrivilegesAllocated != FALSE) - ExFreePoolWithTag(AuxData->PrivilegeSet, TAG_PRIVILEGE_SET); + ExFreePoolWithTag(AuxData->PrivilegesUsed, TAG_PRIVILEGE_SET); /* Now we are using an allocated privilege set */ AccessState->PrivilegesAllocated = TRUE; /* Assign the new privileges to the access state */ - AuxData->PrivilegeSet = PrivilegeSet; + AuxData->PrivilegesUsed = PrivilegeSet; } else { /* Append privileges */ - RtlCopyMemory((PVOID)((ULONG_PTR)AuxData->PrivilegeSet + OldPrivilegeSetSize), + RtlCopyMemory((PVOID)((ULONG_PTR)AuxData->PrivilegesUsed + OldPrivilegeSetSize), (PVOID)((ULONG_PTR)Privileges + sizeof(PRIVILEGE_SET) - sizeof(LUID_AND_ATTRIBUTES)), Privileges->PrivilegeCount * sizeof(LUID_AND_ATTRIBUTES)); /* Adjust the number of privileges in the target privilege set */ - AuxData->PrivilegeSet->PrivilegeCount += Privileges->PrivilegeCount; + AuxData->PrivilegesUsed->PrivilegeCount += Privileges->PrivilegeCount; } return STATUS_SUCCESS; diff --git a/sdk/include/ndk/setypes.h b/sdk/include/ndk/setypes.h index 1f54e097318..d9fe4c4f9b6 100644 --- a/sdk/include/ndk/setypes.h +++ b/sdk/include/ndk/setypes.h @@ -255,9 +255,24 @@ typedef struct _TOKEN typedef struct _AUX_ACCESS_DATA { - PPRIVILEGE_SET PrivilegeSet; + PPRIVILEGE_SET PrivilegesUsed; GENERIC_MAPPING GenericMapping; - ULONG Reserved; + ACCESS_MASK AccessesToAudit; + ACCESS_MASK MaximumAuditMask; +#if (NTDDI_VERSION >= NTDDI_LONGHORN) + GUID TransactionId; +#endif +#if (NTDDI_VERSION >= NTDDI_WIN7) + PVOID NewSecurityDescriptor; + PVOID ExistingSecurityDescriptor; + PVOID ParentSecurityDescriptor; + VOID (NTAPI *DerefSecurityDescriptor)(PVOID, PVOID); + PVOID SDLock; + ACCESS_REASONS AccessReasons; +#endif +#if (NTDDI_VERSION >= NTDDI_WIN8) + BOOLEAN GenerateStagingEvents; +#endif } AUX_ACCESS_DATA, *PAUX_ACCESS_DATA; //

3 months, 2 weeks

[reactos] 03/06: [KMTESTS:SE] Don't modify internal data structure, this might cause buffer overrun.

by Thomas Faber

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=ff410211e960b466c9c5c… commit ff410211e960b466c9c5c887878eb70823ad3599 Author: Thomas Faber <thomas.faber(a)reactos.org> AuthorDate: Sat May 27 14:07:49 2023 -0400 Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org> CommitDate: Thu Sep 12 17:44:13 2024 +0300 [KMTESTS:SE] Don't modify internal data structure, this might cause buffer overrun. --- modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c b/modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c index ae4c50f5561..2ddf864d086 100644 --- a/modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c +++ b/modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c @@ -229,6 +229,7 @@ START_TEST(SeQueryInfoToken) NTSTATUS Status = STATUS_SUCCESS; PAUX_ACCESS_DATA AuxData = NULL; PPRIVILEGE_SET NewPrivilegeSet; + ULONG InitialPrivilegeCount; BOOLEAN Checker; PPRIVILEGE_SET Privileges = NULL; PSECURITY_SUBJECT_CONTEXT SubjectContext = NULL; @@ -278,7 +279,8 @@ START_TEST(SeQueryInfoToken) // Testing SeAppendPrivileges // //----------------------------------------------------------------// - AuxData->PrivilegeSet->PrivilegeCount = 1; + InitialPrivilegeCount = AuxData->PrivilegeSet->PrivilegeCount; + trace("Initial privilege count = %lu\n", InitialPrivilegeCount); // Testing SeAppendPrivileges. Must change PrivilegeCount to 2 (1 + 1) @@ -289,7 +291,7 @@ START_TEST(SeQueryInfoToken) Status = SeAppendPrivileges(AccessState, NewPrivilegeSet); ok(Status == STATUS_SUCCESS, "SeAppendPrivileges failed\n"); - ok((AuxData->PrivilegeSet->PrivilegeCount == 2),"PrivelegeCount must be 2, but it is %d\n", AuxData->PrivilegeSet->PrivilegeCount); + ok_eq_ulong(AuxData->PrivilegeSet->PrivilegeCount, InitialPrivilegeCount + 1); ExFreePoolWithTag(NewPrivilegeSet, 'QSmK'); //----------------------------------------------------------------// @@ -303,7 +305,7 @@ START_TEST(SeQueryInfoToken) Status = SeAppendPrivileges(AccessState, NewPrivilegeSet); ok(Status == STATUS_SUCCESS, "SeAppendPrivileges failed\n"); - ok((AuxData->PrivilegeSet->PrivilegeCount == 6),"PrivelegeCount must be 6, but it is %d\n", AuxData->PrivilegeSet->PrivilegeCount); + ok_eq_ulong(AuxData->PrivilegeSet->PrivilegeCount, InitialPrivilegeCount + 5); ExFreePoolWithTag(NewPrivilegeSet, 'QSmK'); //----------------------------------------------------------------// @@ -373,7 +375,7 @@ START_TEST(SeQueryInfoToken) NewPrivilegeSet->PrivilegeCount = 14; ok((SeAppendPrivileges(AccessState, NewPrivilegeSet)) == STATUS_SUCCESS, "SeAppendPrivileges failed\n"); - ok((AuxData->PrivilegeSet->PrivilegeCount == 20),"PrivelegeCount must be 20, but it is %d\n", AuxData->PrivilegeSet->PrivilegeCount); + ok_eq_ulong(AuxData->PrivilegeSet->PrivilegeCount, InitialPrivilegeCount + 19); ExFreePoolWithTag(NewPrivilegeSet, 'QSmK'); for (i = 0; i < AuxData->PrivilegeSet->PrivilegeCount; i++) {

3 months, 2 weeks

[reactos] 02/06: [KMTESTS:SE] Correctly allocate PrivilegeSet buffers.

by Thomas Faber

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=206df96bc474cb154934b… commit 206df96bc474cb154934b43453159079520e0512 Author: Thomas Faber <thomas.faber(a)reactos.org> AuthorDate: Sat May 27 14:01:44 2023 -0400 Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org> CommitDate: Thu Sep 12 17:44:13 2024 +0300 [KMTESTS:SE] Correctly allocate PrivilegeSet buffers. --- modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c b/modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c index f5f2b0213aa..ae4c50f5561 100644 --- a/modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c +++ b/modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c @@ -282,25 +282,29 @@ START_TEST(SeQueryInfoToken) // Testing SeAppendPrivileges. Must change PrivilegeCount to 2 (1 + 1) - NewPrivilegeSet = ExAllocatePool(PagedPool, sizeof(PRIVILEGE_SET)); + NewPrivilegeSet = ExAllocatePoolZero(PagedPool, + FIELD_OFFSET(PRIVILEGE_SET, Privilege[1]), + 'QSmK'); NewPrivilegeSet->PrivilegeCount = 1; Status = SeAppendPrivileges(AccessState, NewPrivilegeSet); ok(Status == STATUS_SUCCESS, "SeAppendPrivileges failed\n"); ok((AuxData->PrivilegeSet->PrivilegeCount == 2),"PrivelegeCount must be 2, but it is %d\n", AuxData->PrivilegeSet->PrivilegeCount); - ExFreePool(NewPrivilegeSet); + ExFreePoolWithTag(NewPrivilegeSet, 'QSmK'); //----------------------------------------------------------------// // Testing SeAppendPrivileges. Must change PrivilegeCount to 6 (2 + 4) - NewPrivilegeSet = ExAllocatePool(PagedPool, 4*sizeof(PRIVILEGE_SET)); + NewPrivilegeSet = ExAllocatePoolZero(PagedPool, + FIELD_OFFSET(PRIVILEGE_SET, Privilege[4]), + 'QSmK'); NewPrivilegeSet->PrivilegeCount = 4; Status = SeAppendPrivileges(AccessState, NewPrivilegeSet); ok(Status == STATUS_SUCCESS, "SeAppendPrivileges failed\n"); ok((AuxData->PrivilegeSet->PrivilegeCount == 6),"PrivelegeCount must be 6, but it is %d\n", AuxData->PrivilegeSet->PrivilegeCount); - ExFreePool(NewPrivilegeSet); + ExFreePoolWithTag(NewPrivilegeSet, 'QSmK'); //----------------------------------------------------------------// // Testing SePrivilegeCheck // @@ -363,12 +367,14 @@ START_TEST(SeQueryInfoToken) TPrivileges = (PTOKEN_PRIVILEGES)(Buffer); //trace("TPCount = %u\n\n", TPrivileges->PrivilegeCount); - NewPrivilegeSet = ExAllocatePool(PagedPool, 14*sizeof(PRIVILEGE_SET)); + NewPrivilegeSet = ExAllocatePoolZero(PagedPool, + FIELD_OFFSET(PRIVILEGE_SET, Privilege[14]), + 'QSmK'); NewPrivilegeSet->PrivilegeCount = 14; ok((SeAppendPrivileges(AccessState, NewPrivilegeSet)) == STATUS_SUCCESS, "SeAppendPrivileges failed\n"); ok((AuxData->PrivilegeSet->PrivilegeCount == 20),"PrivelegeCount must be 20, but it is %d\n", AuxData->PrivilegeSet->PrivilegeCount); - ExFreePool(NewPrivilegeSet); + ExFreePoolWithTag(NewPrivilegeSet, 'QSmK'); for (i = 0; i < AuxData->PrivilegeSet->PrivilegeCount; i++) { AuxData->PrivilegeSet->Privilege[i].Attributes = TPrivileges->Privileges[i].Attributes;

3 months, 2 weeks

[reactos] 01/06: [KMTESTS:SE] Avoid use of uninitialized pool and hardcoded offsets.

by Thomas Faber

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=64a6bd4c3e88780a6e408… commit 64a6bd4c3e88780a6e4082f004ad7d2c552bc49e Author: Thomas Faber <thomas.faber(a)reactos.org> AuthorDate: Sat May 27 11:19:17 2023 -0400 Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org> CommitDate: Thu Sep 12 17:44:13 2024 +0300 [KMTESTS:SE] Avoid use of uninitialized pool and hardcoded offsets. --- .../rostests/kmtests/ntos_se/SeQueryInfoToken.c | 33 ++++++++++++++-------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c b/modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c index a506ceb78ca..f5f2b0213aa 100644 --- a/modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c +++ b/modules/rostests/kmtests/ntos_se/SeQueryInfoToken.c @@ -13,6 +13,19 @@ #define NDEBUG #include <debug.h> +// Copied from PspProcessMapping -- although the values don't matter much for +// the most part. +static GENERIC_MAPPING ProcessGenericMapping = +{ + STANDARD_RIGHTS_READ | PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, + STANDARD_RIGHTS_WRITE | PROCESS_CREATE_PROCESS | PROCESS_CREATE_THREAD | + PROCESS_VM_OPERATION | PROCESS_VM_WRITE | PROCESS_DUP_HANDLE | + PROCESS_TERMINATE | PROCESS_SET_QUOTA | PROCESS_SET_INFORMATION | + PROCESS_SUSPEND_RESUME, + STANDARD_RIGHTS_EXECUTE | SYNCHRONIZE, + PROCESS_ALL_ACCESS +}; + //------------------------------------------------------------------------------// // Testing Functions // //------------------------------------------------------------------------------// @@ -222,8 +235,6 @@ START_TEST(SeQueryInfoToken) PACCESS_TOKEN Token = NULL; PTOKEN_PRIVILEGES TPrivileges; PVOID Buffer; - POBJECT_TYPE PsProcessType = NULL; - PGENERIC_MAPPING GenericMapping; ULONG i; SubjectContext = ExAllocatePool(PagedPool, sizeof(SECURITY_SUBJECT_CONTEXT)); @@ -240,14 +251,14 @@ START_TEST(SeQueryInfoToken) //----------------------------------------------------------------// AccessState = ExAllocatePool(PagedPool, sizeof(ACCESS_STATE)); - PsProcessType = ExAllocatePool(PagedPool, sizeof(OBJECT_TYPE)); - AuxData = ExAllocatePool(PagedPool, 0xC8); - GenericMapping = ExAllocatePool(PagedPool, sizeof(GENERIC_MAPPING)); + // AUX_ACCESS_DATA gets larger in newer Windows version. + // This is the largest known size, found in Windows 10/11. + AuxData = ExAllocatePoolZero(PagedPool, 0xE0, 'QSmK'); Status = SeCreateAccessState(AccessState, - (PVOID)AuxData, + AuxData, DesiredAccess, - GenericMapping + &ProcessGenericMapping ); ok((Status == STATUS_SUCCESS), "SeCreateAccessState failed with Status 0x%08X\n", Status); @@ -319,7 +330,7 @@ START_TEST(SeQueryInfoToken) AccessState->OriginalDesiredAccess, AccessState->PreviouslyGrantedAccess, &Privileges, - (PGENERIC_MAPPING)((PCHAR*)PsProcessType + 52), + &ProcessGenericMapping, KernelMode, &AccessMask, &Status @@ -379,7 +390,7 @@ START_TEST(SeQueryInfoToken) AccessState->OriginalDesiredAccess, AccessState->PreviouslyGrantedAccess, &Privileges, - (PGENERIC_MAPPING)((PCHAR*)PsProcessType + 52), + &ProcessGenericMapping, KernelMode, &AccessMask, &Status @@ -402,9 +413,7 @@ START_TEST(SeQueryInfoToken) SeDeleteAccessState(AccessState); - if (GenericMapping) ExFreePool(GenericMapping); - if (PsProcessType) ExFreePool(PsProcessType); if (SubjectContext) ExFreePool(SubjectContext); - if (AuxData) ExFreePool(AuxData); + if (AuxData) ExFreePoolWithTag(AuxData, 'QSmK'); if (AccessState) ExFreePool(AccessState); }

3 months, 2 weeks

[reactos] 02/02: [NTOS:KE/x64] Fix exception information on page faults

by Timo Kreuzer

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=2913ef5c9300fe1028de0… commit 2913ef5c9300fe1028de07f238dcb2ed39174edf Author: Timo Kreuzer <timo.kreuzer(a)reactos.org> AuthorDate: Sat Sep 7 23:33:48 2024 +0300 Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org> CommitDate: Thu Sep 12 17:07:59 2024 +0300 [NTOS:KE/x64] Fix exception information on page faults Pass a proper write/execute flag in the ExceptionInformation[0] field of the exception record instead of the raw fault code. This fixes comdlg:filedlg wine test, which writes to a write protected resource section, which needs to be handled by kernel32 UnhandledExceptionFilter, which relies on this parameter to be correct. --- ntoskrnl/ke/amd64/trap.S | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ntoskrnl/ke/amd64/trap.S b/ntoskrnl/ke/amd64/trap.S index 03e71bf9bc3..34b7c906104 100644 --- a/ntoskrnl/ke/amd64/trap.S +++ b/ntoskrnl/ke/amd64/trap.S @@ -461,8 +461,11 @@ IntsDisabled: PageFaultError: - /* Set parameter 1 to error code */ + /* Set parameter 1 to write/execute flag. + See https://learn.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-exceptio… */ mov r9d, [rbp + KTRAP_FRAME_ErrorCode] + shr r9d, 1 + and r9d, 9 /* Set parameter 2 to faulting address */ mov r10, cr2 // Param2 = faulting address

3 months, 2 weeks

[reactos] 01/02: [RTL/x64] Do not overwrite the original context during exception handling

by Timo Kreuzer

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=5eab2ddb2eb362eb2d5c3… commit 5eab2ddb2eb362eb2d5c327003d5193e071c573a Author: Timo Kreuzer <timo.kreuzer(a)reactos.org> AuthorDate: Sat Sep 7 22:10:28 2024 +0300 Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org> CommitDate: Thu Sep 12 17:07:59 2024 +0300 [RTL/x64] Do not overwrite the original context during exception handling This fixes ExceptionContinueExecution cases, where we want to continue execution on the original context (or as modified by the handler), not on some halfway unwinded one. --- sdk/lib/rtl/amd64/unwind.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/sdk/lib/rtl/amd64/unwind.c b/sdk/lib/rtl/amd64/unwind.c index 4a5903361b9..ee7313bc62f 100644 --- a/sdk/lib/rtl/amd64/unwind.c +++ b/sdk/lib/rtl/amd64/unwind.c @@ -679,7 +679,7 @@ RtlpUnwindInternal( ULONG64 ImageBase, EstablisherFrame; CONTEXT UnwindContext; - /* Get the current stack limits and registration frame */ + /* Get the current stack limits */ RtlpGetStackLimits(&StackLow, &StackHigh); /* If we have a target frame, then this is our high limit */ @@ -708,8 +708,11 @@ RtlpUnwindInternal( UnwindContext.Rip = *(DWORD64*)UnwindContext.Rsp; UnwindContext.Rsp += sizeof(DWORD64); - /* Copy the context back for the next iteration */ - *ContextRecord = UnwindContext; + if (HandlerType == UNW_FLAG_UHANDLER) + { + /* Copy the context back for the next iteration */ + *ContextRecord = UnwindContext; + } continue; } @@ -756,7 +759,7 @@ RtlpUnwindInternal( /* Log the exception if it's enabled */ RtlpCheckLogException(ExceptionRecord, - ContextRecord, + &UnwindContext, &DispatcherContext, sizeof(DispatcherContext)); @@ -844,8 +847,11 @@ RtlpUnwindInternal( break; } - /* We have successfully unwound a frame. Copy the unwind context back. */ - *ContextRecord = UnwindContext; + if (HandlerType == UNW_FLAG_UHANDLER) + { + /* We have successfully unwound a frame. Copy the unwind context back. */ + *ContextRecord = UnwindContext; + } } if (ExceptionRecord->ExceptionCode != STATUS_UNWIND_CONSOLIDATE)

3 months, 2 weeks

[reactos] 01/01: [NTGDI] Rewrite NtGdiPolyDraw

by Timo Kreuzer

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=402bc38ba7cabda46e816… commit 402bc38ba7cabda46e816b9cfa35773922b56d60 Author: Timo Kreuzer <timo.kreuzer(a)reactos.org> AuthorDate: Wed Sep 4 21:14:31 2024 +0300 Commit: Timo Kreuzer <timo.kreuzer(a)reactos.org> CommitDate: Thu Sep 12 15:52:30 2024 +0300 [NTGDI] Rewrite NtGdiPolyDraw It is now a wrapper around GdiPolyDraw, which implements the main body of the function, while the Nt function only probes and captures the user mode data into safe kernel mode buffers. Previously some people thought it was a great idea to just wrap the entire implementation in SEH, so when something throws an exception somewhere for whatever reason, we can just catch it here... and LEAK ALL RESOURCES IN THE PROCESS! TODO: Rewrite all of this. It's broken everywhere. --- win32ss/gdi/ntgdi/line.c | 97 ++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 82 insertions(+), 15 deletions(-) diff --git a/win32ss/gdi/ntgdi/line.c b/win32ss/gdi/ntgdi/line.c index 9e3ff20c824..9c4bee2bd13 100644 --- a/win32ss/gdi/ntgdi/line.c +++ b/win32ss/gdi/ntgdi/line.c @@ -11,6 +11,8 @@ #define NDEBUG #include <debug.h> +DBG_DEFAULT_CHANNEL(GdiLine); + // Some code from the WINE project source (www.winehq.com) VOID FASTCALL @@ -531,9 +533,9 @@ NtGdiLineTo(HDC hDC, } // FIXME: This function is completely broken +static BOOL -APIENTRY -NtGdiPolyDraw( +GdiPolyDraw( IN HDC hdc, IN LPPOINT lppt, IN LPBYTE lpbTypes, @@ -567,15 +569,11 @@ NtGdiPolyDraw( line_pts_old = NULL; bzr_pts = NULL; - _SEH2_TRY { - ProbeArrayForRead(lppt, sizeof(POINT), cCount, sizeof(LONG)); - ProbeArrayForRead(lpbTypes, sizeof(BYTE), cCount, sizeof(BYTE)); - if (PATH_IsPathOpen(dc->dclevel)) { result = PATH_PolyDraw(dc, (const POINT *)lppt, (const BYTE *)lpbTypes, cCount); - _SEH2_LEAVE; + goto Cleanup; } /* Check for valid point types */ @@ -595,7 +593,7 @@ NtGdiPolyDraw( break; } default: - _SEH2_LEAVE; + goto Cleanup; } } @@ -604,7 +602,7 @@ NtGdiPolyDraw( if (line_pts == NULL) { result = FALSE; - _SEH2_LEAVE; + goto Cleanup; } num_pts = 1; @@ -639,7 +637,7 @@ NtGdiPolyDraw( space = size * 2; line_pts_old = line_pts; line_pts = ExAllocatePoolWithTag(PagedPool, space * sizeof(POINT), TAG_SHAPE); - if (!line_pts) _SEH2_LEAVE; + if (!line_pts) goto Cleanup; RtlCopyMemory(line_pts, line_pts_old, space_old * sizeof(POINT)); ExFreePoolWithTag(line_pts_old, TAG_SHAPE); line_pts_old = NULL; @@ -659,11 +657,8 @@ NtGdiPolyDraw( IntGdiMoveToEx( dc, line_pts[num_pts - 1].x, line_pts[num_pts - 1].y, NULL ); result = TRUE; } - _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) - { - SetLastNtError(_SEH2_GetExceptionCode()); - } - _SEH2_END; + +Cleanup: if (line_pts != NULL) { @@ -685,6 +680,78 @@ NtGdiPolyDraw( return result; } +__kernel_entry +W32KAPI +BOOL +APIENTRY +NtGdiPolyDraw( + _In_ HDC hdc, + _In_reads_(cpt) LPPOINT ppt, + _In_reads_(cpt) LPBYTE pjAttr, + _In_ ULONG cpt) +{ + PBYTE pjBuffer; + PPOINT pptSafe; + PBYTE pjSafe; + SIZE_T cjSizePt; + BOOL bResult; + + if (cpt == 0) + { + ERR("cpt is 0\n"); + return FALSE; + } + + /* Validate that cpt isn't too large */ + if (cpt > ((MAXULONG - cpt) / sizeof(*ppt))) + { + ERR("cpt is too large\n", cpt); + return FALSE; + } + + /* Calculate size for a buffer */ + cjSizePt = cpt * sizeof(*ppt); + ASSERT(cjSizePt + cpt > cjSizePt); + + /* Allocate a buffer for all data */ + pjBuffer = ExAllocatePoolWithTag(PagedPool, cjSizePt + cpt, TAG_SHAPE); + if (pjBuffer == NULL) + { + ERR("Failed to allocate buffer\n"); + return FALSE; + } + + pptSafe = (PPOINT)pjBuffer; + pjSafe = pjBuffer + cjSizePt; + + _SEH2_TRY + { + ProbeArrayForRead(ppt, sizeof(*ppt), cpt, sizeof(ULONG)); + ProbeArrayForRead(pjAttr, sizeof(*pjAttr), cpt, sizeof(BYTE)); + + /* Copy the arrays */ + RtlCopyMemory(pptSafe, ppt, cjSizePt); + RtlCopyMemory(pjSafe, pjAttr, cpt); + } + _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) + { + SetLastNtError(_SEH2_GetExceptionCode()); + bResult = FALSE; + goto Cleanup; + } + _SEH2_END; + + /* Call the internal function */ + bResult = GdiPolyDraw(hdc, pptSafe, pjSafe, cpt); + +Cleanup: + + /* Free the buffer */ + ExFreePoolWithTag(pjBuffer, TAG_SHAPE); + + return bResult; +} + /* * @implemented */

3 months, 2 weeks

← Newer
1
...
5
6
7
8
9
10
11
12
Older →

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

Ros-diffs September 2024