The Windows implementation of the API is different in user vs kernel-mode.
We should probably try to remain compatible.

Best regards,
Alex Ionescu

On Fri, Apr 10, 2015 at 12:21 PM, <tkreu...@svn.reactos.org> wrote:

> 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/rtl/libsupp.c?rev=67140&r1=67139&r2=67140&view=diff
>
> ==============================================================================
> --- 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=67140&r1=67139&r2=67140&view=diff
>
> ==============================================================================
> --- 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?rev=67140&r1=67139&r2=67140&view=diff
>
> ==============================================================================
> --- 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?rev=67140&r1=67139&r2=67140&view=diff
>
> ==============================================================================
> --- 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&r1=67139&r2=67140&view=diff
>
> ==============================================================================
> --- 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&r1=67139&r2=67140&view=diff
>
> ==============================================================================
> --- 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/amd64/mm.h?rev=67140&r1=67139&r2=67140&view=diff
>
> ==============================================================================
> --- 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?rev=67140&r1=67139&r2=67140&view=diff
>
> ==============================================================================
> --- 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
>
>
>
_______________________________________________
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev

Reply via email to