High firefox Logic Error 🔧 Commit mapped

Overview

High
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Impacthigh
DescriptionUse-after-free in the Networking component
ComponentNetworking
Bug ClassLogic Error
Tracker2079068
Fix commitffbdff5f0da6 (firefox) +23/-31
CISA KEVNot listed
CreditedMozilla
Disclosed2026-10-27

Changed Functions

FunctionChangeNotes
if
netwerk/base/nsStandardURL.cpp
modified
add_task
netwerk/test/unit/test_URIs.js
modified

Files Changed

  • netwerk/base/nsStandardURL.cpp
  • netwerk/base/nsStandardURL.h
  • netwerk/protocol/res/SubstitutingProtocolHandler.cpp
  • netwerk/protocol/res/SubstitutingURL.h
  • netwerk/test/unit/test_URIs.js
diff --git a/netwerk/base/nsStandardURL.cpp b/netwerk/base/nsStandardURL.cpp
index cbe3ca98d1c..e58d9a3f65f 100644
--- a/netwerk/base/nsStandardURL.cpp
+++ b/netwerk/base/nsStandardURL.cpp
@@ -2283,8 +2283,9 @@ nsresult nsStandardURL::EqualsInternal(
     // really failures, more or less, so propagate them to caller.
     *result = false;
 
-    rv = EnsureFile();
-    nsresult rv2 = other->EnsureFile();
+    nsCOMPtr<nsIFile> file, otherFile;
+    rv = ResolveFile(getter_AddRefs(file));
+    nsresult rv2 = other->ResolveFile(getter_AddRefs(otherFile));
 
     // Special case for resource:// urls that don't resolve to files,
     // and for moz-extension://UUID/_generated_background_page.html
@@ -2299,7 +2300,6 @@ nsresult nsStandardURL::EqualsInternal(
            this, mSpec.get()));
       return rv;
     }
-    NS_ASSERTION(mFile, "EnsureFile() lied!");
 
     rv = rv2;
     if (NS_FAILED(rv)) {
@@ -2309,8 +2309,7 @@ nsresult nsStandardURL::EqualsInternal(
            other.get(), other->mSpec.get()));
       return rv;
     }
-    NS_ASSERTION(other->mFile, "EnsureFile() lied!");
-    return mFile->Equals(other->mFile, result);
+    return file->Equals(otherFile, result);
   }
 
   // The URLs are not identical, and they do not correspond to the
@@ -3223,13 +3222,12 @@ nsresult nsStandardURL::SetFileExtensionInternal(const nsACString& input) {
 // nsStandardURL::nsIFileURL
 //----------------------------------------------------------------------------
 
-nsresult nsStandardURL::EnsureFile() {
+nsresult nsStandardURL::ResolveFile(nsIFile** aResult) {
   MOZ_ASSERT(mSupportsFileURL,
-             "EnsureFile() called on a URL that doesn't support files!");
+             "ResolveFile() called on a URL that doesn't support files!");
 
   if (mFile) {
-    // Nothing to do
-    return NS_OK;
+    return mFile->Clone(aResult);
   }
 
   // Parse the spec if we don't have a cached result
@@ -3243,7 +3241,7 @@ nsresult nsStandardURL::EnsureFile() {
     return NS_ERROR_FAILURE;
   }
 
-  return net_GetFileFromURLSpec(mSpec, getter_AddRefs(mFile));
+  return net_GetFileFromURLSpec(mSpec, aResult);
 }
 
 NS_IMETHODIMP
@@ -3251,23 +3249,19 @@ nsStandardURL::GetFile(nsIFile** result) {
   MOZ_ASSERT(mSupportsFileURL,
              "GetFile() called on a URL that doesn't support files!");
 
-  nsresult rv = EnsureFile();
+  nsCOMPtr<nsIFile> file;
+  nsresult rv = ResolveFile(getter_AddRefs(file));
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   if (LOG_ENABLED()) {
     LOG(("nsStandardURL::GetFile [this=%p spec=%s resulting_path=%s]\n", this,
-         mSpec.get(), mFile->HumanReadablePath().get()));
+         mSpec.get(), file->HumanReadablePath().get()));
   }
 
-  // clone the file, so the caller can modify it.
-  // XXX nsIFileURL.idl specifies that the consumer must _not_ modify the
-  // nsIFile returned from this method; but it seems that some folks do
-  // (see bug 161921). until we can be sure that all the consumers are
-  // behaving themselves, we'll stay on the safe side and clone the file.
-  // see bug 212724 about fixing the consumers.
-  return mFile->Clone(result);
+  file.forget(result);
+  return NS_OK;
 }
 
 nsresult nsStandardURL::SetFile(nsIFile* file) {
diff --git a/netwerk/base/nsStandardURL.h b/netwerk/base/nsStandardURL.h
index 7da0f3f24f1..4d412baec5b 100644
--- a/netwerk/base/nsStandardURL.h
+++ b/netwerk/base/nsStandardURL.h
@@ -228,10 +228,10 @@ class nsStandardURL : public nsIFileURL,
                        const nsACString& newRef, bool copyCached = false);
 
   // Helper for subclass implementation of GetFile().  Subclasses that map
