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 <[EMAIL PROTECTED]>: > 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@lists.ximian.com > http://lists.ximian.com/mailman/listinfo/mono-devel-list > > _______________________________________________ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list