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