[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

svn path=/trunk/; revision=67140

diff --git a/reactos/boot/freeldr/freeldr/lib/rtl/libsupp.c b/reactos/boot/freeldr/freeldr/lib/rtl/libsupp.c
index dcc19b2..eb9bc2a 100644 (file)
--- a/reactos/boot/freeldr/freeldr/lib/rtl/libsupp.c
+++ b/reactos/boot/freeldr/freeldr/lib/rtl/libsupp.c
@@ -20,6 +20,8 @@
 
 #include <freeldr.h>
 
+PVOID MmHighestUserAddress = (PVOID)MI_HIGHEST_USER_ADDRESS;
+
 #if DBG
 VOID FASTCALL
 CHECK_PAGED_CODE_RTL(char *file, int line)

diff --git a/reactos/dll/ntdll/rtl/libsupp.c b/reactos/dll/ntdll/rtl/libsupp.c
index 43276ae..81daf81 100644 (file)
--- a/reactos/dll/ntdll/rtl/libsupp.c
+++ b/reactos/dll/ntdll/rtl/libsupp.c
@@ -16,6 +16,7 @@
 
 SIZE_T RtlpAllocDeallocQueryBufferSize = PAGE_SIZE;
 PTEB LdrpTopLevelDllBeingLoadedTeb = NULL;
+PVOID MmHighestUserAddress = (PVOID)MI_HIGHEST_USER_ADDRESS;
 
 /* FUNCTIONS ***************************************************************/
 

diff --git a/reactos/include/ndk/amd64/mmtypes.h b/reactos/include/ndk/amd64/mmtypes.h
index eb28217..49c7b63 100644 (file)
--- a/reactos/include/ndk/amd64/mmtypes.h
+++ b/reactos/include/ndk/amd64/mmtypes.h
@@ -37,6 +37,11 @@ extern "C" {
 #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
 //

diff --git a/reactos/include/ndk/i386/mmtypes.h b/reactos/include/ndk/i386/mmtypes.h
index cd8185c..cea2b95 100644 (file)
--- a/reactos/include/ndk/i386/mmtypes.h
+++ b/reactos/include/ndk/i386/mmtypes.h
@@ -36,6 +36,11 @@ extern "C" {
 #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
 //

diff --git a/reactos/lib/rtl/image.c b/reactos/lib/rtl/image.c
index a7f7c3e..1fcae1d 100644 (file)
--- a/reactos/lib/rtl/image.c
+++ b/reactos/lib/rtl/image.c
@@ -137,32 +137,42 @@ LdrVerifyMappedImageMatchesChecksum(
  */
 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)
+    if (Flags & ~RTL_IMAGE_NT_HEADER_EX_FLAG_NO_RANGE_CHECK)
     {
-        DPRINT1("Invalid flag combination... check for new API flags?\n");
+        DPRINT1("Invalid flags: 0x%lx\n", Flags);
         return STATUS_INVALID_PARAMETER;
     }
 
     /* Validate base */
-    if (!(Base) || (Base == (PVOID)-1)) return STATUS_INVALID_PARAMETER;
+    if ((Base == NULL) || (Base == (PVOID)-1))
+    {
+        DPRINT1("Invalid base address: %p\n", Base);
+        return STATUS_INVALID_PARAMETER;
+    }
 
-    /* Check if the caller wants validation */
+    /* Check if the caller wants range checks */
     WantsRangeCheck = !(Flags & RTL_IMAGE_NT_HEADER_EX_FLAG_NO_RANGE_CHECK);
     if (WantsRangeCheck)
     {
@@ -179,56 +189,56 @@ RtlImageNtHeaderEx(IN ULONG Flags,
     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");
-            return STATUS_INVALID_IMAGE_FORMAT;
-        }
+    /* Now get a pointer to the NT Headers */
+    NtHeaders = (PIMAGE_NT_HEADERS)((ULONG_PTR)Base + NtHeaderOffset);
 
-        /* And the whole file shouldn't overflow past 4GB */
-        if ((DosHeader->e_lfanew +
-            sizeof(IMAGE_DOS_SIGNATURE) - sizeof(IMAGE_FILE_HEADER)) >= Size)
+    /* 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)
         {
-            /* Fail */
-            DPRINT1("PE is larger than 4GB\n");
+            DPRINT1("Image overflows from user space into kernel space!\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;
     }
 
@@ -246,6 +256,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,
                        Base,

diff --git a/reactos/lib/rtl/rtlp.h b/reactos/lib/rtl/rtlp.h
index a57e78b..47ae135 100644 (file)
--- a/reactos/lib/rtl/rtlp.h
+++ b/reactos/lib/rtl/rtlp.h
@@ -34,6 +34,8 @@ extern VOID FASTCALL CHECK_PAGED_CODE_RTL(char *file, int line);
 
 #define RVA(m, b) ((PVOID)((ULONG_PTR)(b) + (ULONG_PTR)(m)))
 
+extern PVOID MmHighestUserAddress;
+
 NTSTATUS
 NTAPI
 RtlpSafeCopyMemory(

diff --git a/reactos/ntoskrnl/include/internal/amd64/mm.h b/reactos/ntoskrnl/include/internal/amd64/mm.h
index 661955c..e78c3ec 100644 (file)
--- a/reactos/ntoskrnl/include/internal/amd64/mm.h
+++ b/reactos/ntoskrnl/include/internal/amd64/mm.h
@@ -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

diff --git a/reactos/ntoskrnl/mm/ARM3/miarm.h b/reactos/ntoskrnl/mm/ARM3/miarm.h
index 524d062..8b62f1c 100644 (file)
--- a/reactos/ntoskrnl/mm/ARM3/miarm.h
+++ b/reactos/ntoskrnl/mm/ARM3/miarm.h
@@ -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