-  // URIs to files in a special way should implement this method.  It should
-  // ensure that our mFile is initialized, if it's possible.
-  // returns NS_ERROR_NO_INTERFACE if the url does not map to a file
-  virtual nsresult EnsureFile();
+  // URIs to files in a special way should implement this method.  Returns a
+  // file the caller owns, or NS_ERROR_NO_INTERFACE if the url does not map to
+  // a file.  Must not write mFile: the URL may be in use on other threads.
+  virtual nsresult ResolveFile(nsIFile** aResult);
 
   virtual nsresult Clone(nsIURI** aURI);
   virtual nsresult SetSpecInternal(const nsACString& input);
@@ -407,11 +407,9 @@ class nsStandardURL : public nsIFileURL,
   // Concretely typed so ParseAll() is reachable without a downcast.
   RefPtr<nsBaseURLParser> mParser;
 
-  // mFile is protected so subclasses can access it directly
- protected:
-  nsCOMPtr<nsIFile> mFile;  // cached result for nsIFileURL::GetFile
+  // Only set via SetFile() or cloning, never lazily.
+  nsCOMPtr<nsIFile> mFile;
 
- private:
   // cached result for nsIURI::GetDisplayHost
   nsCString mDisplayHost;
 
diff --git a/netwerk/protocol/res/SubstitutingProtocolHandler.cpp b/netwerk/protocol/res/SubstitutingProtocolHandler.cpp
index 8aa1a50f0c6..0a89a2eb98c 100644
--- a/netwerk/protocol/res/SubstitutingProtocolHandler.cpp
+++ b/netwerk/protocol/res/SubstitutingProtocolHandler.cpp
@@ -55,7 +55,7 @@ NS_IMPL_ADDREF_INHERITED(SubstitutingURL, nsStandardURL)
 NS_IMPL_RELEASE_INHERITED(SubstitutingURL, nsStandardURL)
 NS_IMPL_QUERY_INTERFACE_CI_INHERITED0(SubstitutingURL, nsStandardURL)
 
-nsresult SubstitutingURL::EnsureFile() {
+nsresult SubstitutingURL::ResolveFile(nsIFile** aResult) {
   nsAutoCString ourScheme;
   nsresult rv = GetScheme(ourScheme);
   NS_ENSURE_SUCCESS(rv, rv);
@@ -87,7 +87,7 @@ nsresult SubstitutingURL::EnsureFile() {
   // when the scheme isn't file.
   if (!scheme.EqualsLiteral("file")) return NS_ERROR_NO_INTERFACE;
 
-  return net_GetFileFromURLSpec(spec, getter_AddRefs(mFile));
+  return net_GetFileFromURLSpec(spec, aResult);
 }
 
 /* virtual */
diff --git a/netwerk/protocol/res/SubstitutingURL.h b/netwerk/protocol/res/SubstitutingURL.h
index e252ebfbc7b..58aae9b2fc4 100644
--- a/netwerk/protocol/res/SubstitutingURL.h
+++ b/netwerk/protocol/res/SubstitutingURL.h
@@ -18,7 +18,7 @@ class SubstitutingURL : public nsStandardURL {
  public:
   NS_DECL_ISUPPORTS_INHERITED
   virtual nsStandardURL* StartClone() override;
-  [[nodiscard]] virtual nsresult EnsureFile() override;
+  [[nodiscard]] virtual nsresult ResolveFile(nsIFile** aResult) override;
 
  private:
   explicit SubstitutingURL() : nsStandardURL(true) {}
diff --git a/netwerk/test/unit/test_URIs.js b/netwerk/test/unit/test_URIs.js
index 4895527fede..df64328d7f5 100644
--- a/netwerk/test/unit/test_URIs.js
+++ b/netwerk/test/unit/test_URIs.js
@@ -1022,7 +1022,7 @@ add_task(async function round_trip_invalid_ace_label() {
 add_task(async function test_bug1875119() {
   let uri1 = Services.io.newURI("file:///path");
   let uri2 = Services.io.newURI("resource://test/bla");
-  // type of uri2 is still SubstitutingURL which overrides the implementation of EnsureFile,
+  // type of uri2 is still SubstitutingURL which overrides the implementation of ResolveFile,
   // but it's scheme is now file.
   // See https://bugzilla.mozilla.org/show_bug.cgi?id=1876483 to disallow this
   uri2 = uri2.mutate().setSpec("file:///path2").finalize();
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/netwerk/test/unit/test_URIs.js b/netwerk/test/unit/test_URIs.js
index 4895527fede..df64328d7f5 100644
--- a/netwerk/test/unit/test_URIs.js
+++ b/netwerk/test/unit/test_URIs.js
@@ -1022,7 +1022,7 @@ add_task(async function round_trip_invalid_ace_label() {
 add_task(async function test_bug1875119() {
   let uri1 = Services.io.newURI("file:///path");
   let uri2 = Services.io.newURI("resource://test/bla");
-  // type of uri2 is still SubstitutingURL which overrides the implementation of EnsureFile,
+  // type of uri2 is still SubstitutingURL which overrides the implementation of ResolveFile,
   // but it's scheme is now file.
   // See https://bugzilla.mozilla.org/show_bug.cgi?id=1876483 to disallow this
   uri2 = uri2.mutate().setSpec("file:///path2").finalize();
Loading diff…