Bug 1605478 - Add temporary MOZ_DIAGNOSTIC_ASSERT to narrow down UntrustedModulesData's integrity problem. r=aklotz

Bug 1603714 showed there were `UntrustedModulesData` instances in which a load
event pointed to a module which did not exist in the modules list.

This patch adds `MOZ_DIAGNOSTIC_ASSERT` to the following places to narrow down
when it happened.

Given that the number of the impected users seems big (~200 crashes/day on Nightly),
we activate the assers with a probability of 1/16 (~12.5 crashes/day).

1. When processing load events
1-1. [Content] `UntrustedModulesProcessor::CompleteProcessing:`
  Verify events of a trusted module were eliminated by `GetModulesTrust`
1-2. [Content] `UntrustedModulesData::AddNewLoads`:
  Verify a new `ModuleRecord` matches the event
1-3. [Content] `UntrustedModulesProcessor::CompleteProcessing`:
  Verify processed data after new items were appended.

2. When processed data is sent
2-1. [Content] `UntrustedModulesProcessor::GetAllProcessedData`:
  Verify processed data before serialization.
2-2. [Content] `ParamTraits<mozilla::UntrustedModulesData>::WriteEvent`:
  Verify processed data before transferring to the browser process
2-3. [Browser] `ParamTraits<mozilla::UntrustedModulesData>::ReadEvent`:
  A final point to catch this integrity problem.  We had an IPC error here.

Differential Revision: https://phabricator.services.mozilla.com/D59964

--HG--
extra : moz-landing-system : lando

toolkit/xre/UntrustedModulesData.cpp

@@ -13,6 +13,7 @@
 #include "mozilla/FileUtilsWin.h"
 #include "mozilla/Likely.h"
 #include "mozilla/MathAlgorithms.h"
+#include "mozilla/RandomNum.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/Unused.h"
 #include "mozilla/WinDllServices.h"
@@ -301,6 +302,35 @@ bool ProcessedModuleLoadEvent::IsTrusted() const {
   return mModule->IsTrusted();
 }
 
+void UntrustedModulesData::VerifyConsistency() const {
+  if (!mIsDiagnosticsAssertEnabled) {
+    return;
+  }
+
+  for (auto& evt : mEvents) {
+    MOZ_DIAGNOSTIC_ASSERT(evt.mModule, "Empty module");
+    MOZ_DIAGNOSTIC_ASSERT(!evt.mModule->mResolvedNtName.IsEmpty(),
+                          "Empty mResolvedNtName");
+    MOZ_DIAGNOSTIC_ASSERT(mModules.Get(evt.mModule->mResolvedNtName, nullptr),
+                          "No match in the table");
+  }
+}
+
+/* static */
+bool UntrustedModulesData::IsDiagnosticsAssertEnabled() {
+#ifdef NIGHTLY_BUILD
+  // Trigger MOZ_DIAGNOSTIC_ASSERT with a probability of 1/16
+  constexpr double kDiagnosticsAssertRatio = 0.0625;
+
+  constexpr uint64_t kBoundary =
+      std::numeric_limits<uint64_t>::max() * kDiagnosticsAssertRatio;
+  Maybe<uint64_t> randomNum = RandomUint64();
+  return randomNum.isSome() && randomNum.value() <= kBoundary;
+#else
+  return false;
+#endif
+}
+
 void UntrustedModulesData::AddNewLoads(
     const ModulesMap& aModules, Vector<ProcessedModuleLoadEvent>&& aEvents,
     Vector<Telemetry::ProcessedStack>&& aStacks) {
@@ -319,6 +349,9 @@ void UntrustedModulesData::AddNewLoads(
     }
 
     RefPtr<ModuleRecord> rec(iter.Data());
+    if (mIsDiagnosticsAssertEnabled) {
+      MOZ_DIAGNOSTIC_ASSERT(rec->mResolvedNtName == iter.Key());
+    }
     addPtr.OrInsert([rec = std::move(rec)]() { return rec; });
   }
 

toolkit/xre/UntrustedModulesData.h

