From 10cd89fb4e5cfefdb8a87755d498ceb45bd8ec10 Mon Sep 17 00:00:00 2001 From: Thomas Bluemel Date: Sun, 21 Aug 2005 15:38:47 +0000 Subject: [PATCH] - use inlined probing macros for basic types - documented dozens of vulnerabilities in NtOpenThread, NtCreateThread and NtOpenProcess (owner may fix them) svn path=/trunk/; revision=17462 --- reactos/ntoskrnl/ps/job.c | 10 +-- reactos/ntoskrnl/ps/locale.c | 113 +++++++++++++++++++++++++-------- reactos/ntoskrnl/ps/process.c | 38 +++++++---- reactos/ntoskrnl/ps/query.c | 4 +- reactos/ntoskrnl/ps/security.c | 4 +- reactos/ntoskrnl/ps/suspend.c | 8 +-- reactos/ntoskrnl/ps/thread.c | 30 ++++----- 7 files changed, 129 insertions(+), 78 deletions(-) diff --git a/reactos/ntoskrnl/ps/job.c b/reactos/ntoskrnl/ps/job.c index 90b2d632af3..e38dd3a5873 100644 --- a/reactos/ntoskrnl/ps/job.c +++ b/reactos/ntoskrnl/ps/job.c @@ -216,10 +216,7 @@ NtCreateJobObject ( { _SEH_TRY { - /* probe with 32bit alignment */ - ProbeForWrite(JobHandle, - sizeof(HANDLE), - sizeof(ULONG)); + ProbeForWriteHandle(JobHandle); } _SEH_HANDLE { @@ -389,10 +386,7 @@ NtOpenJobObject ( { _SEH_TRY { - /* probe with 32bit alignment */ - ProbeForWrite(JobHandle, - sizeof(HANDLE), - sizeof(ULONG)); + ProbeForWriteHandle(JobHandle); } _SEH_HANDLE { diff --git a/reactos/ntoskrnl/ps/locale.c b/reactos/ntoskrnl/ps/locale.c index 65e4183cf1f..1689a5a40ff 100644 --- a/reactos/ntoskrnl/ps/locale.c +++ b/reactos/ntoskrnl/ps/locale.c @@ -199,28 +199,40 @@ NTSTATUS STDCALL NtQueryDefaultLocale(IN BOOLEAN UserProfile, OUT PLCID DefaultLocaleId) { - PAGED_CODE(); + NTSTATUS Status = STATUS_SUCCESS; - if (DefaultLocaleId == NULL) - return STATUS_UNSUCCESSFUL; + PAGED_CODE(); - if (UserProfile) + _SEH_TRY { - if (!PsDefaultThreadLocaleInitialized) - { - PiInitThreadLocale(); - } + if (KeGetPreviousMode() != KernelMode) + { + ProbeForWriteLangid(DefaultLocaleId); + } + + if (UserProfile) + { + if (!PsDefaultThreadLocaleInitialized) + { + PiInitThreadLocale(); + } - /* set thread locale */ - *DefaultLocaleId = PsDefaultThreadLocaleId; + /* set thread locale */ + *DefaultLocaleId = PsDefaultThreadLocaleId; + } + else + { + /* set system locale */ + *DefaultLocaleId = PsDefaultSystemLocaleId; + } } - else + _SEH_EXCEPT(_SEH_ExSystemExceptionFilter) { - /* set system locale */ - *DefaultLocaleId = PsDefaultSystemLocaleId; + Status = _SEH_GetExceptionCode(); } + _SEH_END; - return STATUS_SUCCESS; + return Status; } @@ -353,16 +365,36 @@ NtQueryDefaultUILanguage(OUT PLANGID LanguageId) ULONG Value; HANDLE UserKey; HANDLE KeyHandle; - NTSTATUS Status; + NTSTATUS Status = STATUS_SUCCESS; PAGED_CODE(); + + _SEH_TRY + { + if (KeGetPreviousMode() != KernelMode) + { + ProbeForWriteLangid(LanguageId); + } + + *LanguageId = PsInstallUILanguageId; + } + _SEH_EXCEPT(_SEH_ExSystemExceptionFilter) + { + Status = _SEH_GetExceptionCode(); + } + _SEH_END; + + if (!NT_SUCCESS(Status)) + { + return Status; + } Status = RtlOpenCurrentUser(KEY_READ, &UserKey); if (!NT_SUCCESS(Status)) { - *LanguageId = PsInstallUILanguageId; - return STATUS_SUCCESS; + Value = PsInstallUILanguageId; + goto ReturnSuccess; } InitializeObjectAttributes(&ObjectAttributes, @@ -375,8 +407,8 @@ NtQueryDefaultUILanguage(OUT PLANGID LanguageId) &ObjectAttributes); if (!NT_SUCCESS(Status)) { - *LanguageId = PsInstallUILanguageId; - return STATUS_SUCCESS; + Value = PsInstallUILanguageId; + goto ReturnSuccess; } ValueInfo = (PKEY_VALUE_PARTIAL_INFORMATION)ValueBuffer; @@ -393,8 +425,8 @@ NtQueryDefaultUILanguage(OUT PLANGID LanguageId) if (!NT_SUCCESS(Status) || ValueInfo->Type != REG_SZ) { - *LanguageId = PsInstallUILanguageId; - return STATUS_SUCCESS; + Value = PsInstallUILanguageId; + goto ReturnSuccess; } ValueString.Length = ValueInfo->DataLength; @@ -406,15 +438,25 @@ NtQueryDefaultUILanguage(OUT PLANGID LanguageId) &Value); if (!NT_SUCCESS(Status)) { - *LanguageId = PsInstallUILanguageId; - return STATUS_SUCCESS; + Value = PsInstallUILanguageId; + goto ReturnSuccess; } DPRINT("Default language id: %04lx\n", Value); - *LanguageId = Value; +ReturnSuccess: + _SEH_TRY + { + *LanguageId = Value; + Status = STATUS_SUCCESS; + } + _SEH_EXCEPT(_SEH_ExSystemExceptionFilter) + { + Status = _SEH_GetExceptionCode(); + } + _SEH_END; - return STATUS_SUCCESS; + return Status; } @@ -424,11 +466,26 @@ NtQueryDefaultUILanguage(OUT PLANGID LanguageId) NTSTATUS STDCALL NtQueryInstallUILanguage(OUT PLANGID LanguageId) { - PAGED_CODE(); + NTSTATUS Status = STATUS_SUCCESS; + + PAGED_CODE(); - *LanguageId = PsInstallUILanguageId; + _SEH_TRY + { + if (KeGetPreviousMode() != KernelMode) + { + ProbeForWriteLangid(LanguageId); + } - return STATUS_SUCCESS; + *LanguageId = PsInstallUILanguageId; + } + _SEH_EXCEPT(_SEH_ExSystemExceptionFilter) + { + Status = _SEH_GetExceptionCode(); + } + _SEH_END; + + return Status; } diff --git a/reactos/ntoskrnl/ps/process.c b/reactos/ntoskrnl/ps/process.c index 01426cd0f77..c36006cc6d9 100644 --- a/reactos/ntoskrnl/ps/process.c +++ b/reactos/ntoskrnl/ps/process.c @@ -870,6 +870,7 @@ NtCreateProcess(OUT PHANDLE ProcessHandle, IN HANDLE DebugPort OPTIONAL, IN HANDLE ExceptionPort OPTIONAL) { + HANDLE hProcess; KPROCESSOR_MODE PreviousMode = ExGetPreviousMode(); NTSTATUS Status = STATUS_SUCCESS; @@ -880,9 +881,7 @@ NtCreateProcess(OUT PHANDLE ProcessHandle, { _SEH_TRY { - ProbeForWrite(ProcessHandle, - sizeof(HANDLE), - sizeof(ULONG)); + ProbeForWriteHandle(ProcessHandle); } _SEH_HANDLE { @@ -901,8 +900,9 @@ NtCreateProcess(OUT PHANDLE ProcessHandle, } else { - /* Create a user Process */ - Status = PspCreateProcess(ProcessHandle, + /* Create a user Process, do NOT pass the pointer to the handle supplied + by the caller directly!!! */ + Status = PspCreateProcess(&hProcess, DesiredAccess, ObjectAttributes, ParentProcess, @@ -910,6 +910,18 @@ NtCreateProcess(OUT PHANDLE ProcessHandle, SectionHandle, DebugPort, ExceptionPort); + if (NT_SUCCESS(Status)) + { + _SEH_TRY + { + *ProcessHandle = hProcess; + } + _SEH_HANDLE + { + Status = _SEH_GetExceptionCode(); + } + _SEH_END; + } } /* Return Status */ @@ -940,7 +952,7 @@ NtOpenProcess(OUT PHANDLE ProcessHandle, /* Open by name if one was given */ DPRINT("Checking type\n"); - if (ObjectAttributes->ObjectName) + if (ObjectAttributes->ObjectName) /* FIXME - neither probed nor protected! */ { /* Open it */ DPRINT("Opening by name\n"); @@ -964,11 +976,11 @@ NtOpenProcess(OUT PHANDLE ProcessHandle, else if (ClientId) { /* Open by Thread ID */ - if (ClientId->UniqueThread) + if (ClientId->UniqueThread) /* FIXME - neither probed nor protected! */ { /* Get the Process */ - DPRINT("Opening by Thread ID: %x\n", ClientId->UniqueThread); - Status = PsLookupProcessThreadByCid(ClientId, + DPRINT("Opening by Thread ID: %x\n", ClientId->UniqueThread); /* FIXME - neither probed nor protected! */ + Status = PsLookupProcessThreadByCid(ClientId, /* FIXME - neither probed nor protected! */ &Process, &Thread); DPRINT("Found: %x\n", Process); @@ -976,8 +988,8 @@ NtOpenProcess(OUT PHANDLE ProcessHandle, else { /* Get the Process */ - DPRINT("Opening by Process ID: %x\n", ClientId->UniqueProcess); - Status = PsLookupProcessByProcessId(ClientId->UniqueProcess, + DPRINT("Opening by Process ID: %x\n", ClientId->UniqueProcess); /* FIXME - neither probed nor protected! */ + Status = PsLookupProcessByProcessId(ClientId->UniqueProcess, /* FIXME - neither probed nor protected! */ &Process); DPRINT("Found: %x\n", Process); } @@ -990,12 +1002,12 @@ NtOpenProcess(OUT PHANDLE ProcessHandle, /* Open the Process Object */ Status = ObOpenObjectByPointer(Process, - ObjectAttributes->Attributes, + ObjectAttributes->Attributes, /* FIXME - neither probed nor protected! */ NULL, DesiredAccess, PsProcessType, PreviousMode, - ProcessHandle); + ProcessHandle); /* FIXME - neither probed nor protected! */ if(!NT_SUCCESS(Status)) { DPRINT1("Failure to open process\n"); diff --git a/reactos/ntoskrnl/ps/query.c b/reactos/ntoskrnl/ps/query.c index 6b385a4cd87..f32c0175879 100644 --- a/reactos/ntoskrnl/ps/query.c +++ b/reactos/ntoskrnl/ps/query.c @@ -1303,9 +1303,7 @@ NtQueryInformationThread (IN HANDLE ThreadHandle, 1); if (ReturnLength != NULL) { - ProbeForWrite(ReturnLength, - sizeof(ULONG), - sizeof(ULONG)); + ProbeForWriteUlong(ReturnLength); } } _SEH_HANDLE diff --git a/reactos/ntoskrnl/ps/security.c b/reactos/ntoskrnl/ps/security.c index 8707afaffa2..c78aac1f55d 100644 --- a/reactos/ntoskrnl/ps/security.c +++ b/reactos/ntoskrnl/ps/security.c @@ -79,9 +79,7 @@ NtOpenProcessTokenEx(IN HANDLE ProcessHandle, { _SEH_TRY { - ProbeForWrite(TokenHandle, - sizeof(HANDLE), - sizeof(ULONG)); + ProbeForWriteHandle(TokenHandle); } _SEH_HANDLE { diff --git a/reactos/ntoskrnl/ps/suspend.c b/reactos/ntoskrnl/ps/suspend.c index 98e3a90abda..e50478c0d56 100644 --- a/reactos/ntoskrnl/ps/suspend.c +++ b/reactos/ntoskrnl/ps/suspend.c @@ -49,9 +49,7 @@ NtResumeThread(IN HANDLE ThreadHandle, _SEH_TRY { - ProbeForWrite(SuspendCount, - sizeof(ULONG), - sizeof(ULONG)); + ProbeForWriteUlong(SuspendCount); } _SEH_HANDLE { Status = _SEH_GetExceptionCode(); @@ -124,9 +122,7 @@ NtSuspendThread(IN HANDLE ThreadHandle, { _SEH_TRY { - ProbeForWrite(PreviousSuspendCount, - sizeof(ULONG), - sizeof(ULONG)); + ProbeForWriteUlong(PreviousSuspendCount); } _SEH_HANDLE { diff --git a/reactos/ntoskrnl/ps/thread.c b/reactos/ntoskrnl/ps/thread.c index 5a81d25032f..242bda78596 100644 --- a/reactos/ntoskrnl/ps/thread.c +++ b/reactos/ntoskrnl/ps/thread.c @@ -597,9 +597,7 @@ NtCreateThread(OUT PHANDLE ThreadHandle, _SEH_TRY { - ProbeForWrite(ThreadHandle, - sizeof(HANDLE), - sizeof(ULONG)); + ProbeForWriteHandle(ThreadHandle); if(ClientId != NULL) { @@ -632,18 +630,18 @@ NtCreateThread(OUT PHANDLE ThreadHandle, } /* Use probed data for the Initial TEB */ - SafeInitialTeb = *InitialTeb; + SafeInitialTeb = *InitialTeb; /* FIXME - not protected! */ InitialTeb = &SafeInitialTeb; /* Call the shared function */ - return PspCreateThread(ThreadHandle, + return PspCreateThread(ThreadHandle, /* FIXME - not protected! */ DesiredAccess, ObjectAttributes, ProcessHandle, NULL, - ClientId, - ThreadContext, - InitialTeb, + ClientId, /* FIXME - not protected! */ + ThreadContext, /* FIXME - not protected! */ + InitialTeb, /* FIXME - not protected! */ CreateSuspended, NULL, NULL); @@ -672,9 +670,7 @@ NtOpenThread(OUT PHANDLE ThreadHandle, { _SEH_TRY { - ProbeForWrite(ThreadHandle, - sizeof(HANDLE), - sizeof(ULONG)); + ProbeForWriteHandle(ThreadHandle); if(ClientId != NULL) { @@ -696,7 +692,7 @@ NtOpenThread(OUT PHANDLE ThreadHandle, } /* Open by name if one was given */ - if (ObjectAttributes->ObjectName) + if (ObjectAttributes->ObjectName) /* FIXME - neither probed nor protected! */ { /* Open it */ Status = ObOpenObjectByName(ObjectAttributes, @@ -711,18 +707,18 @@ NtOpenThread(OUT PHANDLE ThreadHandle, { DPRINT1("Could not open object by name\n"); } - + /* FIXME - would be a good idea to return the handle in case of success! */ /* Return Status */ return(Status); } else if (ClientId) { /* Open by Thread ID */ - if (ClientId->UniqueProcess) + if (ClientId->UniqueProcess) /* FIXME - neither probed nor protected! */ { /* Get the Process */ - DPRINT("Opening by Process ID: %x\n", ClientId->UniqueProcess); - Status = PsLookupProcessThreadByCid(ClientId, + DPRINT("Opening by Process ID: %x\n", ClientId->UniqueProcess); /* FIXME - neither probed nor protected! */ + Status = PsLookupProcessThreadByCid(ClientId, /* FIXME - neither probed nor protected! */ NULL, &Thread); } @@ -742,7 +738,7 @@ NtOpenThread(OUT PHANDLE ThreadHandle, /* Open the Thread Object */ Status = ObOpenObjectByPointer(Thread, - ObjectAttributes->Attributes, + ObjectAttributes->Attributes, /* FIXME - neither probed nor protected! */ NULL, DesiredAccess, PsThreadType,