Firefox · Networking
CVE-2026-117907
Logic Error in Networking
Overview
High
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifnetwerk/base/nsStandardURL.cpp |
modified | |
add_tasknetwerk/test/unit/test_URIs.js |
modified |
Files Changed
netwerk/base/nsStandardURL.cppnetwerk/base/nsStandardURL.hnetwerk/protocol/res/SubstitutingProtocolHandler.cppnetwerk/protocol/res/SubstitutingURL.hnetwerk/test/unit/test_URIs.js
Patch
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…
References
On This Page