Bug 1702717 - Skip blocklisting if the mapped region is not executable. r=mhowell, a=jcristau
authorToshihito Kikuchi <tkikuchi@mozilla.com>
Fri, 11 Jun 2021 22:45:46 +0000
changeset 649729 ab0390e77028bed915a8062051f2eb95ff82a9f0
parent 649728 16ba7f3db4dfbf33d3a75135a1ac4597175da2e0
child 649730 0791b692395a6cfffe9c0d1403cd1d08270c8fcd
push id15551
push userjcristau@mozilla.com
push dateThu, 17 Jun 2021 10:25:54 +0000
treeherdermozilla-beta@b50708896f2d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmhowell, jcristau
bugs1702717
milestone90.0
Bug 1702717 - Skip blocklisting if the mapped region is not executable. r=mhowell, a=jcristau When a module is loaded with `LOAD_LIBRARY_AS_IMAGE_RESOURCE`, the mapped region is `MEM_IMAGE`, but it's not executable. We don't have to check the blocklist in such a case. Differential Revision: https://phabricator.services.mozilla.com/D117573
browser/app/winlauncher/freestanding/DllBlocklist.cpp
mozglue/tests/gtest/TestDLLBlocklist.cpp
--- a/browser/app/winlauncher/freestanding/DllBlocklist.cpp
+++ b/browser/app/winlauncher/freestanding/DllBlocklist.cpp
@@ -384,18 +384,24 @@ NTSTATUS NTAPI patched_NtMapViewOfSectio
   NTSTATUS ntStatus =
       ::NtQueryVirtualMemory(aProcess, *aBaseAddress, MemoryBasicInformation,
                              &mbi, sizeof(mbi), nullptr);
   if (!NT_SUCCESS(ntStatus)) {
     ::NtUnmapViewOfSection(aProcess, *aBaseAddress);
     return STATUS_ACCESS_DENIED;
   }
 
-  // We don't care about mappings that aren't MEM_IMAGE
-  if (!(mbi.Type & MEM_IMAGE)) {
+  // We don't care about mappings that aren't MEM_IMAGE or executable.
+  // We check for the AllocationProtect, not the Protect field because
+  // the first section of a mapped image is always PAGE_READONLY even
+  // when it's mapped as an executable.
+  constexpr DWORD kPageExecutable = PAGE_EXECUTE | PAGE_EXECUTE_READ |
+                                    PAGE_EXECUTE_READWRITE |
+                                    PAGE_EXECUTE_WRITECOPY;
+  if (!(mbi.Type & MEM_IMAGE) || !(mbi.AllocationProtect & kPageExecutable)) {
     return stubStatus;
   }
 
   // Get the section name
   nt::MemorySectionNameBuf sectionFileName(
       gLoaderPrivateAPI.GetSectionNameBuffer(*aBaseAddress));
   if (sectionFileName.IsEmpty()) {
     ::NtUnmapViewOfSection(aProcess, *aBaseAddress);
--- a/mozglue/tests/gtest/TestDLLBlocklist.cpp
+++ b/mozglue/tests/gtest/TestDLLBlocklist.cpp
@@ -41,27 +41,36 @@ TEST(TestDllBlocklist, BlockDllByName)
   // is case-insensitive.
   constexpr auto kLeafName = u"TestDllBlocklist_MatchByName.dll"_ns;
   nsString dllPath = GetFullPath(kLeafName);
 
   nsModuleHandle hDll(::LoadLibraryW(dllPath.get()));
 
   EXPECT_TRUE(!hDll);
   EXPECT_TRUE(!::GetModuleHandleW(kLeafName.get()));
+
+  hDll.own(::LoadLibraryExW(dllPath.get(), nullptr, LOAD_LIBRARY_AS_DATAFILE));
+  // Mapped as MEM_MAPPED + PAGE_READONLY
+  EXPECT_TRUE(hDll);
 }
 
 TEST(TestDllBlocklist, BlockDllByVersion)
 {
   constexpr auto kLeafName = u"TestDllBlocklist_MatchByVersion.dll"_ns;
   nsString dllPath = GetFullPath(kLeafName);
 
   nsModuleHandle hDll(::LoadLibraryW(dllPath.get()));
 
   EXPECT_TRUE(!hDll);
   EXPECT_TRUE(!::GetModuleHandleW(kLeafName.get()));
+
+  hDll.own(
+      ::LoadLibraryExW(dllPath.get(), nullptr, LOAD_LIBRARY_AS_IMAGE_RESOURCE));
+  // Mapped as MEM_IMAGE + PAGE_READONLY
+  EXPECT_TRUE(hDll);
 }
 
 TEST(TestDllBlocklist, AllowDllByVersion)
 {
   constexpr auto kLeafName = u"TestDllBlocklist_AllowByVersion.dll"_ns;
   nsString dllPath = GetFullPath(kLeafName);
 
   nsModuleHandle hDll(::LoadLibraryW(dllPath.get()));