From 383736b96b643ba46ad290fc86601fc2d62a9436 Mon Sep 17 00:00:00 2001
From: Ruben Ayrapetyan <ruben-ayrapetyan@users.noreply.github.com>
Date: Tue, 6 Mar 2018 06:37:43 +0000
Subject: 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
---
 src/debug/daccess/nidump.cpp | 2 +-
 src/inc/pedecoder.h          | 2 +-
 src/inc/pedecoder.inl        | 2 +-
 src/utilcode/pedecoder.cpp   | 3 ++-
 src/vm/peimage.cpp           | 7 ++++---
 5 files changed, 9 insertions(+), 7 deletions(-)

(limited to 'src')

diff --git a/src/debug/daccess/nidump.cpp b/src/debug/daccess/nidump.cpp
index 45320dca7f..e948687c0e 100644
--- a/src/debug/daccess/nidump.cpp
+++ b/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);
diff --git a/src/inc/pedecoder.h b/src/inc/pedecoder.h
index 01375e6d88..8163ffff35 100644
--- a/src/inc/pedecoder.h
+++ b/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;
diff --git a/src/inc/pedecoder.inl b/src/inc/pedecoder.inl
index 7f3c79ba59..c8400af46a 100644
--- a/src/inc/pedecoder.inl
+++ b/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 *)
     {
diff --git a/src/utilcode/pedecoder.cpp b/src/utilcode/pedecoder.cpp
index ddd65d390d..e5e8ddce67 100644
--- a/src/utilcode/pedecoder.cpp
+++ b/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);
diff --git a/src/vm/peimage.cpp b/src/vm/peimage.cpp
index 3536b08361..83989f0e4d 100644
--- a/src/vm/peimage.cpp
+++ b/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,
-- 
cgit v1.2.3