Bug 1550544: Clean up untrusted module evaluator; r=agashlin
authorAaron Klotz <aklotz@mozilla.com>
Wed, 15 May 2019 19:38:34 +0000
changeset 532822 50170a11ab589e28996b962da710acd2438bd8ad
parent 532821 e6d06a5ffa07fa63ae2aa747c6f6877029a370cf
child 532823 c496d42a69b48d22510821637c5494200b80aee0
push id11272
push userapavel@mozilla.com
push dateThu, 16 May 2019 15:28:22 +0000
treeherdermozilla-beta@2265bfc5920d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersagashlin
bugs1550544
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1550544: Clean up untrusted module evaluator; r=agashlin This patch takes care of two things: 1) It changes the module evaluator such that, if a binary is signed but the cert is neither Microsoft's nor ours, the binary is automatically disqualified. 2) General cleanup. Use nsIFile::Contains instead of StringBeginsWith for checking path containment. Better OO. Differential Revision: https://phabricator.services.mozilla.com/D30556
toolkit/xre/ModuleEvaluator_windows.cpp
toolkit/xre/ModuleEvaluator_windows.h
toolkit/xre/WinDllServices.cpp
--- a/toolkit/xre/ModuleEvaluator_windows.cpp
+++ b/toolkit/xre/ModuleEvaluator_windows.cpp
@@ -31,26 +31,86 @@ static bool GetDirectoryName(const nsCOM
     return false;
   }
   if (NS_FAILED(parentDir->GetPath(aParent))) {
     return false;
   }
   return true;
 }
 
+ModuleLoadEvent::ModuleInfo::ModuleInfo(uintptr_t aBase)
+    : mBase(aBase), mTrustFlags(ModuleTrustFlags::None) {}
+
 ModuleLoadEvent::ModuleInfo::ModuleInfo(
     const glue::ModuleLoadEvent::ModuleInfo& aOther)
