Fix handling of incorrect assemblies on Unix (#16747)

* Return DPTR from PEDecoder::FindFirstSection()

Change type of the function's return value
to PTR_IMAGE_SECTION_HEADER instead of (IMAGE_SECTION_HEADER *)

* Fix handling of incorrect assemblies on Unix

This fixes the regression that was introduced by #10772 and is
caused by a missing check for validity of loaded assembly file.

Related issue: #15544

Author: ruben-ayrapetyan

src/debug/daccess/nidump.cpp

@@ -720,7 +720,7 @@ NativeImageDumper::DumpNativeImage()
 
     for (COUNT_T i = 0; i < m_decoder.GetNumberOfSections(); i++)
     {
-        PTR_IMAGE_SECTION_HEADER section = dptr_add(m_decoder.FindFirstSection(), i);
+        PTR_IMAGE_SECTION_HEADER section = m_decoder.FindFirstSection() + i;
         m_display->Section(reinterpret_cast<char *>(section->Name),
                            section->VirtualAddress,
                            section->SizeOfRawData);

src/inc/pedecoder.h

@@ -182,7 +182,7 @@ class PEDecoder
     UINT32 GetWin32VersionValue() const;
     COUNT_T GetNumberOfRvaAndSizes() const;
     COUNT_T GetNumberOfSections() const;
-    IMAGE_SECTION_HEADER *FindFirstSection() const;
+    PTR_IMAGE_SECTION_HEADER FindFirstSection() const;
     IMAGE_SECTION_HEADER *FindSection(LPCSTR sectionName) const;
 
     DWORD GetImageIdentity() const;

src/inc/pedecoder.inl

@@ -1178,7 +1178,7 @@ inline DWORD PEDecoder::GetImageIdentity() const
 }
 
 
-inline IMAGE_SECTION_HEADER *PEDecoder::FindFirstSection() const
+inline PTR_IMAGE_SECTION_HEADER PEDecoder::FindFirstSection() const
 {
     CONTRACT(IMAGE_SECTION_HEADER *)
     {

src/utilcode/pedecoder.cpp

@@ -445,6 +445,7 @@ BOOL PEDecoder::HasWriteableSections() const
     CONTRACT_CHECK
     {
         INSTANCE_CHECK;
+        PRECONDITION(CheckNTHeaders());
         PRECONDITION(CheckFormat());
         NOTHROW;
         GC_NOTRIGGER;
@@ -453,7 +454,7 @@ BOOL PEDecoder::HasWriteableSections() const
     }
     CONTRACT_CHECK_END;
 
-    PTR_IMAGE_SECTION_HEADER pSection = FindFirstSection(FindNTHeaders());
+    PTR_IMAGE_SECTION_HEADER pSection = FindFirstSection();
     _ASSERTE(pSection != NULL);
 
     PTR_IMAGE_SECTION_HEADER pSectionEnd = pSection + VAL16(FindNTHeaders()->FileHeader.NumberOfSections);

src/vm/peimage.cpp

@@ -1029,7 +1029,9 @@ PTR_PEImageLayout PEImage::CreateLayoutFlat(BOOL bPermitWriteableSections)
 
     PTR_PEImageLayout pFlatLayout = PEImageLayout::LoadFlat(GetFileHandle(),this);
 
-    if (!bPermitWriteableSections && pFlatLayout->HasWriteableSections())
+    if (!bPermitWriteableSections
+        && pFlatLayout->CheckNTHeaders()
+        && pFlatLayout->HasWriteableSections())
     {
         pFlatLayout->Release();
 
@@ -1114,8 +1116,7 @@ void PEImage::Load()
 
 #ifdef PLATFORM_UNIX
     if (m_pLayouts[IMAGE_FLAT] != NULL
-        && m_pLayouts[IMAGE_FLAT]->CheckFormat()
-        && m_pLayouts[IMAGE_FLAT]->IsILOnly()
+        && m_pLayouts[IMAGE_FLAT]->CheckILOnlyFormat()
         && !m_pLayouts[IMAGE_FLAT]->HasWriteableSections())
     {
         // IL-only images with writeable sections are mapped in general way,

tests/src/Loader/regressions/GitHub_15544/main.cs

@@ -0,0 +1,37 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+//
+
+using System;
+using System.IO;
+using System.Reflection;
+
+public class CMain{
+    public static int Main(String[] args) {
+        string tempFileName = Path.GetTempFileName();
+
+        bool isThrown = false;
+
+        try
+        {
+            AssemblyName.GetAssemblyName(tempFileName);
+        }
+        catch (BadImageFormatException)
+        {
+            isThrown = true;
+        }
+
+        File.Delete(tempFileName);
+
+        if (isThrown) {
+            Console.WriteLine("PASS");
+
+            return 100;
+        } else {
+            Console.WriteLine("FAIL");
+
+            return 101;
+        }
+    }
+}

tests/src/Loader/regressions/GitHub_15544/main.csproj

@@ -0,0 +1,31 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+  <PropertyGroup>
+    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <SchemaVersion>2.0</SchemaVersion>
+    <ProjectGuid>{AC75380E-F196-4F32-9BCF-F0589AF864E6}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+  </PropertyGroup>
+  <!-- Default configurations to help VS understand the configurations -->
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
+  </PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
+  </PropertyGroup>
+  <ItemGroup>
+    <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
+      <Visible>False</Visible>
+    </CodeAnalysisDependentAssemblyPaths>
+  </ItemGroup>
+  <ItemGroup>
+    <Compile Include="main.cs" />
+  </ItemGroup>
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+</Project>