- use inlined probing macros for basic types - documented dozens of vulnerabilities in NtOpenThread, NtCreateThread and NtOpenProcess (owner may fix them) Modified: trunk/reactos/ntoskrnl/ps/job.c Modified: trunk/reactos/ntoskrnl/ps/locale.c Modified: trunk/reactos/ntoskrnl/ps/process.c Modified: trunk/reactos/ntoskrnl/ps/query.c Modified: trunk/reactos/ntoskrnl/ps/security.c Modified: trunk/reactos/ntoskrnl/ps/suspend.c Modified: trunk/reactos/ntoskrnl/ps/thread.c _____
Modified: trunk/reactos/ntoskrnl/ps/job.c --- trunk/reactos/ntoskrnl/ps/job.c 2005-08-21 15:14:36 UTC (rev 17461) +++ trunk/reactos/ntoskrnl/ps/job.c 2005-08-21 15:38:47 UTC (rev 17462) @@ -216,10 +216,7 @@
{ _SEH_TRY { - /* probe with 32bit alignment */ - ProbeForWrite(JobHandle, - sizeof(HANDLE), - sizeof(ULONG)); + ProbeForWriteHandle(JobHandle); } _SEH_HANDLE { @@ -389,10 +386,7 @@ { _SEH_TRY { - /* probe with 32bit alignment */ - ProbeForWrite(JobHandle, - sizeof(HANDLE), - sizeof(ULONG)); + ProbeForWriteHandle(JobHandle); } _SEH_HANDLE { _____
Modified: trunk/reactos/ntoskrnl/ps/locale.c --- trunk/reactos/ntoskrnl/ps/locale.c 2005-08-21 15:14:36 UTC (rev 17461) +++ trunk/reactos/ntoskrnl/ps/locale.c 2005-08-21 15:38:47 UTC (rev 17462) @@ -199,28 +199,40 @@
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 @@ 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 @@ &ObjectAttributes); if (!NT_SUCCESS(Status)) { - *LanguageId = PsInstallUILanguageId; - return STATUS_SUCCESS; + Value = PsInstallUILanguageId; + goto ReturnSuccess; }
ValueInfo = (PKEY_VALUE_PARTIAL_INFORMATION)ValueBuffer; @@ -393,8 +425,8 @@
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 @@ &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 @@ 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; }
_____
Modified: trunk/reactos/ntoskrnl/ps/process.c --- trunk/reactos/ntoskrnl/ps/process.c 2005-08-21 15:14:36 UTC (rev 17461) +++ trunk/reactos/ntoskrnl/ps/process.c 2005-08-21 15:38:47 UTC (rev 17462) @@ -870,6 +870,7 @@
IN HANDLE DebugPort OPTIONAL, IN HANDLE ExceptionPort OPTIONAL) { + HANDLE hProcess; KPROCESSOR_MODE PreviousMode = ExGetPreviousMode(); NTSTATUS Status = STATUS_SUCCESS;
@@ -880,9 +881,7 @@ { _SEH_TRY { - ProbeForWrite(ProcessHandle, - sizeof(HANDLE), - sizeof(ULONG)); + ProbeForWriteHandle(ProcessHandle); } _SEH_HANDLE { @@ -901,8 +900,9 @@ } 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 @@ SectionHandle, DebugPort, ExceptionPort); + if (NT_SUCCESS(Status)) + { + _SEH_TRY + { + *ProcessHandle = hProcess; + } + _SEH_HANDLE + { + Status = _SEH_GetExceptionCode(); + } + _SEH_END; + } }
/* Return Status */ @@ -940,7 +952,7 @@
/* 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 @@ 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 @@ 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 @@
/* 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"); _____
Modified: trunk/reactos/ntoskrnl/ps/query.c --- trunk/reactos/ntoskrnl/ps/query.c 2005-08-21 15:14:36 UTC (rev 17461) +++ trunk/reactos/ntoskrnl/ps/query.c 2005-08-21 15:38:47 UTC (rev 17462) @@ -1303,9 +1303,7 @@
1); if (ReturnLength != NULL) { - ProbeForWrite(ReturnLength, - sizeof(ULONG), - sizeof(ULONG)); + ProbeForWriteUlong(ReturnLength); } } _SEH_HANDLE _____
Modified: trunk/reactos/ntoskrnl/ps/security.c --- trunk/reactos/ntoskrnl/ps/security.c 2005-08-21 15:14:36 UTC (rev 17461) +++ trunk/reactos/ntoskrnl/ps/security.c 2005-08-21 15:38:47 UTC (rev 17462) @@ -79,9 +79,7 @@
{ _SEH_TRY { - ProbeForWrite(TokenHandle, - sizeof(HANDLE), - sizeof(ULONG)); + ProbeForWriteHandle(TokenHandle); } _SEH_HANDLE { _____
Modified: trunk/reactos/ntoskrnl/ps/suspend.c --- trunk/reactos/ntoskrnl/ps/suspend.c 2005-08-21 15:14:36 UTC (rev 17461) +++ trunk/reactos/ntoskrnl/ps/suspend.c 2005-08-21 15:38:47 UTC (rev 17462) @@ -49,9 +49,7 @@
_SEH_TRY {
- ProbeForWrite(SuspendCount, - sizeof(ULONG), - sizeof(ULONG)); + ProbeForWriteUlong(SuspendCount); } _SEH_HANDLE {
Status = _SEH_GetExceptionCode(); @@ -124,9 +122,7 @@ { _SEH_TRY { - ProbeForWrite(PreviousSuspendCount, - sizeof(ULONG), - sizeof(ULONG)); + ProbeForWriteUlong(PreviousSuspendCount); } _SEH_HANDLE { _____
Modified: trunk/reactos/ntoskrnl/ps/thread.c --- trunk/reactos/ntoskrnl/ps/thread.c 2005-08-21 15:14:36 UTC (rev 17461) +++ trunk/reactos/ntoskrnl/ps/thread.c 2005-08-21 15:38:47 UTC (rev 17462) @@ -597,9 +597,7 @@
_SEH_TRY {
- ProbeForWrite(ThreadHandle, - sizeof(HANDLE), - sizeof(ULONG)); + ProbeForWriteHandle(ThreadHandle);
if(ClientId != NULL) {
@@ -632,18 +630,18 @@ }
/* 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 @@ { _SEH_TRY { - ProbeForWrite(ThreadHandle, - sizeof(HANDLE), - sizeof(ULONG)); + ProbeForWriteHandle(ThreadHandle);
if(ClientId != NULL) { @@ -696,7 +692,7 @@ }
/* 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 @@ { 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 @@
/* Open the Thread Object */ Status = ObOpenObjectByPointer(Thread, - ObjectAttributes->Attributes, + ObjectAttributes->Attributes, /* FIXME - neither probed nor protected! */ NULL, DesiredAccess, PsThreadType,