From 7b99293b02fb4e9e3e90c81db0da9324c8372046 Mon Sep 17 00:00:00 2001 From: Joachim Henze Date: Wed, 2 Aug 2023 00:46:53 +0200 Subject: [PATCH] [0.4.11][NTOS][WIN32SS][UDFS] Fix double free in ObCreateObject, fix retvals CORE-14271 Backport the following commits: 0.4.15-dev-6401-g 53b30e3f3bf454356dad537c782543cba35e0f1a [NTOSKRNL][NTGDI] Formatting addendum, no functional change 0.4.15-dev-5487-g e7bbbf049e3c53f7477fb3c53e1910ac417066b5 [NTOS] Fix double free on allocation failure in ObCreateObject partially 0.4.13-dev-358-g 38db074491768faf5a8dd1e83139fd16a7681d92 [WIN32SS][UDFS] Misc addendum to CORE-14271 (#1529) 0.4.12-dev-648-g 17af7f0c2741f2a8173a0bd091e97b9bd14d5777 [UDFS] Correctly check SeSinglePrivilegeCheck() return value (#1324) CORE-14271 0.4.12-dev-552-g 0483a5a38089217868401fb9fe5e9885cddebcc6 [NTOS:OB] Correctly check SeSinglePrivilegeCheck() return value (#1323) CORE-14271 Just for verifying that the size didn't increase anywhere: ntoskrnl.exe GCC8.4.0dbg RosBEWin2.2.2 master 5.124.096 ntoskrnl.exe GCC4.7.2dbg RosBEWin2.1.6 0.4.14 4.413.440 -> 4.413.440 ntoskrnl.exe GCC4.7.2dbg RosBEWin2.1.6 0.4.13 4.367.360 -> 4.367.360 ntoskrnl.exe GCC4.7.2dbg RosBEWin2.1.6 0.4.12 4.377.600 -> 4.377.600 ntoskrnl.exe GCC4.7.2dbg RosBEWin2.1.6 0.4.11 4.377.088 -> 4.377.088 ntoskrnl.exe GCC4.7.2dbg RosBEWin2.1.6 0.4.10 4.344.320 -> 4.344.320 ntoskrnl.exe GCC4.7.2dbg RosBEWin2.1.6 0.4. 9 4.311.552 -> 4.311.552 ntoskrnl.exe GCC4.7.2dbg RosBEWin2.1.6 0.4. 8 4.296.704 -> 4.296.704 ntoskrnl.exe GCC4.7.2dbg RosBEWin2.1.6 0.4. 7 4.231.168 -> 4.231.168 udfs.sys GCC8.4.0dbg RosBEWin2.2.2 master 835.584 udfs.sys GCC4.7.2dbg RosBEWin2.1.6 0.4.14 745.472 -> 745.472 udfs.sys GCC4.7.2dbg RosBEWin2.1.6 0.4.13 745.472 -> 745.472 udfs.sys GCC4.7.2dbg RosBEWin2.1.6 0.4.12 749.568 -> 749.568 udfs.sys GCC4.7.2dbg RosBEWin2.1.6 0.4.11 749.568 -> 749.568 udfs.sys GCC4.7.2dbg RosBEWin2.1.6 0.4.10 749.568 -> 749.568 udfs.sys GCC4.7.2dbg RosBEWin2.1.6 0.4. 9 749.568 -> 749.568 udfs.sys GCC4.7.2dbg RosBEWin2.1.6 0.4. 8 749.568 -> 749.568 udfs.sys GCC4.7.2dbg RosBEWin2.1.6 0.4. 7 749.568 -> 749.568 user32.dll GCC8.4.0dbg RosBEWin2.2.2 master 1.585.152 user32.dll GCC4.7.2dbg RosBEWin2.1.6 0.4.14 1.448.448 -> 1.448.448 user32.dll GCC4.7.2dbg RosBEWin2.1.6 0.4.13 1.445.376 -> 1.445.376 user32.dll GCC4.7.2dbg RosBEWin2.1.6 0.4.12 1.455.616 -> 1.455.616 user32.dll GCC4.7.2dbg RosBEWin2.1.6 0.4.11 1.453.056 -> 1.453.056 user32.dll GCC4.7.2dbg RosBEWin2.1.6 0.4.10 1.434.624 -> 1.434.624 user32.dll GCC4.7.2dbg RosBEWin2.1.6 0.4. 9 1.422.336 -> 1.422.336 user32.dll GCC4.7.2dbg RosBEWin2.1.6 0.4. 8 1.421.824 -> 1.421.824 user32.dll GCC4.7.2dbg RosBEWin2.1.6 0.4. 7 1.418.752 -> 1.418.752 win32k.sys GCC8.4.0dbg RosBEWin2.2.2 master 3.477.504 win32k.sys GCC4.7.2dbg RosBEWin2.1.6 0.4.14 2.904.064 -> 2.904.064 win32k.sys GCC4.7.2dbg RosBEWin2.1.6 0.4.13 2.895.872 -> 2.895.872 win32k.sys GCC4.7.2dbg RosBEWin2.1.6 0.4.12 2.887.680 -> 2.887.680 win32k.sys GCC4.7.2dbg RosBEWin2.1.6 0.4.11 2.867.200 -> 2.867.200 win32k.sys GCC4.7.2dbg RosBEWin2.1.6 0.4.10 2.863.104 -> 2.863.104 win32k.sys GCC4.7.2dbg RosBEWin2.1.6 0.4. 9 2.834.432 -> 2.834.432 win32k.sys GCC4.7.2dbg RosBEWin2.1.6 0.4. 8 2.830.336 -> 2.830.336 win32k.sys GCC4.7.2dbg RosBEWin2.1.6 0.4. 7 2.830.336 -> 2.830.336 --- drivers/filesystems/udfs/create.cpp | 6 +++--- drivers/filesystems/udfs/secursup.cpp | 5 ++--- ntoskrnl/config/cmsysini.c | 8 +------- ntoskrnl/ob/oblife.c | 17 +++++++++-------- win32ss/gdi/ntgdi/freetype.c | 8 ++------ win32ss/user/user32/windows/clipboard.c | 20 +++++++------------- 6 files changed, 24 insertions(+), 40 deletions(-) diff --git a/drivers/filesystems/udfs/create.cpp b/drivers/filesystems/udfs/create.cpp index c28819549de..3b3c78a7d3f 100644 --- a/drivers/filesystems/udfs/create.cpp +++ b/drivers/filesystems/udfs/create.cpp @@ -742,9 +742,9 @@ op_vol_accs_dnd: // we should check appropriate privilege if OpenForBackup requested if(OpenForBackup) { - RC = SeSinglePrivilegeCheck(SeExports->SeBackupPrivilege, UserMode); - if(!NT_SUCCESS(RC)) - try_return(RC); + if (!SeSinglePrivilegeCheck(SeExports->SeBackupPrivilege, UserMode)) { + try_return(RC = STATUS_PRIVILEGE_NOT_HELD); + } } // The FSD might wish to implement the open-by-id option. The "id" diff --git a/drivers/filesystems/udfs/secursup.cpp b/drivers/filesystems/udfs/secursup.cpp index de5bb1536d3..8e4a5e0548e 100644 --- a/drivers/filesystems/udfs/secursup.cpp +++ b/drivers/filesystems/udfs/secursup.cpp @@ -934,9 +934,9 @@ UDFCheckAccessRights( ) { NTSTATUS RC; - BOOLEAN SecurityCheck = TRUE; BOOLEAN ROCheck = FALSE; #ifdef UDF_ENABLE_SECURITY + BOOLEAN SecurityCheck; PSECURITY_DESCRIPTOR SecDesc; SECURITY_SUBJECT_CONTEXT SubjectContext; ACCESS_MASK LocalAccessMask; @@ -1011,8 +1011,7 @@ treat_as_ro: } else #endif //UDF_ENABLE_SECURITY if(DesiredAccess & ACCESS_SYSTEM_SECURITY) { - SecurityCheck = SeSinglePrivilegeCheck(SeExports->SeSecurityPrivilege, UserMode); - if(!SecurityCheck) + if (!SeSinglePrivilegeCheck(SeExports->SeSecurityPrivilege, UserMode)) return STATUS_ACCESS_DENIED; Ccb->PreviouslyGrantedAccess |= ACCESS_SYSTEM_SECURITY; } diff --git a/ntoskrnl/config/cmsysini.c b/ntoskrnl/config/cmsysini.c index 295e4b4376d..f380b4defe3 100644 --- a/ntoskrnl/config/cmsysini.c +++ b/ntoskrnl/config/cmsysini.c @@ -1,7 +1,6 @@ /* * PROJECT: ReactOS Kernel * LICENSE: BSD - See COPYING.ARM in the top level directory - * FILE: ntoskrnl/config/cmsysini.c * PURPOSE: Configuration Manager - System Initialization Code * PROGRAMMERS: ReactOS Portable Systems Group * Alex Ionescu (alex.ionescu@reactos.org) @@ -873,7 +872,6 @@ CmpInitializeSystemHive(IN PLOADER_PARAMETER_BLOCK LoaderBlock) UNICODE_STRING KeyName; PCMHIVE SystemHive = NULL; PSECURITY_DESCRIPTOR SecurityDescriptor; - BOOLEAN Success; PAGED_CODE(); @@ -921,12 +919,8 @@ CmpInitializeSystemHive(IN PLOADER_PARAMETER_BLOCK LoaderBlock) } /* Set the hive filename */ - Success = RtlCreateUnicodeString(&SystemHive->FileFullPath, - L"\\SystemRoot\\System32\\Config\\SYSTEM"); - if (!Success) - { + if (!RtlCreateUnicodeString(&SystemHive->FileFullPath, L"\\SystemRoot\\System32\\Config\\SYSTEM")) return FALSE; - } /* Manually set the hive as volatile, if in Live CD mode */ if (HiveBase && CmpShareSystemHives) diff --git a/ntoskrnl/ob/oblife.c b/ntoskrnl/ob/oblife.c index 6cc1403e631..a37848ad561 100644 --- a/ntoskrnl/ob/oblife.c +++ b/ntoskrnl/ob/oblife.c @@ -872,10 +872,11 @@ ObpAllocateObject(IN POBJECT_CREATE_INFORMATION ObjectCreateInfo, NTSTATUS NTAPI -ObQueryTypeInfo(IN POBJECT_TYPE ObjectType, - OUT POBJECT_TYPE_INFORMATION ObjectTypeInfo, - IN ULONG Length, - OUT PULONG ReturnLength) +ObQueryTypeInfo( + IN POBJECT_TYPE ObjectType, + OUT POBJECT_TYPE_INFORMATION ObjectTypeInfo, + IN ULONG Length, + OUT PULONG ReturnLength) { NTSTATUS Status = STATUS_SUCCESS; PWSTR InfoBuffer; @@ -887,7 +888,7 @@ ObQueryTypeInfo(IN POBJECT_TYPE ObjectType, *ReturnLength += sizeof(*ObjectTypeInfo) + ALIGN_UP(ObjectType->Name.MaximumLength, ULONG); - /* Check if thats too much though. */ + /* Check if that is too much */ if (Length < *ReturnLength) { _SEH2_YIELD(return STATUS_INFO_LENGTH_MISMATCH); @@ -1036,6 +1037,7 @@ ObCreateObject(IN KPROCESSOR_MODE ProbeMode OPTIONAL, /* Release the Capture Info, we don't need it */ ObpFreeObjectCreateInformation(ObjectCreateInfo); if (ObjectName.Buffer) ObpFreeObjectNameBuffer(&ObjectName); + return Status; } /* We failed, so release the Buffer */ @@ -1427,9 +1429,8 @@ NtMakePermanentObject(IN HANDLE ObjectHandle) PAGED_CODE(); /* Make sure that the caller has SeCreatePermanentPrivilege */ - Status = SeSinglePrivilegeCheck(SeCreatePermanentPrivilege, - PreviousMode); - if (!NT_SUCCESS(Status)) return STATUS_PRIVILEGE_NOT_HELD; + if (!SeSinglePrivilegeCheck(SeCreatePermanentPrivilege, PreviousMode)) + return STATUS_PRIVILEGE_NOT_HELD; /* Reference the object */ Status = ObReferenceObjectByHandle(ObjectHandle, diff --git a/win32ss/gdi/ntgdi/freetype.c b/win32ss/gdi/ntgdi/freetype.c index 8634cba99d4..eca2bdb876f 100644 --- a/win32ss/gdi/ntgdi/freetype.c +++ b/win32ss/gdi/ntgdi/freetype.c @@ -1,7 +1,6 @@ /* * PROJECT: ReactOS win32 kernel mode subsystem * LICENSE: GPL - See COPYING in the top level directory - * FILE: win32ss/gdi/ntgdi/freetype.c * PURPOSE: FreeType font engine interface * PROGRAMMERS: Copyright 2001 Huw D M Davies for CodeWeavers. * Copyright 2006 Dmitry Timoshkov for CodeWeavers. @@ -474,7 +473,6 @@ IntLoadFontSubstList(PLIST_ENTRY pHead) BYTE CharSets[FONTSUBST_FROM_AND_TO]; LPWSTR pch; PFONTSUBST_ENTRY pEntry; - BOOLEAN Success; /* the FontSubstitutes registry key */ static UNICODE_STRING FontSubstKey = @@ -519,8 +517,7 @@ IntLoadFontSubstList(PLIST_ENTRY pHead) pInfo = (PKEY_VALUE_FULL_INFORMATION)InfoBuffer; Length = pInfo->NameLength / sizeof(WCHAR); pInfo->Name[Length] = UNICODE_NULL; /* truncate */ - Success = RtlCreateUnicodeString(&FromW, pInfo->Name); - if (!Success) + if (!RtlCreateUnicodeString(&FromW, pInfo->Name)) { Status = STATUS_INSUFFICIENT_RESOURCES; DPRINT("RtlCreateUnicodeString failed\n"); @@ -542,8 +539,7 @@ IntLoadFontSubstList(PLIST_ENTRY pHead) pch = (LPWSTR)((PUCHAR)pInfo + pInfo->DataOffset); Length = pInfo->DataLength / sizeof(WCHAR); pch[Length] = UNICODE_NULL; /* truncate */ - Success = RtlCreateUnicodeString(&ToW, pch); - if (!Success) + if (!RtlCreateUnicodeString(&ToW, pch)) { Status = STATUS_INSUFFICIENT_RESOURCES; DPRINT("RtlCreateUnicodeString failed\n"); diff --git a/win32ss/user/user32/windows/clipboard.c b/win32ss/user/user32/windows/clipboard.c index 66a4217301e..15ece3a63c8 100644 --- a/win32ss/user/user32/windows/clipboard.c +++ b/win32ss/user/user32/windows/clipboard.c @@ -91,7 +91,7 @@ UINT WINAPI RegisterClipboardFormatA(LPCSTR lpszFormat) { - UINT ret = 0; + UINT ret; UNICODE_STRING usFormat = {0}; if (lpszFormat == NULL) @@ -100,19 +100,17 @@ RegisterClipboardFormatA(LPCSTR lpszFormat) return 0; } - /* check for "" */ if (*lpszFormat == 0) //NULL { SetLastError(ERROR_INVALID_NAME); return 0; } - ret = RtlCreateUnicodeStringFromAsciiz(&usFormat, lpszFormat); - if (ret) - { - ret = NtUserRegisterWindowMessage(&usFormat); //(LPCWSTR) - RtlFreeUnicodeString(&usFormat); - } + if (!RtlCreateUnicodeStringFromAsciiz(&usFormat, lpszFormat)) + return 0; + + ret = NtUserRegisterWindowMessage(&usFormat); //(LPCWSTR) + RtlFreeUnicodeString(&usFormat); return ret; } @@ -124,7 +122,6 @@ UINT WINAPI RegisterClipboardFormatW(LPCWSTR lpszFormat) { - UINT ret = 0; UNICODE_STRING usFormat = {0}; if (lpszFormat == NULL) @@ -133,7 +130,6 @@ RegisterClipboardFormatW(LPCWSTR lpszFormat) return 0; } - /* check for "" */ if (*lpszFormat == 0) //NULL { SetLastError(ERROR_INVALID_NAME); @@ -141,9 +137,7 @@ RegisterClipboardFormatW(LPCWSTR lpszFormat) } RtlInitUnicodeString(&usFormat, lpszFormat); - ret = NtUserRegisterWindowMessage(&usFormat); - - return ret; + return NtUserRegisterWindowMessage(&usFormat); } static PVOID WINAPI