@@ -171,7 +171,8 @@ class UntrustedModulesData final {
       : mProcessType(XRE_GetProcessType()),
         mPid(::GetCurrentProcessId()),
         mSanitizationFailures(0),
-        mTrustTestFailures(0) {}
+        mTrustTestFailures(0),
+        mIsDiagnosticsAssertEnabled(IsDiagnosticsAssertEnabled()) {}
 
   UntrustedModulesData(UntrustedModulesData&&) = default;
   UntrustedModulesData& operator=(UntrustedModulesData&&) = default;
@@ -190,6 +191,9 @@ class UntrustedModulesData final {
 
   void Swap(UntrustedModulesData& aOther);
 
+  void VerifyConsistency() const;
+  static bool IsDiagnosticsAssertEnabled();
+
   GeckoProcessType mProcessType;
   DWORD mPid;
   TimeDuration mElapsed;
@@ -199,6 +203,10 @@ class UntrustedModulesData final {
   Maybe<double> mXULLoadDurationMS;
   uint32_t mSanitizationFailures;
   uint32_t mTrustTestFailures;
+
+  // This is not serialized.
+  // Cannot be const as we have the default move ctor.
+  bool mIsDiagnosticsAssertEnabled;
 };
 
 class ModulesMapResult final {
@@ -426,6 +434,8 @@ struct ParamTraits<mozilla::UntrustedModulesData> {
   typedef mozilla::UntrustedModulesData paramType;
 
   static void Write(Message* aMsg, const paramType& aParam) {
+    aParam.VerifyConsistency();
+
     aMsg->WriteUInt32(aParam.mProcessType);
     aMsg->WriteULong(aParam.mPid);
     WriteParam(aMsg, aParam.mElapsed);
@@ -475,7 +485,7 @@ struct ParamTraits<mozilla::UntrustedModulesData> {
 
     for (uint32_t curEventIdx = 0; curEventIdx < eventsLen; ++curEventIdx) {
       if (!ReadEvent(aMsg, aIter, &(aResult->mEvents[curEventIdx]),
-                     aResult->mModules)) {
+                     aResult->mModules, aResult->mIsDiagnosticsAssertEnabled)) {
         return false;
       }
     }
@@ -523,7 +533,8 @@ struct ParamTraits<mozilla::UntrustedModulesData> {
   // specialization.
   static bool ReadEvent(const Message* aMsg, PickleIterator* aIter,
                         mozilla::ProcessedModuleLoadEvent* aResult,
-                        const mozilla::ModulesMap& aModulesMap) {
+                        const mozilla::ModulesMap& aModulesMap,
+                        bool aIsDiagnosticsAssertEnabled) {
     if (!aMsg->ReadUInt64(aIter, &aResult->mProcessUptimeMS)) {
       return false;
     }
@@ -558,6 +569,11 @@ struct ParamTraits<mozilla::UntrustedModulesData> {
     // rather than an IPC error. The error is detected and dealt with in
     // telemetry.
     aResult->mModule = aModulesMap.Get(resolvedNtName);
+    if (!aResult->mModule && aIsDiagnosticsAssertEnabled) {
+      MOZ_DIAGNOSTIC_ASSERT(aModulesMap.Count() > 0, "Empty module list");
+      MOZ_DIAGNOSTIC_ASSERT(!resolvedNtName.IsEmpty(), "Empty resolvedNtName");
+      MOZ_DIAGNOSTIC_ASSERT(false, "Something else");
+    }
 
     return true;
   }

toolkit/xre/UntrustedModulesProcessor.cpp

@@ -381,6 +381,8 @@ RefPtr<UntrustedModulesPromise> UntrustedModulesProcessor::GetAllProcessedData(
 
   result.mElapsed = TimeStamp::Now() - TimeStamp::ProcessCreation();
 
+  result.VerifyConsistency();
+
   return UntrustedModulesPromise::CreateAndResolve(
       Some(UntrustedModulesData(std::move(result))), aSource);
 }
@@ -875,6 +877,12 @@ void UntrustedModulesProcessor::CompleteProcessing(
         return;
       }
 
+      // Trusted modules should have been eliminated by GetModulesTrustInternal
+      // in the browser process
+      if (mProcessedModuleLoads.mIsDiagnosticsAssertEnabled) {
+        MOZ_DIAGNOSTIC_ASSERT(!event.IsTrusted());
+      }
+
       Telemetry::ProcessedStack processedStack =
           stackProcessor.GetStackAndModules(backtrace);
 
@@ -895,6 +903,9 @@ void UntrustedModulesProcessor::CompleteProcessing(
 
   mProcessedModuleLoads.AddNewLoads(modules, std::move(processedEvents),
                                     std::move(processedStacks));
+
+  mProcessedModuleLoads.VerifyConsistency();
+
   if (maybeXulLoadDuration) {
     MOZ_ASSERT(!mProcessedModuleLoads.mXULLoadDurationMS);
     mProcessedModuleLoads.mXULLoadDurationMS = maybeXulLoadDuration;