-    : mBase(aOther.mBase), mLoadDurationMS(Some(aOther.mLoadDurationMS)) {
+    : mBase(aOther.mBase),
+      mLoadDurationMS(Some(aOther.mLoadDurationMS)),
+      mTrustFlags(ModuleTrustFlags::None) {
   if (aOther.mLdrName) {
     mLdrName.Assign(aOther.mLdrName.get());
   }
+
   if (aOther.mFullPath) {
     nsDependentString tempPath(aOther.mFullPath.get());
-    Unused << NS_NewLocalFile(tempPath, false, getter_AddRefs(mFile));
+    DebugOnly<nsresult> rv =
+        NS_NewLocalFile(tempPath, false, getter_AddRefs(mFile));
+    MOZ_ASSERT(NS_SUCCEEDED(rv));
+  }
+}
+
+bool ModuleLoadEvent::ModuleInfo::PopulatePathInfo() {
+  MOZ_ASSERT(mBase && mLdrName.IsEmpty() && !mFile);
+  if (!widget::WinUtils::GetModuleFullPath(reinterpret_cast<HMODULE>(mBase),
+                                           mLdrName)) {
+    return false;
+  }
+
+  return NS_SUCCEEDED(NS_NewLocalFile(mLdrName, false, getter_AddRefs(mFile)));
+}
+
+bool ModuleLoadEvent::ModuleInfo::PrepForTelemetry() {
+  MOZ_ASSERT(!mLdrName.IsEmpty() && mFile);
+  if (mLdrName.IsEmpty() || !mFile) {
+    return false;
   }
+
+  using PathTransformFlags = widget::WinUtils::PathTransformFlags;
+
+  if (!widget::WinUtils::PreparePathForTelemetry(
+          mLdrName,
+          PathTransformFlags::Default & ~PathTransformFlags::Canonicalize)) {
+    return false;
+  }
+
+  nsAutoString dllFullPath;
+  if (NS_FAILED(mFile->GetPath(dllFullPath))) {
+    return false;
+  }
+
+  if (!widget::WinUtils::MakeLongPath(dllFullPath)) {
+    return false;
+  }
+
+  // Replace mFile with the lengthened version
+  if (NS_FAILED(NS_NewLocalFile(dllFullPath, false, getter_AddRefs(mFile)))) {
+    return false;
+  }
+
+  nsAutoString sanitized(dllFullPath);
+
+  if (!widget::WinUtils::PreparePathForTelemetry(
+          sanitized,
+          PathTransformFlags::Default & ~(PathTransformFlags::Canonicalize |
+                                          PathTransformFlags::Lengthen))) {
+    return false;
+  }
+
+  mFilePathClean = std::move(sanitized);
+
+  return true;
 }
 
 ModuleLoadEvent::ModuleLoadEvent(const ModuleLoadEvent& aOther,
                                  CopyOption aOption)
     : mIsStartup(aOther.mIsStartup),
       mThreadID(aOther.mThreadID),
       mThreadName(aOther.mThreadName),
       mProcessUptimeMS(aOther.mProcessUptimeMS) {
@@ -112,165 +172,168 @@ static void GetKeyboardLayoutDlls(
       Unused << aOut.emplaceBack(ws);
     }
   }
 }
 
 ModuleEvaluator::ModuleEvaluator() {
   GetKeyboardLayoutDlls(mKeyboardLayoutDlls);
 
-  nsCOMPtr<nsIFile> sysDir;
-  if (NS_SUCCEEDED(
-          NS_GetSpecialDirectory(NS_OS_SYSTEM_DIR, getter_AddRefs(sysDir)))) {
-    sysDir->GetPath(mSysDirectory);
-  }
+  nsresult rv =
+      NS_GetSpecialDirectory(NS_OS_SYSTEM_DIR, getter_AddRefs(mSysDirectory));
+  MOZ_ASSERT(NS_SUCCEEDED(rv));
 
   nsCOMPtr<nsIFile> winSxSDir;
-  if (NS_SUCCEEDED(NS_GetSpecialDirectory(NS_WIN_WINDOWS_DIR,
-                                          getter_AddRefs(winSxSDir)))) {
-    if (NS_SUCCEEDED(winSxSDir->Append(NS_LITERAL_STRING("WinSxS")))) {
-      winSxSDir->GetPath(mWinSxSDirectory);
+  rv = NS_GetSpecialDirectory(NS_WIN_WINDOWS_DIR, getter_AddRefs(winSxSDir));
+  MOZ_ASSERT(NS_SUCCEEDED(rv));
+  if (NS_SUCCEEDED(rv)) {
+    rv = winSxSDir->Append(NS_LITERAL_STRING("WinSxS"));
+    MOZ_ASSERT(NS_SUCCEEDED(rv));
+    if (NS_SUCCEEDED(rv)) {
+      mWinSxSDirectory = std::move(winSxSDir);
     }
   }
 
 #ifdef _M_IX86
-  mSysWOW64Directory.SetLength(MAX_PATH);
-  UINT sysWOWlen =
-      ::GetSystemWow64DirectoryW((char16ptr_t)mSysWOW64Directory.BeginWriting(),
-                                 mSysWOW64Directory.Length());
-  if (!sysWOWlen || (sysWOWlen > mSysWOW64Directory.Length())) {
-    // This could be the following cases:
-    // - Genuine error
-    // - GetLastError == ERROR_CALL_NOT_IMPLEMENTED (32-bit Windows)
-    // - Buffer too small. The buffer being MAX_PATH, this should be so rare we
-    //   don't bother with this case.
-    // In all these cases, consider this directory unavailable.
-    mSysWOW64Directory.Truncate();
-  } else {
-    // In this case, GetSystemWow64DirectoryW returns the length of the string,
-    // not including the null-terminator.
-    mSysWOW64Directory.SetLength(sysWOWlen);
+  WCHAR sysWow64Buf[MAX_PATH + 1] = {};
+
+  UINT sysWowLen =
+      ::GetSystemWow64DirectoryW(sysWow64Buf, ArrayLength(sysWow64Buf));
+  if (sysWowLen > 0 && sysWowLen < ArrayLength(sysWow64Buf)) {
+    rv = NS_NewLocalFile(nsDependentString(sysWow64Buf, sysWowLen), false,
+                         getter_AddRefs(mSysWOW64Directory));
+    MOZ_ASSERT(NS_SUCCEEDED(rv));
   }
 #endif  // _M_IX86
 
-  nsCOMPtr<nsIFile> exeDir;
-  if (NS_SUCCEEDED(
-          NS_GetSpecialDirectory(NS_GRE_DIR, getter_AddRefs(exeDir)))) {
-    exeDir->GetPath(mExeDirectory);
-  }
+  rv = NS_GetSpecialDirectory(NS_GRE_DIR, getter_AddRefs(mExeDirectory));
+  MOZ_ASSERT(NS_SUCCEEDED(rv));
 
   nsCOMPtr<nsIFile> exeFile;
-  if (NS_SUCCEEDED(XRE_GetBinaryPath(getter_AddRefs(exeFile)))) {
+  rv = XRE_GetBinaryPath(getter_AddRefs(exeFile));
+  MOZ_ASSERT(NS_SUCCEEDED(rv));
+  if (NS_SUCCEEDED(rv)) {
     nsAutoString exePath;
-    if (NS_SUCCEEDED(exeFile->GetPath(exePath))) {
+    rv = exeFile->GetPath(exePath);
+    MOZ_ASSERT(NS_SUCCEEDED(rv));
+    if (NS_SUCCEEDED(rv)) {
       ModuleVersionInfo exeVi;
       if (exeVi.GetFromImage(exePath)) {
         mExeVersion = Some(exeVi.mFileVersion.Version64());
       }
     }
   }
 }
 
 Maybe<bool> ModuleEvaluator::IsModuleTrusted(
     ModuleLoadEvent::ModuleInfo& aDllInfo, const ModuleLoadEvent& aEvent,
     Authenticode* aSvc) const {
-  // The JIT profiling module doesn't really have any other practical way to
-  // match; hard-code it as being trusted.
-  if (aDllInfo.mLdrName.EqualsLiteral("JitPI.dll")) {
-    aDllInfo.mTrustFlags = ModuleTrustFlags::JitPI;
-    return Some(true);
-  }
-
-  aDllInfo.mTrustFlags = ModuleTrustFlags::None;
-
-  if (!aDllInfo.mFile) {
-    return Nothing();  // Every check here depends on having a valid image file.
-  }
-
-  using PathTransformFlags = widget::WinUtils::PathTransformFlags;
-
-  Unused << widget::WinUtils::PreparePathForTelemetry(
-      aDllInfo.mLdrName,
-      PathTransformFlags::Default & ~PathTransformFlags::Canonicalize);
+  MOZ_ASSERT(aDllInfo.mTrustFlags == ModuleTrustFlags::None);
+  MOZ_ASSERT(aDllInfo.mFile);
 
   nsAutoString dllFullPath;
   if (NS_FAILED(aDllInfo.mFile->GetPath(dllFullPath))) {
     return Nothing();
   }
-  widget::WinUtils::MakeLongPath(dllFullPath);
+
+  // We start by checking authenticode signatures, since any result from this
+  // test will produce an immediate pass/fail.
+  if (aSvc) {
+    UniquePtr<wchar_t[]> szSignedBy = aSvc->GetBinaryOrgName(dllFullPath.get());
+
+    if (szSignedBy) {
+      nsDependentString signedBy(szSignedBy.get());
 
-  aDllInfo.mFilePathClean = dllFullPath;
-  if (!widget::WinUtils::PreparePathForTelemetry(
-          aDllInfo.mFilePathClean,
-          PathTransformFlags::Default & ~(PathTransformFlags::Canonicalize |
-                                          PathTransformFlags::Lengthen))) {
-    return Nothing();
-  }
-
-  if (NS_FAILED(NS_NewLocalFile(dllFullPath, false,
-                                getter_AddRefs(aDllInfo.mFile)))) {
-    return Nothing();
+      if (signedBy.EqualsLiteral("Microsoft Windows")) {
+        aDllInfo.mTrustFlags |= ModuleTrustFlags::MicrosoftWindowsSignature;
+        return Some(true);
+      } else if (signedBy.EqualsLiteral("Microsoft Corporation")) {
+        aDllInfo.mTrustFlags |= ModuleTrustFlags::MicrosoftWindowsSignature;
+        return Some(true);
+      } else if (signedBy.EqualsLiteral("Mozilla Corporation")) {
+        aDllInfo.mTrustFlags |= ModuleTrustFlags::MozillaSignature;
+        return Some(true);
+      } else {
+        // Being signed by somebody who is neither Microsoft nor us is an
+        // automatic disqualification.
+        aDllInfo.mTrustFlags = ModuleTrustFlags::None;
+        return Some(false);
+      }
+    }
   }
 
   nsAutoString dllDirectory;
   if (!GetDirectoryName(aDllInfo.mFile, dllDirectory)) {
     return Nothing();
   }
 
   nsAutoString dllLeafLower;
   if (NS_FAILED(aDllInfo.mFile->GetLeafName(dllLeafLower))) {
     return Nothing();
   }
+
   ToLowerCase(dllLeafLower);  // To facilitate case-insensitive searching
 
+  // The JIT profiling module doesn't really have any other practical way to
+  // match; hard-code it as being trusted.
+  if (dllLeafLower.EqualsLiteral("jitpi.dll")) {
+    aDllInfo.mTrustFlags = ModuleTrustFlags::JitPI;
+    return Some(true);
+  }
+
   // Accumulate a trustworthiness score as the module passes through several
   // checks. If the score ever reaches above the threshold, it's considered
   // trusted.
-  int scoreThreshold = 100;
+  uint32_t scoreThreshold = 100;
+
 #ifdef ENABLE_TESTS
   // Check whether we are running as an xpcshell test.
   if (mozilla::EnvHasValue("XPCSHELL_TEST_PROFILE_DIR")) {
     // During xpcshell tests, these DLLs are hard-coded to pass through all
     // criteria checks and still result in "untrusted" status, so they show up
     // in the untrusted modules ping for the test to examine.
     // Setting the threshold very high ensures the test will cover all criteria.
     if (dllLeafLower.EqualsLiteral("untrusted-startup-test-dll.dll") ||
         dllLeafLower.EqualsLiteral("modules-test.dll")) {
       scoreThreshold = 99999;
     }
   }
 #endif
 
-  int score = 0;
+  nsresult rv;
+  bool contained;
+
+  uint32_t score = 0;
 
-  // Is the DLL in the system directory?
-  if (!mSysDirectory.IsEmpty() &&
-      StringBeginsWith(dllFullPath, mSysDirectory,
-                       nsCaseInsensitiveStringComparator())) {
-    aDllInfo.mTrustFlags |= ModuleTrustFlags::SystemDirectory;
-    score += 50;
+  if (score < scoreThreshold) {
+    // Is the DLL in the system directory?
+    rv = mSysDirectory->Contains(aDllInfo.mFile, &contained);
+    if (NS_SUCCEEDED(rv) && contained) {
+      aDllInfo.mTrustFlags |= ModuleTrustFlags::SystemDirectory;
+      score += 50;
+    }
   }
 
 #ifdef _M_IX86
   // Under WOW64, SysWOW64 is the effective system directory. Give SysWOW64 the
   // same trustworthiness as ModuleTrustFlags::SystemDirectory.
-  if (!mSysWOW64Directory.IsEmpty() &&
-      StringBeginsWith(dllFullPath, mSysWOW64Directory,
-                       nsCaseInsensitiveStringComparator())) {
-    aDllInfo.mTrustFlags |= ModuleTrustFlags::SysWOW64Directory;
-    score += 50;
+  if (mSysWOW64Directory) {
+    rv = mSysWOW64Directory->Contains(aDllInfo.mFile, &contained);
+    if (NS_SUCCEEDED(rv) && contained) {
+      aDllInfo.mTrustFlags |= ModuleTrustFlags::SysWOW64Directory;
+      score += 50;
+    }
   }
 #endif  // _M_IX86
 
   // Is the DLL in the WinSxS directory? Some Microsoft DLLs (e.g. comctl32) are
   // loaded from here and don't have digital signatures. So while this is not a
   // guarantee of trustworthiness, but is at least as valid as system32.
-  if (!mWinSxSDirectory.IsEmpty() &&
-      StringBeginsWith(dllFullPath, mWinSxSDirectory,
-                       nsCaseInsensitiveStringComparator())) {
+  rv = mWinSxSDirectory->Contains(aDllInfo.mFile, &contained);
+  if (NS_SUCCEEDED(rv) && contained) {
     aDllInfo.mTrustFlags |= ModuleTrustFlags::WinSxSDirectory;
     score += 50;
   }
 
   // Is it a keyboard layout DLL?
   if (std::find(mKeyboardLayoutDlls.begin(), mKeyboardLayoutDlls.end(),
                 dllLeafLower) != mKeyboardLayoutDlls.end()) {
     aDllInfo.mTrustFlags |= ModuleTrustFlags::KeyboardLayout;
@@ -284,19 +347,18 @@ Maybe<bool> ModuleEvaluator::IsModuleTru
     if (vi.GetFromImage(dllFullPath)) {
       aDllInfo.mFileVersion = vi.mFileVersion.ToString();
 
       if (vi.mCompanyName.EqualsLiteral("Microsoft Corporation")) {
         aDllInfo.mTrustFlags |= ModuleTrustFlags::MicrosoftVersion;
         score += 50;
       }
 
-      if (!mExeDirectory.IsEmpty() &&
-          StringBeginsWith(dllFullPath, mExeDirectory,
-                           nsCaseInsensitiveStringComparator())) {
+      rv = mExeDirectory->Contains(aDllInfo.mFile, &contained);
+      if (NS_SUCCEEDED(rv) && contained) {
         score += 50;
         aDllInfo.mTrustFlags |= ModuleTrustFlags::FirefoxDirectory;
 
         if (dllLeafLower.EqualsLiteral("xul.dll")) {
           // The caller wants to know if this DLL is xul.dll, but this flag
           // doesn't need to affect trust score. Xul will be considered trusted
           // by other measures.
           aDllInfo.mTrustFlags |= ModuleTrustFlags::Xul;
@@ -308,32 +370,12 @@ Maybe<bool> ModuleEvaluator::IsModuleTru
             (vi.mFileVersion.Version64() == mExeVersion.value())) {
           aDllInfo.mTrustFlags |= ModuleTrustFlags::FirefoxDirectoryAndVersion;
           score += 50;
         }
       }
     }
   }
 
-  if (score < scoreThreshold) {
-    if (aSvc) {
-      UniquePtr<wchar_t[]> szSignedBy =
-          aSvc->GetBinaryOrgName(dllFullPath.get());
-      if (szSignedBy) {
-        nsAutoString signedBy(szSignedBy.get());
-        if (signedBy.EqualsLiteral("Microsoft Windows")) {
-          aDllInfo.mTrustFlags |= ModuleTrustFlags::MicrosoftWindowsSignature;
-          score = 100;
-        } else if (signedBy.EqualsLiteral("Microsoft Corporation")) {
-          aDllInfo.mTrustFlags |= ModuleTrustFlags::MicrosoftWindowsSignature;
-          score = 100;
-        } else if (signedBy.EqualsLiteral("Mozilla Corporation")) {
-          aDllInfo.mTrustFlags |= ModuleTrustFlags::MozillaSignature;
-          score = 100;
-        }
-      }
-    }
-  }
-
   return Some(score >= scoreThreshold);
 }
 
 }  // namespace mozilla
--- a/toolkit/xre/ModuleEvaluator_windows.h
+++ b/toolkit/xre/ModuleEvaluator_windows.h
@@ -38,26 +38,30 @@ MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(Mo
 
 // This class is very similar to mozilla::glue::ModuleLoadEvent, except this is
 // more Gecko-friendly, and has a few more fields that enable us to evaluate
 // trustworthiness.
 class ModuleLoadEvent {
  public:
   class ModuleInfo {
    public:
-    ModuleInfo() = default;
+    ModuleInfo() = delete;
     ModuleInfo(const ModuleInfo&) = default;
     ModuleInfo(ModuleInfo&&) = default;
     ModuleInfo& operator=(const ModuleInfo&) = default;
     ModuleInfo& operator=(ModuleInfo&&) = default;
 
+    explicit ModuleInfo(uintptr_t aBase);
+
     // Construct from the mozilla::glue version of this class.
     explicit ModuleInfo(const glue::ModuleLoadEvent::ModuleInfo&);
 
-    // The following members should be populated always.
+    bool PopulatePathInfo();
+    bool PrepForTelemetry();
+
     uintptr_t mBase;
     nsString mLdrName;
     nsCOMPtr<nsIFile> mFile;  // Path as reported by GetModuleFileName()
     Maybe<double> mLoadDurationMS;
 
     // The following members are populated as we evaluate the module.
     nsString mFilePathClean;  // Path sanitized for telemetry reporting.
     ModuleTrustFlags mTrustFlags;
@@ -91,27 +95,33 @@ class ModuleLoadEvent {
   uint64_t mProcessUptimeMS;
   Vector<uintptr_t, 0, InfallibleAllocPolicy> mStack;
   Vector<ModuleInfo, 0, InfallibleAllocPolicy> mModules;
 };
 
 // This class performs trustworthiness evaluation for incoming DLLs.
 class ModuleEvaluator {
   Maybe<uint64_t> mExeVersion;  // Version number of the running EXE image
-  nsString mExeDirectory;
-  nsString mSysDirectory;
-  nsString mWinSxSDirectory;
+  nsCOMPtr<nsIFile> mExeDirectory;
+  nsCOMPtr<nsIFile> mSysDirectory;
+  nsCOMPtr<nsIFile> mWinSxSDirectory;
 #ifdef _M_IX86
-  nsString mSysWOW64Directory;
+  nsCOMPtr<nsIFile> mSysWOW64Directory;
 #endif  // _M_IX86
   Vector<nsString, 0, InfallibleAllocPolicy> mKeyboardLayoutDlls;
 
  public:
   ModuleEvaluator();
 
+  explicit operator bool() const {
+    // We exclude mSysWOW64Directory as it may not always be present
+    return mExeVersion.isSome() && mExeDirectory && mSysDirectory &&
+           mWinSxSDirectory;
+  }
+
   /**
    * Evaluates the trustworthiness of the given module and fills in remaining
    * fields in ModuleInfo.
    *
    * @param  aDllInfo [in,out] Info about the DLL in question.
    *                  The following members must be valid:
    *                  mBase
    *                  mFile
--- a/toolkit/xre/WinDllServices.cpp
+++ b/toolkit/xre/WinDllServices.cpp
@@ -138,16 +138,17 @@ class UntrustedModulesManager {
    *          mHasProcessedStartupModules. We grab this value during a lock, and
    *          the caller will need it for subsequent calls, so passing it around
    *          like this avoids at least one lock. The only risk with this is
    *          that we could end up calling ProcessStartupModules() multiple
    *          times, which is totally safe, and would be extremely rare.
    */
   void ProcessQueuedEvents(bool& aHasProcessedStartupModules) {
     MOZ_ASSERT(!NS_IsMainThread());
+    MOZ_ASSERT(!!mEvaluator);
 
     // Hold a reference to DllServices to ensure the object doesn't get deleted
     // during this call.
     RefPtr<DllServices> dllSvcRef(DllServices::Get());
     if (!dllSvcRef) {
       return;
     }
 
@@ -160,29 +161,39 @@ class UntrustedModulesManager {
       // Lock mQueuedEvents to steal its contents, and
       // mHasProcessedStartupModules to see if we can skip some steps.
       // Only trivial (loader lock friendly) code allowed here!
       MutexAutoLock lock(mMutex);
       aHasProcessedStartupModules = mHasProcessedStartupModules;
       mQueuedEvents.swap(queuedEvents);
     }
 
+    if (!mEvaluator) {
+      return;
+    }
+
     Vector<ModuleLoadEvent, 0, InfallibleAllocPolicy> processedEvents;
     int errorModules = 0;
 
     HashSet<uintptr_t, DefaultHasher<uintptr_t>, InfallibleAllocPolicy>
         newTrustedModuleBases;
 
     // Process queued events, weeding out trusted items as we go.
     for (auto& e : queuedEvents) {
       // Create a copy of the event without its modules; we'll then fill them
       // in, filtering out any trusted modules we can ignore.
       ModuleLoadEvent eventCopy(
           e, ModuleLoadEvent::CopyOption::CopyWithoutModules);
       for (auto& m : e.mModules) {
+        bool ok = m.PrepForTelemetry();
+        MOZ_ASSERT(ok);
+        if (!ok) {
+          continue;
+        }
+
         Maybe<bool> maybeIsTrusted =
             mEvaluator.IsModuleTrusted(m, eventCopy, dllSvcRef.get());
 
         // Save xul.dll load timing for the ping payload.
         if ((m.mTrustFlags & ModuleTrustFlags::Xul) &&
             mXULLoadDurationMS.isNothing()) {
           mXULLoadDurationMS = m.mLoadDurationMS;
         }
@@ -303,19 +314,17 @@ class UntrustedModulesManager {
           }
         }
         if (wasFound) {
           continue;
         }
 
         // It's never been seen before so it must be a startup module.
         // One module = one event here.
-        ModuleLoadEvent::ModuleInfo mi;
-        mi.mBase = base;
-        mi.mLoadDurationMS = Nothing();
+        ModuleLoadEvent::ModuleInfo mi(base);
         ModuleLoadEvent e;
         e.mIsStartup = true;
         e.mProcessUptimeMS = 0;
         Unused << e.mModules.emplaceBack(std::move(mi));
         Unused << startupEvents.emplaceBack(std::move(e));
       }
 
       // Since we process startup modules only once, this data is no longer
@@ -326,18 +335,17 @@ class UntrustedModulesManager {
     if (startupEvents.empty()) {
       return false;
     }
 
     // Fill out more info in startupEvents.
     for (auto& e : startupEvents) {
       MOZ_ASSERT(e.mModules.length() == 1);
       ModuleLoadEvent::ModuleInfo& mi(e.mModules[0]);
-      widget::WinUtils::GetModuleFullPath((HMODULE)mi.mBase, mi.mLdrName);
-      Unused << NS_NewLocalFile(mi.mLdrName, false, getter_AddRefs(mi.mFile));
+      mi.PopulatePathInfo();
     }
 
     // Lock mQueuedEvents to add the new items.
     // Only trivial (loader lock friendly) code allowed here!
     MutexAutoLock lock(mMutex);
     for (auto&& e : startupEvents) {
       Unused << mQueuedEvents.emplaceBack(std::move(e));
     }