Ros-diffs

ros-diffs@reactos.org

81390 discussions

[reactos] 01/01: [KS] Fix bug in KsStreamIo (#4663)
by Oleg Dubinskiy 06 Oct '23

06 Oct '23

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=d96b3cd45c888d7e37c92… commit d96b3cd45c888d7e37c9207dbb44321fc47bfec9 Author: Oleg Dubinskiy <oleg.dubinskij30(a)gmail.com> AuthorDate: Fri Oct 6 12:36:09 2023 +0200 Commit: GitHub <noreply(a)github.com> CommitDate: Fri Oct 6 12:36:09 2023 +0200 [KS] Fix bug in KsStreamIo (#4663) Properly set output buffer length in IO Stack Location of the current IRP, since it is passed to KsProbeStreamIrp when calling KsStreamIo, so it fails if the length isn't set properly. Don't set an input buffer length and the buffer itself, since it isn't passed anywhere, so setting it makes no sense. Moreover, MSDN says that for IOCTL_KS_READ/WRITE_STREAM, only output buffer (and its length) is needed to be set, but not an input one. So it indeed is more correct. It fixes buffer overflow in KsProbeStreamIrp when attempting to perform the streaming via KsStreamIo. I discovered this bug during my audio refactoring from PR #4660. --- drivers/ksfilter/ks/irp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/ksfilter/ks/irp.c b/drivers/ksfilter/ks/irp.c index ab5d1b21523..91302e67299 100644 --- a/drivers/ksfilter/ks/irp.c +++ b/drivers/ksfilter/ks/irp.c @@ -634,8 +634,7 @@ KsStreamIo( IoStack = IoGetNextIrpStackLocation(Irp); /* setup stack parameters */ IoStack->FileObject = FileObject; - IoStack->Parameters.DeviceIoControl.InputBufferLength = Length; - IoStack->Parameters.DeviceIoControl.Type3InputBuffer = StreamHeaders; + IoStack->Parameters.DeviceIoControl.OutputBufferLength = Length; IoStack->Parameters.DeviceIoControl.IoControlCode = (Flags == KSSTREAM_READ ? IOCTL_KS_READ_STREAM : IOCTL_KS_WRITE_STREAM); if (CompletionRoutine)

1 0

[reactos] 01/01: [APPHELP_APITEST] Fix build, addendum to commit a8755711b
by Hermès Bélusca-Maïto 05 Oct '23

05 Oct '23

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=2a5fd78f1bcd3706a7f17… commit 2a5fd78f1bcd3706a7f172668c35739aec3a28a2 Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Thu Oct 5 21:32:26 2023 +0200 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Thu Oct 5 21:32:38 2023 +0200 [APPHELP_APITEST] Fix build, addendum to commit a8755711b --- modules/rostests/apitests/apphelp/apphelp_apitest.h | 4 ++-- modules/rostests/apitests/apphelp/data.c | 2 +- modules/rostests/apitests/apphelp/register.cpp | 5 ++--- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/modules/rostests/apitests/apphelp/apphelp_apitest.h b/modules/rostests/apitests/apphelp/apphelp_apitest.h index 58cade256ab..5a9e9d470cc 100644 --- a/modules/rostests/apitests/apphelp/apphelp_apitest.h +++ b/modules/rostests/apitests/apphelp/apphelp_apitest.h @@ -21,9 +21,9 @@ void silence_debug_output(void); // Silence output if the environment var #define test_create_ne (winetest_set_location(__FILE__, __LINE__), 0) ? (void)0 : test_create_ne_imp /* register.cpp */ -BOOL IsUserAdmin(); +BOOL IsUserAdmin(VOID); -static DWORD g_WinVersion; +extern DWORD g_WinVersion; #define WINVER_ANY 0 #define WINVER_WINXP 0x0501 diff --git a/modules/rostests/apitests/apphelp/data.c b/modules/rostests/apitests/apphelp/data.c index dffad647572..dd8162626e2 100644 --- a/modules/rostests/apitests/apphelp/data.c +++ b/modules/rostests/apitests/apphelp/data.c @@ -792,7 +792,7 @@ void test_create_db_imp(const WCHAR* name, int win10) } } -static DWORD g_WinVersion; +DWORD g_WinVersion; DWORD get_host_winver(void) { if (!g_WinVersion) diff --git a/modules/rostests/apitests/apphelp/register.cpp b/modules/rostests/apitests/apphelp/register.cpp index 647ab6b4a9f..092d0221088 100644 --- a/modules/rostests/apitests/apphelp/register.cpp +++ b/modules/rostests/apitests/apphelp/register.cpp @@ -13,8 +13,6 @@ #include <strsafe.h> #include "wine/test.h" -#include "apphelp_apitest.h" - static const unsigned char rawDB[] = { /* Header: Major, Minor, 'sdbf' */ @@ -79,7 +77,8 @@ BOOL (WINAPI *pSdbRegisterDatabaseEx)(LPCWSTR pszDatabasePath, DWORD dwDatabaseT BOOL (WINAPI *pSdbUnregisterDatabase)(REFGUID pguidDB); -BOOL IsUserAdmin() +extern "C" +BOOL IsUserAdmin(VOID) { BOOL Result; SID_IDENTIFIER_AUTHORITY NtAuthority = { SECURITY_NT_AUTHORITY };

1 0

[reactos] 02/02: [APPHELP_APITEST] Deduplicate the IsUserAdmin() helper.
by Hermès Bélusca-Maïto 05 Oct '23

05 Oct '23

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=a8755711b986ed1e02cfa… commit a8755711b986ed1e02cfa2123f57d2e8b61a3d07 Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Thu Oct 5 15:40:21 2023 +0200 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Thu Oct 5 20:58:57 2023 +0200 [APPHELP_APITEST] Deduplicate the IsUserAdmin() helper. --- .../rostests/apitests/apphelp/apphelp_apitest.h | 13 ++---------- modules/rostests/apitests/apphelp/db.cpp | 23 ---------------------- modules/rostests/apitests/apphelp/register.cpp | 4 +++- 3 files changed, 5 insertions(+), 35 deletions(-) diff --git a/modules/rostests/apitests/apphelp/apphelp_apitest.h b/modules/rostests/apitests/apphelp/apphelp_apitest.h index 5623c21fd69..58cade256ab 100644 --- a/modules/rostests/apitests/apphelp/apphelp_apitest.h +++ b/modules/rostests/apitests/apphelp/apphelp_apitest.h @@ -5,7 +5,6 @@ extern "C" { #endif - /* data.c */ void test_create_db_imp(const WCHAR* name, int win10); DWORD test_get_db_size(); @@ -21,6 +20,8 @@ void silence_debug_output(void); // Silence output if the environment var #define test_create_file (winetest_set_location(__FILE__, __LINE__), 0) ? (void)0 : test_create_file_imp #define test_create_ne (winetest_set_location(__FILE__, __LINE__), 0) ? (void)0 : test_create_ne_imp +/* register.cpp */ +BOOL IsUserAdmin(); static DWORD g_WinVersion; @@ -42,7 +43,6 @@ typedef VOID* HSDB; typedef INT PATH_TYPE; - #define SDB_MAX_SDBS 16 #define SDB_MAX_EXES_VISTA 16 #define SDB_MAX_LAYERS 8 @@ -50,8 +50,6 @@ typedef INT PATH_TYPE; #define SDBQUERYRESULT_EXPECTED_SIZE_VISTA 456 - - typedef struct tagSDBQUERYRESULT_VISTA { TAGREF atrExes[SDB_MAX_EXES_VISTA]; @@ -67,7 +65,6 @@ typedef struct tagSDBQUERYRESULT_VISTA GUID rgGuidDB[SDB_MAX_SDBS]; } SDBQUERYRESULT_VISTA, *PSDBQUERYRESULT_VISTA; - #define SDBQUERYRESULT_EXPECTED_SIZE_2k3 344 #define SDB_MAX_EXES_2k3 4 @@ -86,16 +83,10 @@ typedef struct tagSDBQUERYRESULT_2k3 GUID rgGuidDB[SDB_MAX_SDBS]; } SDBQUERYRESULT_2k3, *PSDBQUERYRESULT_2k3; - - - - C_ASSERT(sizeof(SDBQUERYRESULT_VISTA) == SDBQUERYRESULT_EXPECTED_SIZE_VISTA); C_ASSERT(sizeof(SDBQUERYRESULT_2k3) == SDBQUERYRESULT_EXPECTED_SIZE_2k3); - - #ifdef __cplusplus } // extern "C" #endif diff --git a/modules/rostests/apitests/apphelp/db.cpp b/modules/rostests/apitests/apphelp/db.cpp index 9cebc82506d..7dc5427846b 100644 --- a/modules/rostests/apitests/apphelp/db.cpp +++ b/modules/rostests/apitests/apphelp/db.cpp @@ -1189,27 +1189,6 @@ static void test_is_testdb(PDB pdb) } } -static BOOL IsUserAdmin() -{ - BOOL Result; - SID_IDENTIFIER_AUTHORITY NtAuthority = { SECURITY_NT_AUTHORITY }; - PSID AdministratorsGroup; - - Result = AllocateAndInitializeSid(&NtAuthority, 2, - SECURITY_BUILTIN_DOMAIN_RID, - DOMAIN_ALIAS_RID_ADMINS, - 0, 0, 0, 0, 0, 0, - &AdministratorsGroup); - if (Result) - { - if (!CheckTokenMembership( NULL, AdministratorsGroup, &Result)) - Result = FALSE; - FreeSid(AdministratorsGroup); - } - - return Result; -} - template<typename SDBQUERYRESULT_T> static void check_adwExeFlags(DWORD adwExeFlags_0, SDBQUERYRESULT_T& query, const char* file, int line, size_t cur) @@ -1599,8 +1578,6 @@ static void test_MatchApplicationsEx(void) } - - static void test_TagRef(void) { WCHAR tmpdir[MAX_PATH], dbpath[MAX_PATH]; diff --git a/modules/rostests/apitests/apphelp/register.cpp b/modules/rostests/apitests/apphelp/register.cpp index 146dc8cf373..647ab6b4a9f 100644 --- a/modules/rostests/apitests/apphelp/register.cpp +++ b/modules/rostests/apitests/apphelp/register.cpp @@ -13,6 +13,8 @@ #include <strsafe.h> #include "wine/test.h" +#include "apphelp_apitest.h" + static const unsigned char rawDB[] = { /* Header: Major, Minor, 'sdbf' */ @@ -90,7 +92,7 @@ BOOL IsUserAdmin() &AdministratorsGroup); if (Result) { - if (!CheckTokenMembership( NULL, AdministratorsGroup, &Result)) + if (!CheckTokenMembership(NULL, AdministratorsGroup, &Result)) Result = FALSE; FreeSid(AdministratorsGroup); }

1 0

[reactos] 01/02: [DESK] Remove deprecated code.
by Hermès Bélusca-Maïto 05 Oct '23

05 Oct '23

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=f1cff6ef272de5795ae5f… commit f1cff6ef272de5795ae5ffdb04ce781e69f0707b Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> AuthorDate: Thu Oct 5 12:34:33 2023 +0200 Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org> CommitDate: Thu Oct 5 20:58:28 2023 +0200 [DESK] Remove deprecated code. This mostly reverts commit 221f21c62 (r29328), that was a hack for building with MSVC. We can nowadays correctly link with the shell32 SHCreatePropSheetExtArrayEx() export. --- dll/cpl/desk/advmon.c | 27 ++------------------------- 1 file changed, 2 insertions(+), 25 deletions(-) diff --git a/dll/cpl/desk/advmon.c b/dll/cpl/desk/advmon.c index e933072316e..c0512028199 100644 --- a/dll/cpl/desk/advmon.c +++ b/dll/cpl/desk/advmon.c @@ -74,9 +74,6 @@ BuildAdvPropTitle(IDataObject *pdo, LPTSTR lpBuffer, DWORD dwBufferLen) LocalFree((HLOCAL)lpDisplayName); } - -typedef HPSXA (WINAPI * CPSEAE)(HKEY,LPCWSTR,UINT,IDataObject*); - BOOL DisplayAdvancedSettings(HWND hWndParent, PDISPLAY_DEVICE_ENTRY DisplayDevice) { @@ -86,10 +83,6 @@ DisplayAdvancedSettings(HWND hWndParent, PDISPLAY_DEVICE_ENTRY DisplayDevice) HPSXA hpsxaDev, hpsxaDisp; BOOL Ret; IDataObject *pdo; -#ifdef _MSC_VER - HMODULE hShell32 = NULL; - CPSEAE msvc_SHCreatePropSheetExtArrayEx; -#endif /* FIXME: Build the "%s and %s" caption string for the monitor and adapter name */ szCaption[0] = _T('\0'); @@ -109,25 +102,13 @@ DisplayAdvancedSettings(HWND hWndParent, PDISPLAY_DEVICE_ENTRY DisplayDevice) if (pdo != NULL) BuildAdvPropTitle(pdo, szCaption, sizeof(szCaption) / sizeof(szCaption[0])); -#ifdef _MSC_VER - hShell32 = LoadLibrary(_T("shell32.dll")); - if (hShell32 == NULL) - return FALSE; - msvc_SHCreatePropSheetExtArrayEx = (CPSEAE)GetProcAddress(hShell32, (LPCSTR)194); - hpsxaDev = msvc_SHCreatePropSheetExtArrayEx(HKEY_LOCAL_MACHINE, REGSTR_PATH_CONTROLSFOLDER TEXT("\\Device"), MAX_ADVANCED_PAGES - psh.nPages, pdo); -#else hpsxaDev = SHCreatePropSheetExtArrayEx(HKEY_LOCAL_MACHINE, REGSTR_PATH_CONTROLSFOLDER TEXT("\\Device"), MAX_ADVANCED_PAGES - psh.nPages, pdo); -#endif if (hpsxaDev != NULL) - SHAddFromPropSheetExtArray(hpsxaDev, PropSheetAddPage, (LPARAM)&psh); + SHAddFromPropSheetExtArray(hpsxaDev, PropSheetAddPage, (LPARAM)&psh); -#ifdef _MSC_VER - hpsxaDisp = msvc_SHCreatePropSheetExtArrayEx(HKEY_LOCAL_MACHINE, REGSTR_PATH_CONTROLSFOLDER TEXT("\\Display"), MAX_ADVANCED_PAGES - psh.nPages, pdo); -#else hpsxaDisp = SHCreatePropSheetExtArrayEx(HKEY_LOCAL_MACHINE, REGSTR_PATH_CONTROLSFOLDER TEXT("\\Display"), MAX_ADVANCED_PAGES - psh.nPages, pdo); -#endif if (hpsxaDisp != NULL) - SHAddFromPropSheetExtArray(hpsxaDisp, PropSheetAddPage, (LPARAM)&psh); + SHAddFromPropSheetExtArray(hpsxaDisp, PropSheetAddPage, (LPARAM)&psh); Ret = (LONG)(PropertySheet(&psh) != -1); @@ -139,9 +120,5 @@ DisplayAdvancedSettings(HWND hWndParent, PDISPLAY_DEVICE_ENTRY DisplayDevice) IDataObject_Release(pdo); -#ifdef _MSC_VER - FreeLibrary(hShell32); -#endif - return Ret; }

1 0

[reactos] 01/01: [MSPAINT] Update scroll info on loading file (#5761)
by Katayama Hirofumi MZ 05 Oct '23

05 Oct '23

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=1a90009bb9651d3c52777… commit 1a90009bb9651d3c52777a8d356bb4392937be81 Author: Katayama Hirofumi MZ <katayama.hirofumi.mz(a)gmail.com> AuthorDate: Thu Oct 5 20:01:41 2023 +0900 Commit: GitHub <noreply(a)github.com> CommitDate: Thu Oct 5 20:01:41 2023 +0900 [MSPAINT] Update scroll info on loading file (#5761) The scroll info was not updated when the file is loading. - Rename CCanvasWindow::Update as CCanvasWindow::updateScrollInfo. - Update scroll info on ImageModel::NotifyImageChanged. - Improve ImageModel::ClearHistory. CORE-19094 --- base/applications/mspaint/canvas.cpp | 8 ++++---- base/applications/mspaint/canvas.h | 2 +- base/applications/mspaint/dib.cpp | 6 ++++++ base/applications/mspaint/history.cpp | 17 +++++++++++++++-- 4 files changed, 26 insertions(+), 7 deletions(-) diff --git a/base/applications/mspaint/canvas.cpp b/base/applications/mspaint/canvas.cpp index d62779f39a6..a4eeead446b 100644 --- a/base/applications/mspaint/canvas.cpp +++ b/base/applications/mspaint/canvas.cpp @@ -180,7 +180,7 @@ VOID CCanvasWindow::DoDraw(HDC hDC, RECT& rcClient, RECT& rcPaint) ::DeleteDC(hdcMem0); } -VOID CCanvasWindow::Update(HWND hwndFrom) +VOID CCanvasWindow::updateScrollInfo() { CRect rcClient; GetClientRect(&rcClient); @@ -214,7 +214,7 @@ VOID CCanvasWindow::Update(HWND hwndFrom) LRESULT CCanvasWindow::OnSize(UINT nMsg, WPARAM wParam, LPARAM lParam, BOOL& bHandled) { if (m_hWnd) - Update(m_hWnd); + updateScrollInfo(); return 0; } @@ -245,7 +245,7 @@ VOID CCanvasWindow::OnHVScroll(WPARAM wParam, INT fnBar) break; } SetScrollInfo(fnBar, &si); - Update(m_hWnd); + updateScrollInfo(); Invalidate(FALSE); // FIXME: Flicker } @@ -627,7 +627,7 @@ LRESULT CCanvasWindow::OnLRButtonUp(BOOL bLeftButton, UINT nMsg, WPARAM wParam, m_hitCanvasSizeBox = HIT_NONE; toolsModel.resetTool(); // resets the point-buffer of the polygon and bezier functions - Update(NULL); + updateScrollInfo(); Invalidate(TRUE); return 0; } diff --git a/base/applications/mspaint/canvas.h b/base/applications/mspaint/canvas.h index db0e549837b..dc26707be24 100644 --- a/base/applications/mspaint/canvas.h +++ b/base/applications/mspaint/canvas.h @@ -42,7 +42,7 @@ public: VOID cancelDrawing(); VOID finishDrawing(); - VOID Update(HWND hwndFrom); + VOID updateScrollInfo(); VOID ImageToCanvas(POINT& pt); VOID ImageToCanvas(RECT& rc); diff --git a/base/applications/mspaint/dib.cpp b/base/applications/mspaint/dib.cpp index cd069c9ef6f..9e7080e3da4 100644 --- a/base/applications/mspaint/dib.cpp +++ b/base/applications/mspaint/dib.cpp @@ -274,8 +274,14 @@ HBITMAP DoLoadImageFile(HWND hwnd, LPCWSTR name, BOOL fIsMainFile) CImageDx img; float xDpi = 0, yDpi = 0; HRESULT hr = img.LoadDx(name, &xDpi, &yDpi); + if (FAILED(hr) && fIsMainFile) + { + imageModel.ClearHistory(); + hr = img.LoadDx(name, &xDpi, &yDpi); + } if (FAILED(hr)) { + ATLTRACE("hr: 0x%08lX\n", hr); ShowError(IDS_LOADERRORTEXT, name); return NULL; } diff --git a/base/applications/mspaint/history.cpp b/base/applications/mspaint/history.cpp index 9e924b9e3db..05990df42b8 100644 --- a/base/applications/mspaint/history.cpp +++ b/base/applications/mspaint/history.cpp @@ -15,9 +15,13 @@ ImageModel imageModel; void ImageModel::NotifyImageChanged() { if (canvasWindow.IsWindow()) - canvasWindow.Invalidate(FALSE); + { + canvasWindow.updateScrollInfo(); + canvasWindow.Invalidate(); + } + if (miniature.IsWindow()) - miniature.Invalidate(FALSE); + miniature.Invalidate(); } ImageModel::ImageModel() @@ -99,6 +103,15 @@ void ImageModel::ResetToPrevious() void ImageModel::ClearHistory() { + for (int i = 0; i < HISTORYSIZE; ++i) + { + if (m_hBms[i] && i != m_currInd) + { + ::DeleteObject(m_hBms[i]); + m_hBms[i] = NULL; + } + } + m_undoSteps = 0; m_redoSteps = 0; }

1 0

[reactos] 01/01: [MSPAINT] Fix assertion failure on text tool (#5762)
by Katayama Hirofumi MZ 05 Oct '23

05 Oct '23

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=3579ea78889b664b3b977… commit 3579ea78889b664b3b977f5d0a725cabf7f92109 Author: Katayama Hirofumi MZ <katayama.hirofumi.mz(a)gmail.com> AuthorDate: Thu Oct 5 19:51:36 2023 +0900 Commit: GitHub <noreply(a)github.com> CommitDate: Thu Oct 5 19:51:36 2023 +0900 [MSPAINT] Fix assertion failure on text tool (#5762) Kill the assertion failure on choosing text tool. Check whether textEditWindow is not null by using IsWindowVisible. CORE-19094 --- base/applications/mspaint/mouse.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/base/applications/mspaint/mouse.cpp b/base/applications/mspaint/mouse.cpp index 793efedf3ec..8412cc027c0 100644 --- a/base/applications/mspaint/mouse.cpp +++ b/base/applications/mspaint/mouse.cpp @@ -762,7 +762,8 @@ struct TextTool : ToolBase void OnFinishDraw() override { - if (textEditWindow.GetWindowTextLength() > 0) + if (::IsWindowVisible(textEditWindow) && + textEditWindow.GetWindowTextLength() > 0) { imageModel.PushImageForUndo(); draw(m_hdc);

1 0

[reactos] 05/05: [NTOS:SE] Mute the access denied DPRINTs
by George Bișoc 04 Oct '23

04 Oct '23

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=d72025649b769061b5f9f… commit d72025649b769061b5f9f38aa57742ef17ae7c9e Author: George Bișoc <george.bisoc(a)reactos.org> AuthorDate: Sun Oct 1 15:02:48 2023 +0200 Commit: George Bișoc <george.bisoc(a)reactos.org> CommitDate: Wed Oct 4 18:04:30 2023 +0200 [NTOS:SE] Mute the access denied DPRINTs They can be spammy. Also clarify these debug prints, because some people think that "failed to grant access rights" means there's something wrong in the core access check functions. --- ntoskrnl/se/accesschk.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/ntoskrnl/se/accesschk.c b/ntoskrnl/se/accesschk.c index 858bfeef994..ad931da38fb 100644 --- a/ntoskrnl/se/accesschk.c +++ b/ntoskrnl/se/accesschk.c @@ -1174,7 +1174,7 @@ SepAccessCheckWorker( RemainingAccess &= ~(MAXIMUM_ALLOWED | AccessCheckRights.GrantedAccessRights); if (RemainingAccess != 0) { - DPRINT1("Failed to grant access rights. RemainingAccess = 0x%08lx DesiredAccess = 0x%08lx\n", RemainingAccess, DesiredAccess); + DPRINT("Failed to grant access rights, access denied. RemainingAccess = 0x%08lx DesiredAccess = 0x%08lx\n", RemainingAccess, DesiredAccess); PreviouslyGrantedAccess = 0; Status = STATUS_ACCESS_DENIED; goto ReturnCommonStatus; @@ -1188,7 +1188,7 @@ SepAccessCheckWorker( } else { - DPRINT1("Failed to grant access rights. PreviouslyGrantedAccess == 0 DesiredAccess = %08lx\n", DesiredAccess); + DPRINT("Failed to grant access rights, access denied. PreviouslyGrantedAccess == 0 DesiredAccess = %08lx\n", DesiredAccess); Status = STATUS_ACCESS_DENIED; } @@ -1217,7 +1217,7 @@ SepAccessCheckWorker( RemainingAccess &= ~(MAXIMUM_ALLOWED | GrantedRights); if (RemainingAccess != 0) { - DPRINT1("Failed to grant access rights to the whole object hierarchy list. RemainingAccess = 0x%08lx DesiredAccess = 0x%08lx\n", + DPRINT("Failed to grant access rights to the whole object hierarchy list, access denied. RemainingAccess = 0x%08lx DesiredAccess = 0x%08lx\n", RemainingAccess, DesiredAccess); PreviouslyGrantedAccess = 0; Status = STATUS_ACCESS_DENIED; @@ -1232,7 +1232,7 @@ SepAccessCheckWorker( } else { - DPRINT1("Failed to grant access rights to the whole object hierarchy list. PreviouslyGrantedAccess == 0 DesiredAccess = %08lx\n", + DPRINT("Failed to grant access rights to the whole object hierarchy list, access denied. PreviouslyGrantedAccess == 0 DesiredAccess = %08lx\n", DesiredAccess); Status = STATUS_ACCESS_DENIED; } @@ -1262,7 +1262,7 @@ SepAccessCheckWorker( RemainingAccess = (~GrantedRights & WantedRights); if (RemainingAccess != 0) { - DPRINT1("Failed to grant access rights at specific object at index %lu. RemainingAccess = 0x%08lx DesiredAccess = 0x%08lx\n", + DPRINT("Failed to grant access rights at specific object at index %lu, access denied. RemainingAccess = 0x%08lx DesiredAccess = 0x%08lx\n", ObjectTypeIndex, RemainingAccess, DesiredAccess); AccessStatusList[ObjectTypeIndex] = STATUS_ACCESS_DENIED; } @@ -1274,7 +1274,7 @@ SepAccessCheckWorker( else { /* No access is given */ - DPRINT1("Failed to grant access rights at specific object at index %lu. No access is given\n", ObjectTypeIndex); + DPRINT("Failed to grant access rights at specific object at index %lu. No access is given\n", ObjectTypeIndex); AccessStatusList[ObjectTypeIndex] = STATUS_ACCESS_DENIED; } @@ -1324,7 +1324,7 @@ SepAccessCheckWorker( /* Fail if some rights have not been granted */ if (AccessCheckRights.RemainingAccessRights != 0) { - DPRINT1("Failed to grant access rights. RemainingAccess = 0x%08lx DesiredAccess = 0x%08lx\n", AccessCheckRights.RemainingAccessRights, DesiredAccess); + DPRINT("Failed to grant access rights, access denied. RemainingAccess = 0x%08lx DesiredAccess = 0x%08lx\n", AccessCheckRights.RemainingAccessRights, DesiredAccess); PreviouslyGrantedAccess = 0; Status = STATUS_ACCESS_DENIED; goto ReturnCommonStatus; @@ -1349,7 +1349,7 @@ SepAccessCheckWorker( if (!AccessIsGranted) { - DPRINT1("Failed to grant access rights to the whole object hierarchy list. DesiredAccess = 0x%08lx\n", DesiredAccess); + DPRINT("Failed to grant access rights to the whole object hierarchy list, access denied. DesiredAccess = 0x%08lx\n", DesiredAccess); PreviouslyGrantedAccess = 0; Status = STATUS_ACCESS_DENIED; goto ReturnCommonStatus; @@ -1381,7 +1381,7 @@ SepAccessCheckWorker( /* Fail if some rights have not been granted */ if (AccessCheckRights.RemainingAccessRights != 0) { - DPRINT1("Failed to grant access rights. RemainingAccess = 0x%08lx DesiredAccess = 0x%08lx\n", AccessCheckRights.RemainingAccessRights, DesiredAccess); + DPRINT("Failed to grant access rights, access denied. RemainingAccess = 0x%08lx DesiredAccess = 0x%08lx\n", AccessCheckRights.RemainingAccessRights, DesiredAccess); PreviouslyGrantedAccess = 0; Status = STATUS_ACCESS_DENIED; goto ReturnCommonStatus; @@ -1410,7 +1410,7 @@ SepAccessCheckWorker( if (!AccessIsGranted) { - DPRINT1("Failed to grant access rights to the whole object hierarchy list. DesiredAccess = 0x%08lx\n", DesiredAccess); + DPRINT("Failed to grant access rights to the whole object hierarchy list, access denied. DesiredAccess = 0x%08lx\n", DesiredAccess); PreviouslyGrantedAccess = 0; Status = STATUS_ACCESS_DENIED; goto ReturnCommonStatus; @@ -1424,7 +1424,7 @@ SepAccessCheckWorker( /* Fail if no rights have been granted */ if (PreviouslyGrantedAccess == 0) { - DPRINT1("Failed to grant access rights. PreviouslyGrantedAccess == 0 DesiredAccess = %08lx\n", DesiredAccess); + DPRINT("Failed to grant access rights, access denied. PreviouslyGrantedAccess == 0 DesiredAccess = %08lx\n", DesiredAccess); Status = STATUS_ACCESS_DENIED; goto ReturnCommonStatus; }

1 0

[reactos] 04/05: [NTOS:SE] HACK: Temporarily add the Local group SID to the system token
by George Bișoc 04 Oct '23

04 Oct '23

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=09bfd96f3bdb8a190ddf5… commit 09bfd96f3bdb8a190ddf51100204be19cbbc0b8c Author: George Bișoc <george.bisoc(a)reactos.org> AuthorDate: Sun Oct 1 14:01:22 2023 +0200 Commit: George Bișoc <george.bisoc(a)reactos.org> CommitDate: Wed Oct 4 18:04:30 2023 +0200 [NTOS:SE] HACK: Temporarily add the Local group SID to the system token Temporarily add the local group to the system token so that Virtualbox GA services can properly set up network drives for shared folders. What happens is that a security descriptor has a DACL with only one ACE that grants access to Local SID (presumably coming from Vbox?) but the client token is that of the service which is a SYSTEM token. Perhaps we are not impersonating the right user or whatever else. This is only a temporary placebo, until a proper solution is found. CORE-18250 --- ntoskrnl/se/token.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ntoskrnl/se/token.c b/ntoskrnl/se/token.c index dae5a411dff..83335415669 100644 --- a/ntoskrnl/se/token.c +++ b/ntoskrnl/se/token.c @@ -1784,12 +1784,14 @@ SepCreateSystemProcessToken(VOID) { {SeAliasAdminsSid, OwnerAttributes}, {SeWorldSid, GroupAttributes}, - {SeAuthenticatedUsersSid, GroupAttributes} + {SeAuthenticatedUsersSid, GroupAttributes}, + {SeLocalSid, SE_GROUP_ENABLED} // HACK: Temporarily add the local group. See CORE-18250. }; GroupsLength = sizeof(SID_AND_ATTRIBUTES) + SeLengthSid(Groups[0].Sid) + SeLengthSid(Groups[1].Sid) + - SeLengthSid(Groups[2].Sid); + SeLengthSid(Groups[2].Sid) + + SeLengthSid(Groups[3].Sid); // HACK ASSERT(GroupsLength <= (sizeof(Groups) * sizeof(ULONG))); /* Setup the privileges */

1 0

[reactos] 03/05: [NTDLL_APITEST] Write NtAccessCheck testcase that tests for empty generic mapping scenario
by George Bișoc 04 Oct '23

04 Oct '23

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=fefb1b77458f05ecdc4e0… commit fefb1b77458f05ecdc4e0956c1b69c109f51142e Author: George Bișoc <george.bisoc(a)reactos.org> AuthorDate: Sat Sep 30 21:05:18 2023 +0200 Commit: George Bișoc <george.bisoc(a)reactos.org> CommitDate: Wed Oct 4 18:04:29 2023 +0200 [NTDLL_APITEST] Write NtAccessCheck testcase that tests for empty generic mapping scenario See b79aadd50bb7d5d6f08532284941c0334514c477 for further information. --- modules/rostests/apitests/ntdll/CMakeLists.txt | 1 + modules/rostests/apitests/ntdll/NtAccessCheck.c | 188 ++++++++++++++++++++++++ modules/rostests/apitests/ntdll/testlist.c | 2 + 3 files changed, 191 insertions(+) diff --git a/modules/rostests/apitests/ntdll/CMakeLists.txt b/modules/rostests/apitests/ntdll/CMakeLists.txt index 85515a2efaf..b6ce3a1cf4d 100644 --- a/modules/rostests/apitests/ntdll/CMakeLists.txt +++ b/modules/rostests/apitests/ntdll/CMakeLists.txt @@ -10,6 +10,7 @@ list(APPEND SOURCE load_notifications.c locale.c NtAcceptConnectPort.c + NtAccessCheck.c NtAccessCheckByType.c NtAccessCheckByTypeResultList.c NtAdjustGroupsToken.c diff --git a/modules/rostests/apitests/ntdll/NtAccessCheck.c b/modules/rostests/apitests/ntdll/NtAccessCheck.c new file mode 100644 index 00000000000..c987a445b13 --- /dev/null +++ b/modules/rostests/apitests/ntdll/NtAccessCheck.c @@ -0,0 +1,188 @@ +/* + * PROJECT: ReactOS API tests + * LICENSE: GPL-2.0-or-later (https://spdx.org/licenses/GPL-2.0-or-later) + * PURPOSE: Tests for the NtAccessCheck API + * COPYRIGHT: Copyright 2023 George Bișoc <george.bisoc(a)reactos.org> + */ + +#include "precomp.h" + +static +HANDLE +GetToken(VOID) +{ + NTSTATUS Status; + HANDLE Token; + HANDLE DuplicatedToken; + OBJECT_ATTRIBUTES ObjectAttributes; + SECURITY_QUALITY_OF_SERVICE Sqos; + + Status = NtOpenProcessToken(NtCurrentProcess(), + TOKEN_QUERY | TOKEN_DUPLICATE, + &Token); + if (!NT_SUCCESS(Status)) + { + trace("Failed to get current process token (Status 0x%08lx)\n", Status); + return NULL; + } + + Sqos.Length = sizeof(SECURITY_QUALITY_OF_SERVICE); + Sqos.ImpersonationLevel = SecurityImpersonation; + Sqos.ContextTrackingMode = 0; + Sqos.EffectiveOnly = FALSE; + + InitializeObjectAttributes(&ObjectAttributes, + NULL, + 0, + NULL, + NULL); + ObjectAttributes.SecurityQualityOfService = &Sqos; + + Status = NtDuplicateToken(Token, + TOKEN_QUERY | TOKEN_DUPLICATE, + &ObjectAttributes, + FALSE, + TokenImpersonation, + &DuplicatedToken); + if (!NT_SUCCESS(Status)) + { + trace("Failed to duplicate token (Status 0x%08lx)\n", Status); + NtClose(Token); + return NULL; + } + + return DuplicatedToken; +} + +static +VOID +AccessCheckEmptyMappingTest(VOID) +{ + NTSTATUS Status; + NTSTATUS AccessStatus; + ACCESS_MASK GrantedAccess; + PPRIVILEGE_SET PrivilegeSet = NULL; + ULONG PrivilegeSetLength; + HANDLE Token = NULL; + PACL Dacl = NULL; + ULONG DaclSize; + SECURITY_DESCRIPTOR Sd; + PSID WorldSid = NULL; + static SID_IDENTIFIER_AUTHORITY WorldAuthority = {SECURITY_WORLD_SID_AUTHORITY}; + static GENERIC_MAPPING EmptyMapping = {0, 0, 0, 0}; + + /* Allocate all the stuff we need */ + PrivilegeSetLength = FIELD_OFFSET(PRIVILEGE_SET, Privilege[16]); + PrivilegeSet = RtlAllocateHeap(RtlGetProcessHeap(), 0, PrivilegeSetLength); + if (PrivilegeSet == NULL) + { + skip("Failed to allocate PrivilegeSet, skipping tests\n"); + return; + } + + Status = RtlAllocateAndInitializeSid(&WorldAuthority, + 1, + SECURITY_WORLD_RID, + 0, + 0, + 0, + 0, + 0, + 0, + 0, + &WorldSid); + if (!NT_SUCCESS(Status)) + { + skip("Failed to create World SID, skipping tests\n"); + goto Quit; + } + + Token = GetToken(); + if (Token == NULL) + { + skip("Failed to get token, skipping tests\n"); + goto Quit; + } + + Status = RtlCreateSecurityDescriptor(&Sd, SECURITY_DESCRIPTOR_REVISION); + if (!NT_SUCCESS(Status)) + { + skip("Failed to create a security descriptor, skipping tests\n"); + goto Quit; + } + + DaclSize = sizeof(ACL) + + sizeof(ACCESS_ALLOWED_OBJECT_ACE) + RtlLengthSid(WorldSid); + Dacl = RtlAllocateHeap(RtlGetProcessHeap(), + HEAP_ZERO_MEMORY, + DaclSize); + if (Dacl == NULL) + { + skip("Failed to allocate memory for DACL, skipping tests\n"); + goto Quit; + } + + /* Setup a ACL and give full access to everyone */ + Status = RtlCreateAcl(Dacl, + DaclSize, + ACL_REVISION); + if (!NT_SUCCESS(Status)) + { + skip("Failed to create DACL, skipping tests\n"); + goto Quit; + } + + Status = RtlAddAccessAllowedAce(Dacl, + ACL_REVISION, + GENERIC_ALL, + WorldSid); + if (!NT_SUCCESS(Status)) + { + skip("Failed to add allowed ACE for World SID, skipping tests\n"); + goto Quit; + } + + /* Setup the descriptor */ + RtlSetGroupSecurityDescriptor(&Sd, WorldSid, FALSE); + RtlSetOwnerSecurityDescriptor(&Sd, WorldSid, FALSE); + RtlSetDaclSecurityDescriptor(&Sd, TRUE, Dacl, FALSE); + + /* Do an access check with empty mapping */ + Status = NtAccessCheck(&Sd, + Token, + MAXIMUM_ALLOWED, + &EmptyMapping, + PrivilegeSet, + &PrivilegeSetLength, + &GrantedAccess, + &AccessStatus); + ok_hex(Status, STATUS_SUCCESS); + ok(AccessStatus == STATUS_SUCCESS, "Expected a success status but got 0x%08lx\n", AccessStatus); + trace("GrantedAccess == 0x%08lx\n", GrantedAccess); + +Quit: + if (Dacl) + { + RtlFreeHeap(RtlGetProcessHeap(), 0, Dacl); + } + + if (Token) + { + NtClose(Token); + } + + if (WorldSid) + { + RtlFreeSid(WorldSid); + } + + if (PrivilegeSet) + { + RtlFreeHeap(RtlGetProcessHeap(), 0, PrivilegeSet); + } +} + +START_TEST(NtAccessCheck) +{ + AccessCheckEmptyMappingTest(); +} diff --git a/modules/rostests/apitests/ntdll/testlist.c b/modules/rostests/apitests/ntdll/testlist.c index 2be58938238..438cc56a172 100644 --- a/modules/rostests/apitests/ntdll/testlist.c +++ b/modules/rostests/apitests/ntdll/testlist.c @@ -6,6 +6,7 @@ extern void func_LdrEnumResources(void); extern void func_load_notifications(void); extern void func_NtAcceptConnectPort(void); +extern void func_NtAccessCheck(void); extern void func_NtAccessCheckByType(void); extern void func_NtAccessCheckByTypeResultList(void); extern void func_NtAdjustGroupsToken(void); @@ -102,6 +103,7 @@ const struct test winetest_testlist[] = { "LdrEnumResources", func_LdrEnumResources }, { "load_notifications", func_load_notifications }, { "NtAcceptConnectPort", func_NtAcceptConnectPort }, + { "NtAccessCheck", func_NtAccessCheck }, { "NtAccessCheckByType", func_NtAccessCheckByType }, { "NtAccessCheckByTypeResultList", func_NtAccessCheckByTypeResultList }, { "NtAdjustGroupsToken", func_NtAdjustGroupsToken },

1 0

[reactos] 02/05: [NTOS:SE] HACK: Temporarily grant access to the client if empty generic mapping was passed
by George Bișoc 04 Oct '23

04 Oct '23

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=4b4638dc5599d5061a01a… commit 4b4638dc5599d5061a01a21ed04a1ce9b68e732d Author: George Bișoc <george.bisoc(a)reactos.org> AuthorDate: Sat Sep 30 20:51:12 2023 +0200 Commit: George Bișoc <george.bisoc(a)reactos.org> CommitDate: Wed Oct 4 18:04:29 2023 +0200 [NTOS:SE] HACK: Temporarily grant access to the client if empty generic mapping was passed Certain apps such as AIM installer passes an empty generic mapping (this can be understood with their generic masks set to 0) and our code tries to map the access right from an ACE with the mapping provided by AccessCheck. This can lead to a bug where we would not be able to decode the generic right from an ACE as we need a proper generic mapping in order to do so. A mask right that is not decoded it cannot be used to mask out the remaining rights, further resulting into a denied access right. What Windows does instead is they are mapping the ACE's rights in another place, presumably when setting security data to an object, and they are using the generic mapping passed by the kernel. What we can do for the time being is to temporarily grant access to the client, but only if they are an administrator. CORE-18576 --- ntoskrnl/se/accesschk.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/ntoskrnl/se/accesschk.c b/ntoskrnl/se/accesschk.c index eaf159f8ce1..858bfeef994 100644 --- a/ntoskrnl/se/accesschk.c +++ b/ntoskrnl/se/accesschk.c @@ -1058,6 +1058,34 @@ SepAccessCheckWorker( goto ReturnCommonStatus; } + /* + * HACK: Temporary hack that checks if the caller passed an empty + * generic mapping. In such cases we cannot mask out the remaining + * access rights without a proper mapping so the only option we + * can do is to check if the client is an administrator, + * since they are powerful users. + * + * See CORE-18576 for information. + */ + if (GenericMapping->GenericRead == 0 && + GenericMapping->GenericWrite == 0 && + GenericMapping->GenericExecute == 0 && + GenericMapping->GenericAll == 0) + { + if (SeTokenIsAdmin(Token)) + { + /* Grant him access */ + PreviouslyGrantedAccess |= RemainingAccess; + Status = STATUS_SUCCESS; + goto ReturnCommonStatus; + } + + /* It's not an admin so bail out */ + PreviouslyGrantedAccess = 0; + Status = STATUS_ACCESS_DENIED; + goto ReturnCommonStatus; + } + /* Get the DACL */ Status = RtlGetDaclSecurityDescriptor(SecurityDescriptor, &Present,

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

Ros-diffs