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

[Zoltan Varga]

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
>
>