[mjansen] 71290: [APPHELP] Addendum to r71226, fix some failing testcases of SdbGetFileAttributes + make it more robust. CORE-10367 - Work around a ROS bug (CORE-11206) by doing some better bounds ...

Author: mjansen
Date: Sun May  8 20:43:59 2016
New Revision: 71290

URL: http://svn.reactos.org/svn/reactos?rev=71290&view=rev
Log:
[APPHELP] Addendum to r71226, fix some failing testcases of SdbGetFileAttributes + make it
more robust. CORE-10367
- Work around a ROS bug (CORE-11206) by doing some better bounds checking.
- Silence a VAD warning by checking for NULL instead of blindly passing mem to
NtUnmapViewOfSection
- Rewrite module type lookup code to use an enum and to return the found header.

Modified:
    trunk/reactos/dll/appcompat/apphelp/sdbapi.c
    trunk/reactos/dll/appcompat/apphelp/sdbfileattr.c

Modified: trunk/reactos/dll/appcompat/apphelp/sdbapi.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/dll/appcompat/apphelp/sdba…
==============================================================================
--- trunk/reactos/dll/appcompat/apphelp/sdbapi.c	[iso-8859-1] (original)
+++ trunk/reactos/dll/appcompat/apphelp/sdbapi.c	[iso-8859-1] Sun May  8 20:43:59 2016
@@ -284,7 +284,9 @@
 
 void WINAPI SdbpCloseMemMappedFile(PMEMMAPPED mapping)
 {
-    NtUnmapViewOfSection(NtCurrentProcess(), mapping->view);
+    /* Prevent a VAD warning */
+    if (mapping->view)
+        NtUnmapViewOfSection(NtCurrentProcess(), mapping->view);
     NtClose(mapping->section);
     NtClose(mapping->file);
     RtlZeroMemory(mapping, sizeof(*mapping));

Modified: trunk/reactos/dll/appcompat/apphelp/sdbfileattr.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/dll/appcompat/apphelp/sdbf…
==============================================================================
--- trunk/reactos/dll/appcompat/apphelp/sdbfileattr.c	[iso-8859-1] (original)
+++ trunk/reactos/dll/appcompat/apphelp/sdbfileattr.c	[iso-8859-1] Sun May  8 20:43:59
2016
@@ -27,6 +27,14 @@
 #include "wine/unicode.h"
 
 #define NUM_ATTRIBUTES  28
+enum APPHELP_MODULETYPE
+{
+    MODTYPE_UNKNOWN = 0,
+    MODTYPE_DOS = 1,
+    MODTYPE_NE = 2,
+    MODTYPE_PE = 3,
+};
+
 
 static void WINAPI SdbpSetDWORDAttr(PATTRINFO attr, TAG tag, DWORD value)
 {
@@ -176,25 +184,33 @@
     return checks;
 }
 
-static DWORD WINAPI SdbpGetModuleType(PMEMMAPPED mapping)
+static DWORD WINAPI SdbpGetModuleType(PMEMMAPPED mapping, PIMAGE_NT_HEADERS* nt_headers)
 {
     PIMAGE_DOS_HEADER dos = (PIMAGE_DOS_HEADER)mapping->view;
     PIMAGE_OS2_HEADER os2;
 
+    *nt_headers = NULL;
+
     if (mapping->size < 2 || dos->e_magic != IMAGE_DOS_SIGNATURE)
-        return 0;
+        return MODTYPE_UNKNOWN;
 
     if (mapping->size < sizeof(IMAGE_DOS_HEADER) || mapping->size <
(dos->e_lfanew+2))
-        return 1;
+        return MODTYPE_DOS;
 
     os2 = (PIMAGE_OS2_HEADER)((PBYTE)dos + dos->e_lfanew);
     if (os2->ne_magic == IMAGE_OS2_SIGNATURE || os2->ne_magic ==
IMAGE_OS2_SIGNATURE_LE)
-        return 2;
+    {
+        *nt_headers = (PIMAGE_NT_HEADERS)os2;
+        return MODTYPE_NE;
+    }
 
     if (mapping->size >= (dos->e_lfanew + 4) &&
((PIMAGE_NT_HEADERS)os2)->Signature == IMAGE_NT_SIGNATURE)
-        return 3;
-
-    return 1;
+    {
+        *nt_headers = (PIMAGE_NT_HEADERS)os2;
+        return MODTYPE_PE;
+    }
+
+    return MODTYPE_DOS;
 }
 
 /**
@@ -246,8 +262,9 @@
 
     PIMAGE_NT_HEADERS headers;
     MEMMAPPED mapped;
+    PBYTE mapping_end;
     PVOID file_info = 0;
-    DWORD headersum, checksum, module_type;
+    DWORD module_type;
     WCHAR translation[128] = {0};
     PATTRINFO attr_info;
 
@@ -261,6 +278,7 @@
         SHIM_ERR("Error retrieving FILEINFO structure\n");
         return FALSE;
     }
+    mapping_end = mapped.view + mapped.size;
 
     attr_info = (PATTRINFO)SdbAlloc(NUM_ATTRIBUTES * sizeof(ATTRINFO));
 
@@ -269,15 +287,14 @@
         SdbpSetDWORDAttr(&attr_info[1], TAG_CHECKSUM,
SdbpCalculateFileChecksum(&mapped));
     else
         SdbpSetAttrFail(&attr_info[1]);
-    module_type = SdbpGetModuleType(&mapped);
-
-    if (module_type)
+    module_type = SdbpGetModuleType(&mapped, &headers);
+
+    if (module_type != MODTYPE_UNKNOWN)
         SdbpSetDWORDAttr(&attr_info[16], TAG_MODULE_TYPE, module_type);
     else
         SdbpSetAttrFail(&attr_info[16]); /* TAG_MODULE_TYPE */
 
-    headers = CheckSumMappedFile(mapped.view, mapped.size, &headersum,
&checksum);
-    if (headers)
+    if (headers && module_type == MODTYPE_PE && ((PBYTE)(headers+1) <=
mapping_end))
     {
         DWORD info_size;
         SIZE_T export_dir_size;
@@ -318,7 +335,7 @@
         SdbpSetDWORDAttr(&attr_info[24], TAG_UPTO_LINK_DATE,
headers->FileHeader.TimeDateStamp);
 
         export_dir = (PIMAGE_EXPORT_DIRECTORY)ImageDirectoryEntryToData(mapped.view,
FALSE, IMAGE_DIRECTORY_ENTRY_EXPORT, &export_dir_size);
-        if (export_dir)
+        if (export_dir && ((PBYTE)(export_dir+1) <= mapping_end))
         {
             PIMAGE_SECTION_HEADER section = NULL;
             PBYTE export_name = ImageRvaToVa(headers, mapped.view, export_dir->Name,
&section);
@@ -345,18 +362,17 @@
             if (n != 16 && n != 26)
                 SdbpSetAttrFail(&attr_info[n]);
         }
-        if (module_type == 2)
-        {
-            PIMAGE_DOS_HEADER dos = (PIMAGE_DOS_HEADER)mapped.view;
-            PBYTE end = mapped.view + mapped.size, ptr;
-            PIMAGE_OS2_HEADER os2 = (PIMAGE_OS2_HEADER)((PBYTE)dos + dos->e_lfanew);
-            if ((PBYTE)(os2 + 1) <= end)
+        if (module_type == MODTYPE_NE)
+        {
+            PBYTE ptr;
+            PIMAGE_OS2_HEADER os2 = (PIMAGE_OS2_HEADER)headers;
+            if ((PBYTE)(os2 + 1) <= mapping_end)
             {
-                ptr = (PBYTE)dos + os2->ne_nrestab;
-                if (ptr <= end && (ptr + 1 + *ptr) <= end)
+                ptr = mapped.view + os2->ne_nrestab;
+                if (ptr <= mapping_end && (ptr + 1 + *ptr) <= mapping_end)
                     SdbpSetStringAttrFromPascalString(&attr_info[19],
TAG_16BIT_DESCRIPTION, ptr);
                 ptr = (PBYTE)os2 + os2->ne_restab;
-                if (ptr <= end && (ptr + 1 + *ptr) <= end)
+                if (ptr <= mapping_end && (ptr + 1 + *ptr) <= mapping_end)
                     SdbpSetStringAttrFromPascalString(&attr_info[20],
TAG_16BIT_MODULE_NAME, ptr);
             }
         }