[tkreuzer] 67140: [RTL] Improve RtlImageNtHeaderEx: - Fix signed/unsigned mismatch when comparing NT header offset - Simplify overflow checks - Add missing overflow-into-systemspace check CR-77 / C...
Author: tkreuzer Date: Fri Apr 10 19:21:22 2015 New Revision: 67140
URL: http://svn.reactos.org/svn/reactos?rev=67140&view=rev Log: [RTL] Improve RtlImageNtHeaderEx: - Fix signed/unsigned mismatch when comparing NT header offset - Simplify overflow checks - Add missing overflow-into-systemspace check CR-77 / CORE-8091 #resolve
Modified: trunk/reactos/boot/freeldr/freeldr/lib/rtl/libsupp.c trunk/reactos/dll/ntdll/rtl/libsupp.c trunk/reactos/include/ndk/amd64/mmtypes.h trunk/reactos/include/ndk/i386/mmtypes.h trunk/reactos/lib/rtl/image.c trunk/reactos/lib/rtl/rtlp.h trunk/reactos/ntoskrnl/include/internal/amd64/mm.h trunk/reactos/ntoskrnl/mm/ARM3/miarm.h
Modified: trunk/reactos/boot/freeldr/freeldr/lib/rtl/libsupp.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/boot/freeldr/freeldr/lib/rt... ============================================================================== --- trunk/reactos/boot/freeldr/freeldr/lib/rtl/libsupp.c [iso-8859-1] (original) +++ trunk/reactos/boot/freeldr/freeldr/lib/rtl/libsupp.c [iso-8859-1] Fri Apr 10 19:21:22 2015 @@ -19,6 +19,8 @@ */
#include <freeldr.h> + +PVOID MmHighestUserAddress = (PVOID)MI_HIGHEST_USER_ADDRESS;
#if DBG VOID FASTCALL
Modified: trunk/reactos/dll/ntdll/rtl/libsupp.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/ntdll/rtl/libsupp.c?rev... ============================================================================== --- trunk/reactos/dll/ntdll/rtl/libsupp.c [iso-8859-1] (original) +++ trunk/reactos/dll/ntdll/rtl/libsupp.c [iso-8859-1] Fri Apr 10 19:21:22 2015 @@ -16,6 +16,7 @@
SIZE_T RtlpAllocDeallocQueryBufferSize = PAGE_SIZE; PTEB LdrpTopLevelDllBeingLoadedTeb = NULL; +PVOID MmHighestUserAddress = (PVOID)MI_HIGHEST_USER_ADDRESS;
/* FUNCTIONS ***************************************************************/
Modified: trunk/reactos/include/ndk/amd64/mmtypes.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/include/ndk/amd64/mmtypes.h... ============================================================================== --- trunk/reactos/include/ndk/amd64/mmtypes.h [iso-8859-1] (original) +++ trunk/reactos/include/ndk/amd64/mmtypes.h [iso-8859-1] Fri Apr 10 19:21:22 2015 @@ -36,6 +36,11 @@ #define MM_ALLOCATION_GRANULARITY 0x10000 #define MM_ALLOCATION_GRANULARITY_SHIFT 16L #define MM_PAGE_FRAME_NUMBER_SIZE 52 + +// +// User space range limit +// +#define MI_HIGHEST_USER_ADDRESS (PVOID)0x000007FFFFFEFFFFULL
// // Address of the shared user page
Modified: trunk/reactos/include/ndk/i386/mmtypes.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/include/ndk/i386/mmtypes.h?... ============================================================================== --- trunk/reactos/include/ndk/i386/mmtypes.h [iso-8859-1] (original) +++ trunk/reactos/include/ndk/i386/mmtypes.h [iso-8859-1] Fri Apr 10 19:21:22 2015 @@ -35,6 +35,11 @@ #define MM_ALLOCATION_GRANULARITY 0x10000 #define MM_ALLOCATION_GRANULARITY_SHIFT 16L #define MM_PAGE_FRAME_NUMBER_SIZE 20 + +// +// User space range limit +// +#define MI_HIGHEST_USER_ADDRESS (PVOID)0x7FFEFFFF
// // Address of the shared user page
Modified: trunk/reactos/lib/rtl/image.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/lib/rtl/image.c?rev=67140&a... ============================================================================== --- trunk/reactos/lib/rtl/image.c [iso-8859-1] (original) +++ trunk/reactos/lib/rtl/image.c [iso-8859-1] Fri Apr 10 19:21:22 2015 @@ -137,32 +137,42 @@ */ NTSTATUS NTAPI -RtlImageNtHeaderEx(IN ULONG Flags, - IN PVOID Base, - IN ULONG64 Size, - OUT PIMAGE_NT_HEADERS *OutHeaders) +RtlImageNtHeaderEx( + _In_ ULONG Flags, + _In_ PVOID Base, + _In_ ULONG64 Size, + _Out_ PIMAGE_NT_HEADERS *OutHeaders) { PIMAGE_NT_HEADERS NtHeaders; PIMAGE_DOS_HEADER DosHeader; BOOLEAN WantsRangeCheck; + ULONG NtHeaderOffset;
/* You must want NT Headers, no? */ - if (!OutHeaders) return STATUS_INVALID_PARAMETER; + if (OutHeaders == NULL) + { + DPRINT1("OutHeaders is NULL\n"); + return STATUS_INVALID_PARAMETER; + }
/* Assume failure */ *OutHeaders = NULL;
/* Validate Flags */ - if (Flags &~ RTL_IMAGE_NT_HEADER_EX_FLAG_NO_RANGE_CHECK) - { - DPRINT1("Invalid flag combination... check for new API flags?\n"); + if (Flags & ~RTL_IMAGE_NT_HEADER_EX_FLAG_NO_RANGE_CHECK) + { + DPRINT1("Invalid flags: 0x%lx\n", Flags); return STATUS_INVALID_PARAMETER; }
/* Validate base */ - if (!(Base) || (Base == (PVOID)-1)) return STATUS_INVALID_PARAMETER; - - /* Check if the caller wants validation */ + if ((Base == NULL) || (Base == (PVOID)-1)) + { + DPRINT1("Invalid base address: %p\n", Base); + return STATUS_INVALID_PARAMETER; + } + + /* Check if the caller wants range checks */ WantsRangeCheck = !(Flags & RTL_IMAGE_NT_HEADER_EX_FLAG_NO_RANGE_CHECK); if (WantsRangeCheck) { @@ -179,56 +189,56 @@ if (DosHeader->e_magic != IMAGE_DOS_SIGNATURE) { /* Not a valid COFF */ - DPRINT1("Not an MZ file\n"); + DPRINT1("Invalid image DOS signature!\n"); + return STATUS_INVALID_IMAGE_FORMAT; + } + + /* Get the offset to the NT headers (and copy from LONG to ULONG) */ + NtHeaderOffset = DosHeader->e_lfanew; + + /* The offset must not be larger than 256MB, as a hard-coded check. + In Windows this check is only done in user mode, not in kernel mode, + but it shouldn't harm to have it anyway. Note that without this check, + other overflow checks would become necessary! */ + if (NtHeaderOffset >= (256 * 1024 * 1024)) + { + /* Fail */ + DPRINT1("NT headers offset is larger than 256MB!\n"); return STATUS_INVALID_IMAGE_FORMAT; }
/* Check if the caller wants validation */ if (WantsRangeCheck) { - /* The offset should fit in the passsed-in size */ - if (DosHeader->e_lfanew >= Size) + /* Make sure the file header fits into the size */ + if ((NtHeaderOffset + + RTL_SIZEOF_THROUGH_FIELD(IMAGE_NT_HEADERS, FileHeader)) >= Size) { /* Fail */ - DPRINT1("e_lfanew is larger than PE file\n"); + DPRINT1("NT headers beyond image size!\n"); return STATUS_INVALID_IMAGE_FORMAT; } - - /* It shouldn't be past 4GB either */ - if (DosHeader->e_lfanew >= - (MAXULONG - sizeof(IMAGE_DOS_SIGNATURE) - sizeof(IMAGE_FILE_HEADER))) - { - /* Fail */ - DPRINT1("e_lfanew is larger than 4GB\n"); + } + + /* Now get a pointer to the NT Headers */ + NtHeaders = (PIMAGE_NT_HEADERS)((ULONG_PTR)Base + NtHeaderOffset); + + /* Check if the mapping is in user space */ + if (Base <= MmHighestUserAddress) + { + /* Make sure we don't overflow into kernel space */ + if ((PVOID)(NtHeaders + 1) > MmHighestUserAddress) + { + DPRINT1("Image overflows from user space into kernel space!\n"); return STATUS_INVALID_IMAGE_FORMAT; } - - /* And the whole file shouldn't overflow past 4GB */ - if ((DosHeader->e_lfanew + - sizeof(IMAGE_DOS_SIGNATURE) - sizeof(IMAGE_FILE_HEADER)) >= Size) - { - /* Fail */ - DPRINT1("PE is larger than 4GB\n"); - return STATUS_INVALID_IMAGE_FORMAT; - } - } - - /* The offset also can't be larger than 256MB, as a hard-coded check */ - if (DosHeader->e_lfanew >= (256 * 1024 * 1024)) - { - /* Fail */ - DPRINT1("PE offset is larger than 256MB\n"); - return STATUS_INVALID_IMAGE_FORMAT; - } - - /* Now it's safe to get the NT Headers */ - NtHeaders = (PIMAGE_NT_HEADERS)((ULONG_PTR)Base + DosHeader->e_lfanew); + }
/* Verify the PE Signature */ if (NtHeaders->Signature != IMAGE_NT_SIGNATURE) { /* Fail */ - DPRINT1("PE signature missing\n"); + DPRINT1("Invalid image NT signature!\n"); return STATUS_INVALID_IMAGE_FORMAT; }
@@ -245,6 +255,10 @@ RtlImageNtHeader(IN PVOID Base) { PIMAGE_NT_HEADERS NtHeader; + + ULONG c = 1; + ULONG s = FIELD_OFFSET(IMAGE_OPTIONAL_HEADER32, DataDirectory[c]); + (void)s;
/* Call the new API */ RtlImageNtHeaderEx(RTL_IMAGE_NT_HEADER_EX_FLAG_NO_RANGE_CHECK,
Modified: trunk/reactos/lib/rtl/rtlp.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/lib/rtl/rtlp.h?rev=67140&am... ============================================================================== --- trunk/reactos/lib/rtl/rtlp.h [iso-8859-1] (original) +++ trunk/reactos/lib/rtl/rtlp.h [iso-8859-1] Fri Apr 10 19:21:22 2015 @@ -34,6 +34,8 @@
#define RVA(m, b) ((PVOID)((ULONG_PTR)(b) + (ULONG_PTR)(m)))
+extern PVOID MmHighestUserAddress; + NTSTATUS NTAPI RtlpSafeCopyMemory(
Modified: trunk/reactos/ntoskrnl/include/internal/amd64/mm.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/a... ============================================================================== --- trunk/reactos/ntoskrnl/include/internal/amd64/mm.h [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/include/internal/amd64/mm.h [iso-8859-1] Fri Apr 10 19:21:22 2015 @@ -5,7 +5,6 @@
/* Memory layout base addresses */ #define MI_LOWEST_VAD_ADDRESS (PVOID)0x000000007FF00000ULL -#define MI_HIGHEST_USER_ADDRESS (PVOID)0x000007FFFFFEFFFFULL #define MI_USER_PROBE_ADDRESS (PVOID)0x000007FFFFFF0000ULL #define MI_DEFAULT_SYSTEM_RANGE_START (PVOID)0xFFFF080000000000ULL #define MI_REAL_SYSTEM_RANGE_START 0xFFFF800000000000ULL
Modified: trunk/reactos/ntoskrnl/mm/ARM3/miarm.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/miarm.h?re... ============================================================================== --- trunk/reactos/ntoskrnl/mm/ARM3/miarm.h [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/mm/ARM3/miarm.h [iso-8859-1] Fri Apr 10 19:21:22 2015 @@ -29,7 +29,6 @@
#define MI_SYSTEM_VIEW_SIZE (32 * _1MB)
-#define MI_HIGHEST_USER_ADDRESS (PVOID)0x7FFEFFFF #define MI_USER_PROBE_ADDRESS (PVOID)0x7FFF0000 #define MI_DEFAULT_SYSTEM_RANGE_START (PVOID)0x80000000 #define MI_SYSTEM_CACHE_WS_START (PVOID)0xC0C00000
-
tkreuzer＠svn.reactos.org