[Mono-dev] [PATCH] Mixed-mode: Ensure that the image is managed before calling LoadLibrary

[Kornél Pál]

OK, thank you very much.

Kornél

Zoltan Varga wrote:
> Hi,
> 
>   This is ok to check in. Changes to coreee.{h,c} and to the windows
> specific parts of
> image.c you wrote do not need a review.
> 
>                        Zoltan
> 
> 2008/6/29 Kornél Pál <kornelpal at gmail.com>:
>> Hi,
>>
>> Currently LoadLibrary is called on the file that is possibly a CLI image.
>> This patch adds a check to ensure that the image is really a CLI image
>> before calling LoadLibrary.
>>
>> _CorValidateImage is only called for CLI images so this extra check is
>> required to ensure security.
>>
>> Please review and approve the patch.
>>
>> Thanks.
>>
>> Kornél
>>
>> Index: mono/mono/metadata/image.c
>> ===================================================================
>> --- mono/mono/metadata/image.c  (revision 106858)
>> +++ mono/mono/metadata/image.c  (working copy)
>> @@ -1130,7 +1130,7 @@
>>                                /* Increment reference count on images loaded
>> outside of the runtime. */
>>                                fname_utf16 = g_utf8_to_utf16 (absfname, -1,
>> NULL, NULL, NULL);
>>                                module_handle = LoadLibrary (fname_utf16);
>> -                               g_assert (module_handle != NULL);
>> +                               g_assert (module_handle == (HMODULE)
>> image->raw_data);
>>                        }
>>                        mono_image_addref (image);
>>                        mono_images_unlock ();
>> @@ -1141,7 +1141,7 @@
>>                }
>>
>>                fname_utf16 = g_utf8_to_utf16 (absfname, -1, NULL, NULL,
>> NULL);
>> -               module_handle = LoadLibrary (fname_utf16);
>> +               module_handle = MonoLoadImage (fname_utf16);
>>                if (status && module_handle == NULL)
>>                        last_error = GetLastError ();
>>
>> Index: mono/mono/metadata/coree.c
>> ===================================================================
>> --- mono/mono/metadata/coree.c  (revision 106858)
>> +++ mono/mono/metadata/coree.c  (working copy)
>> @@ -301,8 +301,8 @@
>>
>>  NtHeaders64->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress
>> =
>> NtHeaders32->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress;
>>
>>  NtHeaders64->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_BASERELOC].Size
>> =
>> NtHeaders32->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_BASERELOC].Size;
>>
>>  NtHeaders64->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_BASERELOC].VirtualAddress
>> =
>> NtHeaders32->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_BASERELOC].VirtualAddress;
>> -
>> NtHeaders64->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_SECURITY].Size
>> = 0;
>> -
>> NtHeaders64->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_SECURITY].VirtualAddress
>> = 0;
>> +
>> NtHeaders64->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_SECURITY].Size
>> =
>> NtHeaders32->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_SECURITY].Size;
>> +
>> NtHeaders64->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_SECURITY].VirtualAddress
>> =
>> NtHeaders32->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_SECURITY].VirtualAddress;
>>
>>  NtHeaders64->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_EXCEPTION].Size
>> = 0;
>>
>>  NtHeaders64->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_EXCEPTION].VirtualAddress
>> = 0;
>>
>>  NtHeaders64->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_RESOURCE].Size
>> =
>> NtHeaders32->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_RESOURCE].Size;
>> @@ -344,6 +344,9 @@
>>        if (NtHeaders32->OptionalHeader.Magic !=
>> IMAGE_NT_OPTIONAL_HDR32_MAGIC)
>>                return STATUS_INVALID_IMAGE_FORMAT;
>>
>> +       if (NtHeaders32->OptionalHeader.NumberOfRvaAndSizes <=
>> IMAGE_DIRECTORY_ENTRY_COM_DESCRIPTOR)
>> +               return STATUS_INVALID_IMAGE_FORMAT;
>> +
>>        CliHeaderDir =
>> &NtHeaders32->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_COM_DESCRIPTOR];
>>        if (!CliHeaderDir->VirtualAddress)
>>                return STATUS_INVALID_IMAGE_FORMAT;
>> @@ -382,6 +385,79 @@
>>        return CorBindToRuntimeEx (pwszVersion, pwszBuildFlavor, 0, rclsid,
>> riid, ppv);
>>  }
>>
>> +HMODULE WINAPI MonoLoadImage(LPCWSTR FileName)
>> +{
>> +       HANDLE FileHandle;
>> +       DWORD FileSize;
>> +       HANDLE MapHandle;
>> +       IMAGE_DOS_HEADER* DosHeader;
>> +       IMAGE_NT_HEADERS32* NtHeaders32;
>> +       IMAGE_NT_HEADERS64* NtHeaders64;
>> +       HMODULE ModuleHandle;
>> +
>> +       FileHandle = CreateFile(FileName, GENERIC_READ, FILE_SHARE_READ,
>> NULL, OPEN_EXISTING, 0, NULL);
>> +       if (FileHandle == INVALID_HANDLE_VALUE)
>> +               return NULL;
>> +
>> +       FileSize = GetFileSize(FileHandle, NULL);
>> +       if (FileSize == INVALID_FILE_SIZE)
>> +               goto CloseFile;
>> +
>> +       MapHandle = CreateFileMapping(FileHandle, NULL, PAGE_READONLY, 0, 0,
>> NULL);
>> +       if (MapHandle == NULL)
>> +               goto CloseFile;
>> +
>> +       DosHeader = (IMAGE_DOS_HEADER*)MapViewOfFile(MapHandle,
>> FILE_MAP_READ, 0, 0, 0);
>> +       if (DosHeader == NULL)
>> +               goto CloseMap;
>> +
>> +       if (FileSize < sizeof(IMAGE_DOS_HEADER) || DosHeader->e_magic !=
>> IMAGE_DOS_SIGNATURE || FileSize < DosHeader->e_lfanew +
>> sizeof(IMAGE_NT_HEADERS32))
>> +               goto InvalidImageFormat;
>> +
>> +       NtHeaders32 = (IMAGE_NT_HEADERS32*)((DWORD_PTR)DosHeader +
>> DosHeader->e_lfanew);
>> +       if (NtHeaders32->Signature != IMAGE_NT_SIGNATURE)
>> +               goto InvalidImageFormat;
>> +
>> +#ifdef _WIN64
>> +       NtHeaders64 = (IMAGE_NT_HEADERS64*)NtHeaders32;
>> +       if (NtHeaders64->OptionalHeader.Magic ==
>> IMAGE_NT_OPTIONAL_HDR64_MAGIC)
>> +       {
>> +               if (FileSize < DosHeader->e_lfanew +
>> sizeof(IMAGE_NT_HEADERS64) ||
>> +                       NtHeaders64->OptionalHeader.NumberOfRvaAndSizes <=
>> IMAGE_DIRECTORY_ENTRY_COM_DESCRIPTOR ||
>> +
>> !NtHeaders64->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_COM_DESCRIPTOR].VirtualAddress)
>> +                               goto InvalidImageFormat;
>> +
>> +               goto ValidImage;
>> +       }
>> +#endif
>> +
>> +       if (NtHeaders32->OptionalHeader.Magic !=
>> IMAGE_NT_OPTIONAL_HDR32_MAGIC ||
>> +               NtHeaders32->OptionalHeader.NumberOfRvaAndSizes <=
>> IMAGE_DIRECTORY_ENTRY_COM_DESCRIPTOR ||
>> +
>> !NtHeaders32->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_COM_DESCRIPTOR].VirtualAddress)
>> +       {
>> +InvalidImageFormat:
>> +               SetLastError(STATUS_INVALID_IMAGE_FORMAT);
>> +               goto UnmapView;
>> +       }
>> +
>> +ValidImage:
>> +       UnmapViewOfFile(DosHeader);
>> +       CloseHandle(MapHandle);
>> +
>> +       ModuleHandle = LoadLibrary(FileName);
>> +
>> +       CloseHandle(FileHandle);
>> +       return ModuleHandle;
>> +
>> +UnmapView:
>> +       UnmapViewOfFile(DosHeader);
>> +CloseMap:
>> +       CloseHandle(MapHandle);
>> +CloseFile:
>> +       CloseHandle(FileHandle);
>> +       return NULL;
>> +}
>> +
>>  typedef struct _EXPORT_FIXUP
>>  {
>>        LPCSTR Name;
>> Index: mono/mono/metadata/coree.h
>> ===================================================================
>> --- mono/mono/metadata/coree.h  (revision 106858)
>> +++ mono/mono/metadata/coree.h  (working copy)
>> @@ -28,6 +28,8 @@
>>
>>  extern HMODULE coree_module_handle MONO_INTERNAL;
>>
>> +HMODULE WINAPI MonoLoadImage(LPCWSTR FileName) MONO_INTERNAL;
>> +
>>  gchar* mono_get_module_file_name (HMODULE module_handle) MONO_INTERNAL;
>>  void mono_load_coree (const char* file_name) MONO_INTERNAL;
>>  void mono_fixup_exe_image (MonoImage* image) MONO_INTERNAL;
>>
>> _______________________________________________
>> Mono-devel-list mailing list
>> Mono-devel-list at lists.ximian.com
>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>
>>
>