tor-browser

commit cf9164095983617265d14b4df0fa78dfec3c98ec
parent 991223d67804acc5d1c81631898fae77669ccbe3
Author: Bastian Gruber <foreach@me.com>
Date:   Fri,  5 Dec 2025 20:15:10 +0000

Bug 2004168 - Prevent UAF in viaduct-necko ResponseListener constructor r=bdk,mccr8

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

Diffstat:
M
services/application-services/components/viaduct-necko/backend.cpp
 | 
78
++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------

1 file changed, 52 insertions(+), 26 deletions(-)
diff --git a/services/application-services/components/viaduct-necko/backend.cpp b/services/application-services/components/viaduct-necko/backend.cpp
@@ -161,32 +161,52 @@ class ViaductResponseListener final : public nsIHttpHeaderVisitor,
   NS_DECL_NSITIMERCALLBACK
   NS_DECL_NSINAMED
 
-  explicit ViaductResponseListener(ViaductRequestGuard&& aGuard,
-                                   uint32_t aTimeoutSecs)
-      : mGuard(std::move(aGuard)), mChannel(nullptr) {
-    MOZ_LOG(gViaductLogger, LogLevel::Info,
-            ("TRACE: ViaductResponseListener constructor called with timeout: "
-             "%u seconds, guard valid: %s",
-             aTimeoutSecs, mGuard.IsValid() ? "true" : "false"));
-
-    // Create timeout timer if timeout > 0
-    if (aTimeoutSecs > 0) {
-      MOZ_LOG(gViaductLogger, LogLevel::Debug,
-              ("Setting timeout timer for %u seconds", aTimeoutSecs));
-      nsresult rv =
-          NS_NewTimerWithCallback(getter_AddRefs(mTimeoutTimer), this,
-                                  aTimeoutSecs * 1000, nsITimer::TYPE_ONE_SHOT);
-      if (NS_WARN_IF(NS_FAILED(rv))) {
-        MOZ_LOG(gViaductLogger, LogLevel::Error,
-                ("Failed to create timeout timer: 0x%08x",
-                 static_cast<uint32_t>(rv)));
-      }
+  // Use Create() instead of calling the constructor directly.
+  // Timer creation must happen after a RefPtr holds a reference.
+  // Returns nullptr if timer creation fails (when aTimeoutSecs > 0).
+  static already_AddRefed<ViaductResponseListener> Create(
+      ViaductRequestGuard&& aGuard, uint32_t aTimeoutSecs,
+      nsresult* aOutTimerRv = nullptr) {
+    RefPtr<ViaductResponseListener> listener =
+        new ViaductResponseListener(std::move(aGuard));
+    nsresult rv = listener->StartTimeoutTimer(aTimeoutSecs);
+    if (aOutTimerRv) {
+      *aOutTimerRv = rv;
     }
+    if (NS_FAILED(rv)) {
+      return nullptr;
+    }
+    return listener.forget();
   }
 
   void SetChannel(nsIChannel* aChannel) { mChannel = aChannel; }
 
  private:
+  explicit ViaductResponseListener(ViaductRequestGuard&& aGuard)
+      : mGuard(std::move(aGuard)), mChannel(nullptr) {
+    MOZ_LOG(gViaductLogger, LogLevel::Info,
+            ("TRACE: ViaductResponseListener constructor called, guard valid: "
+             "%s",
+             mGuard.IsValid() ? "true" : "false"));
+  }
+
+  nsresult StartTimeoutTimer(uint32_t aTimeoutSecs) {
+    if (aTimeoutSecs == 0) {
+      return NS_OK;
+    }
+    MOZ_LOG(gViaductLogger, LogLevel::Debug,
+            ("Setting timeout timer for %u seconds", aTimeoutSecs));
+    nsresult rv =
+        NS_NewTimerWithCallback(getter_AddRefs(mTimeoutTimer), this,
+                                aTimeoutSecs * 1000, nsITimer::TYPE_ONE_SHOT);
+    if (NS_FAILED(rv)) {
+      MOZ_LOG(gViaductLogger, LogLevel::Error,
+              ("Failed to create timeout timer: 0x%08x",
+               static_cast<uint32_t>(rv)));
+    }
+    return rv;
+  }
+
   ~ViaductResponseListener() {
     MOZ_LOG(gViaductLogger, LogLevel::Info,
             ("TRACE: ViaductResponseListener destructor called"));
@@ -639,14 +659,20 @@ void viaduct_necko_backend_send_request(const ViaductRequest* request,
         // Get timeout before moving the guard
         uint32_t timeout = guard.Request()->timeout;
 
-        // Create listener with timeout support.
-        // Move the guard into the listener so it owns the request/result
-        // pointers.
+        // Create listener using factory method. This ensures the timer is
+        // created after a RefPtr holds a reference.
+        nsresult timerRv;
         RefPtr<ViaductResponseListener> listener =
-            new ViaductResponseListener(std::move(guard), timeout);
+            ViaductResponseListener::Create(std::move(guard), timeout,
+                                            &timerRv);
+        if (!listener) {
+          MOZ_LOG(gViaductLogger, LogLevel::Error,
+                  ("Failed to create listener: timer creation failed 0x%08x",
+                   static_cast<uint32_t>(timerRv)));
+          return;
+        }
 
-        // Store the channel in the listener so it can cancel it on
-        // timeout
+        // Store the channel in the listener so it can cancel it on timeout.
         listener->SetChannel(channel);
 
         MOZ_LOG(gViaductLogger, LogLevel::Debug, ("Opening HTTP channel"));