1826867 - Reloading an extension page that was redirected causes "Access to the file was denied"

Reporter

Description

•

2 years ago

Attached file extension.zip — Details

Steps to reproduce:

Using an extension, inside the listener for onHeadersReceived, redirect any request to an extension page (moz-extension://).
After the redirect happened, reload the page (F5).

run the extension i attached using "web-ext run"
go to the extension settings and give the extension host permissions for any url
visit https://example.com
reload the page

Extension code:

// background.js browser.webRequest.onHeadersReceived.addListener(() => { return {redirectUrl: browser.runtime.getURL("test.html")} }, { urls: ["*://*/*"], }, ["blocking"]);

// test.html <!doctype html> <html lang="en"> <head> <title>Document</title> </head> <body> Hello World! </body> </html>

// manifest.json { "manifest_version": 3, "version": "1", "name": "test", "permissions": [ "webRequest", "webRequestBlocking" ], "host_permissions": [ "<all_urls>" ], "background": { "scripts": [ "background.js" ] }, "web_accessible_resources": [ { "resources" : [ "test.html" ], "matches" : [ "*://*/*" ] } ] }

Actual results:

On first load, the extension page gets rendered.
On reload the error "Access to the file was denied" gets shown and the extension page cannot be rendered anymore.

Furthermore, it shows:
The file at /C:/<project-path>/test.html is not readable.
It may have been removed, moved, or file permissions may be preventing access.

Expected results:

On reload, the extension page should still be rendered.

If you manually copy the link to the extension page (in my case:
moz-extension://0fd8b6b1-b983-480f-aea4-43e8af8dd92a/test.html), the reload works perfectly fine.

Alex Cornestean

Comment 1

•

2 years ago

Hello,

I reproduced the issue on the latest Nightly (113.0a1/20230409095525), Beta (112.0/20230406114409) and Release (111.0.1/20230321111920) under Windows 10 x64 and macOS11.3.1.

After accessing https://example.com, the extension page gets rendered, however, upon page reload, the page is no longer rendered and the "Access to the file was denied" error is shown, as per Comment 0.

However, the above behavior I observed to be happening when loading the manifest.json file of the unpacked extension via about:debugging.

Loading the entire .zip via about:debugging will no longer cause the page to not render upon page reload.

Status: UNCONFIRMED → NEW

status-firefox112: --- → affected

status-firefox113: --- → affected

Ever confirmed: true

Rob Wu [:robwu]

Assignee

Comment 2

•

2 years ago

Somehow the URI associated with the nsIHistoryEntry is the file:-URL instead of the moz-extension:-URL (It is a file:-URL because files within temporarily loaded extensions resolve to the underlying file on disk; for packed extensions it would resolve to jar:-URIs). I would expect a moz-extension:-URI instead (which should resolve to file/jar as needed on refresh).

Cmd (run in the global browser console after visiting example.com that redirects to the moz-extension:-URL): gBrowser.selectedBrowser.browsingContext.sessionHistory.getEntryAtIndex(gBrowser.selectedBrowser.browsingContext.sessionHistory.index)
Actual result: "file:///private/tmp/repro/test.html"
Expected: moz-extension://b0d08b69-57d0-4f7b-a962-67382bd14bd4/test.html

Command that produces the output below (logs captured after reloading the example.com->moz-extension redirected page):

MOZ_LOG=SessionHistory:5 web-ext run -v -u https://example.com -f /path/to/gecko/objdir-debug/dist/NightlyDebug.app/Contents/MacOS/firefox

[Child 80516: Main Thread]: D/SessionHistory Moving the loading entry to the active entry on nsDocShell 111043000 to file:///private/tmp/repro/test.html
[Parent 80513: Main Thread]: V/SessionHistory CanonicalBrowsingContext::SessionHistoryCommit 14b4a3c00 39
[Parent 80513: Main Thread]: V/SessionHistory SHEntry::RemoveLoadId(39)
[Parent 80513: Main Thread]: D/SessionHistory nsSHistory 1117fa4c0
[Parent 80513: Main Thread]: D/SessionHistory +- 0 SH Entry 14b5fe780 36 {5657d34b-e4ba-4009-9ed5-832c9e07f6a7}
[Parent 80513: Main Thread]: D/SessionHistory | URL = about:newtab
[Parent 80513: Main Thread]: D/SessionHistory | Title = New Tab
[Parent 80513: Main Thread]: D/SessionHistory | Name =
[Parent 80513: Main Thread]: D/SessionHistory | Is in BFCache = false
[Parent 80513: Main Thread]: D/SessionHistory >+- 1 SH Entry 146157300 47 {5657d34b-e4ba-4009-9ed5-832c9e07f6a7}
[Parent 80513: Main Thread]: D/SessionHistory URL = file:///private/tmp/repro/test.html
[Parent 80513: Main Thread]: D/SessionHistory Title = Document
[Parent 80513: Main Thread]: D/SessionHistory Name =
[Parent 80513: Main Thread]: D/SessionHistory Is in BFCache = false

Packed xpi, similar start but different URL/Title:

[Parent 80513: Main Thread]: D/SessionHistory +- 0 SH Entry 14be13300 41 {fee0502d-fc90-43b2-9cee-e6fc578c74d0}
[Parent 80513: Main Thread]: D/SessionHistory | URL = about:newtab
[Parent 80513: Main Thread]: D/SessionHistory | Title = New Tab
[Parent 80513: Main Thread]: D/SessionHistory | Name =
[Parent 80513: Main Thread]: D/SessionHistory | Is in BFCache = false
[Parent 80513: Main Thread]: D/SessionHistory >+- 1 SH Entry 13e827c80 56 {fee0502d-fc90-43b2-9cee-e6fc578c74d0}
[Parent 80513: Main Thread]: D/SessionHistory URL = jar:file:///private/tmp/repro/x.xpi!/test.html
[Parent 80513: Main Thread]: D/SessionHistory Title = jar:file:///private/tmp/repro/x.xpi!/test.html
[Parent 80513: Main Thread]: D/SessionHistory Name =
[Parent 80513: Main Thread]: D/SessionHistory Is in BFCache = false

And for the record, the file:-load is denied, with the following stderr output on debug builds:

[Child 80455, StreamTrans #7] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520015 (NS_ERROR_FILE_ACCESS_DENIED): file /path/to/gecko/netwerk/base/nsFileStreams.cpp:509

BugBot [:suhaib / :marco/ :calixte]

Comment 3

•

2 years ago

The severity field is not set for this bug.
:robwu, could you have a look please?

For more information, please visit BugBot documentation.

Flags: needinfo?(rob)

Rob Wu [:robwu]

Assignee

Comment 4

•

2 years ago

We should at least understand what is going on, given the observations in comment 2.

Severity: -- → S3

Flags: needinfo?(rob)

Priority: -- → P2

Whiteboard: [addons-jira]

Jira Integration Bot

Updated

•

2 years ago

See Also: → https://mozilla-hub.atlassian.net/browse/WEBEXT-1239

Rob Wu [:robwu]

Assignee

Updated

•

6 months ago

Duplicate of this bug: 1940339

Rob Wu [:robwu]

Assignee

Comment 6

•

6 months ago

Bug 1940339 provides a test case using DNR, with an analysis at https://bugzilla.mozilla.org/show_bug.cgi?id=1940339#c8 that shows the same issue as comment 2 here.

Rob Wu [:robwu]

Assignee

Comment 7

•

6 months ago

After some debugging, basically adding logging in several places and running the following command (with the test extension from bug 1940339):

MOZ_LOG=DocumentChannel:5,SessionHistory:5 web-ext run -f /path/to/gecko/objdir-debug/dist/NightlyDebug.app/Contents/MacOS/firefox -v -u https://example.org

... I was able to identify a transition from example.org to the file:-URL at https://searchfox.org/mozilla-central/rev/d788991012a1a8ec862787f9799db4954a33045f/netwerk/ipc/DocumentLoadListener.cpp#2556-2558

    fprintf(stderr, "boog?DocumentLoadListener::DoOnStartRequest-old, uri=%s\n", 
        mLoadingSessionHistoryInfo->mInfo.GetURI()->GetSpecOrDefault().get());
    mLoadingSessionHistoryInfo =
        GetDocumentBrowsingContext()->ReplaceLoadingSessionHistoryEntryForLoad(
            mLoadingSessionHistoryInfo.get(), mChannel);
    fprintf(stderr, "boog?DocumentLoadListener::DoOnStartRequest-new, uri=%s\n", 
        mLoadingSessionHistoryInfo->mInfo.GetURI()->GetSpecOrDefault().get());

shows

boog?DocumentLoadListener::DoOnStartRequest-old, uri=https://example.org/
boog?DocumentLoadListener::DoOnStartRequest-new, uri=file:///private/tmp/boog/index.html

mChannel above supplies the URL that is used here, ultimately. Where does it come from? I suspected the extension protocol handler, so I added logging in a place that I expected to be called when resolving moz-extension:-URLs, and got the following call stack:

#01: mozilla::net::ExtensionProtocolHandler::ResolveSpecialCases
#02: mozilla::net::SubstitutingProtocolHandler::ResolveURI
#03: mozilla::net::SubstitutingProtocolHandler::NewChannel
#04: mozilla::net::nsIOService::NewChannelFromURIWithProxyFlagsInternal
#05: NS_NewChannelInternal
#06: mozilla::net::nsHttpChannel::StartRedirectChannelToURI
#07: mozilla::net::nsHttpChannel::HandleAsyncAPIRedirect

Seems like we are getting somewhere. I added additional logging to confirm that the URL mapping was identified correctly:

MOZ_LOG=DocumentChannel:5,SessionHistory:5,nsHttp:5,URILoader:5,nsResProtocol:5 web-ext run -f /path/to/gecko/objdir-debug/dist/NightlyDebug.app/Contents/MacOS/firefox -v -u https://example.org

[Parent 51366: Main Thread]: D/nsResProtocol moz-extension://f87fded6-9f48-4387-be32-1b7b879f74ab/index.html
 -> file:///private/tmp/boog/index.html
Firefox stderr: [Parent 51366: Main Thread]: D/nsHttp nsHttpChannel::SetupReplacementChannel [this=1412b1000 newChannel=13f03ec60 preserveMethod=1]

So, what we have is:

the moz-extension:-URL is resolved to its true internal URL in SubstitutingProtocolHandler::NewChannel(, by calling ResolveURI() (that does the actual URL mapping), followed by ExtensionProtocolHandler::SubstituteChannel, which offers a chance for the ExtensionProtocolHandler to modify the nsIChannel if needed.

Since the ExtensionProtocolHandler is responsible for providing the actual stream, I think that it may be feasible to force the channel URL to the moz-extension:-URL at that point. I note that we already do that for .css files, and when I change the STR to use a css file instead of a HTML file, the bug is not observed.

I have a patch locally that fixes the behavior, but I'll have to verify whether there are unwanted side effects, and write some unit tests before it is even ready for review.

Rob Wu [:robwu]

Assignee

Comment 8

•

6 months ago

I explored the possibility of forcing the channel URL to moz-extension. It works by default but there are edge cases where it breaks.
Another option which I am going to choose as the final direction is to change SessionHistoryEntry's SessionHistoryInfo's use of nsIChannel::GetURI to NS_GetFinalChannelURI. The issue and fix is similar to bug 1685403 and bug 1802385.

On forcing the channel to have a moz-extension:-URL (as sketched in my previous comment at comment 7). Since the parent is responsible for generating a channel, I figured that we could add the following change at https://searchfox.org/mozilla-central/rev/86c208f86f35d53dc824f18f8e540fe5b0663870/netwerk/protocol/res/ExtensionProtocolHandler.cpp#573

+  } else if (!IsNeckoChild()) {
+    // To make sure that downstream consumers show the moz-extension:-URL
+    // instead of leaking the internal "real" URL, wrap the real channel in a
+    // simple channel whose URL is the original moz-extension:-URL.
+    channel = NS_NewSimpleChannel(
+        aURI, aLoadInfo, *result,
+        [](nsIStreamListener* listener, nsIChannel* channel, nsIChannel* origChannel) -> RequestOrReason {
+          MOZ_TRY(origChannel->AsyncOpen(listener));
+          nsCOMPtr<nsIRequest> request(origChannel);
+          return RequestOrCancelable(WrapNotNull(request));
+        });
   } else {
     return NS_OK;
   }

The above works, but fails when extensions.webextensions.remote is disabled, when a process switch occurs as a result of the navigation. This failure is not unique to the above code; as I mentioned before, the .css moz-extension:-URLs are already intercepted similarly. Opening a new tab, pasting a moz-extension:-CSS URL and pressing Enter results in a never-loading tab. On debug builds, the following stack is shown due an assertion failure at https://searchfox.org/mozilla-central/rev/86c208f86f35d53dc824f18f8e540fe5b0663870/docshell/base/nsDocShell.cpp#13305:

[Parent 39583, Main Thread] ###!!! ASSERTION: We should have connected the pending channel!: 'pending', file /path/to/gecko/worktree2/docshell/base/nsDocShell.cpp:13300
#01: NS_DebugBreak
#02: std::__1::__function::__func<nsDocShell::ResumeRedirectedLoad(unsigned long long, int)::$_0, std::__1::allocator<nsDocShell::ResumeRedirectedLoad(unsigned long long, int)::$_0>, nsresult (nsDocShellLoadState*, nsTArray<mozilla::ipc::Endpoint<mozilla::exte
#03: mozilla::dom::ChildProcessChannelListener::OnChannelReady
#04: mozilla::net::DocumentLoadListener::RedirectToParentProcess
#05: mozilla::net::DocumentLoadListener::RedirectToRealChannel
#06: mozilla::net::DocumentLoadListener::TriggerRedirectToRealChannel

... this itself is a pre-existing bug, but let's not make it worse here. (hence fixing this by fixing the downstream issue in SessionHistoryEntry.cpp instead of ).

Assignee: nobody → rob

See Also: → CVE-2023-28160, CVE-2023-29538

Rob Wu [:robwu]

Assignee

Comment 9

•

6 months ago

(In reply to Rob Wu [:robwu] from comment #8)

I explored the possibility of forcing the channel URL to moz-extension. It works by default but there are edge cases where it breaks.
Another option which I am going to choose as the final direction is to change SessionHistoryEntry's SessionHistoryInfo's use of nsIChannel::GetURI to NS_GetFinalChannelURI. The issue and fix is similar to bug 1685403 and bug 1802385.

Note: there have been multiple attempts to select the "correct" URL before, in bug 1732250 and bug 1757458. This would hopefully be the final correct fix.

Comment 10

•

6 months ago

Attached file Bug 1826867 - Use NS_GetFinalChannelURI instead of GetURI in SessionHistoryEntry — Details

When redirects are involved, DocumentLoadListener::DoOnStartRequest
may call ReplaceLoadingSessionHistoryEntryForLoad, which updates the
history entry with the destination of a redirect. But if the redirection
target is a moz-extension:-URL, the URL becomes a jar:file:/file: URL.
This is because SessionHistoryInfo (in SessionHistoryEntry.cpp) looks up
the URL with nsIChannel::GetURI. For moz-extension:-URLs, the
underlying channel has a jar:file: or file: URL, as provided by
ExtensionProtocolHandler (via SubstitutingProtocolHandler::NewChannel).
For details, see https://bugzilla.mozilla.org/show_bug.cgi?id=1826867#c7

To fix this, this patch switches to NS_GetFinalChannelURI instead. For
more history on this type of bug and SessionHistoryInfo, see
https://bugzilla.mozilla.org/show_bug.cgi?id=1826867#c9

Pulsebot

Comment 11

•

6 months ago

Pushed by rob@robwu.nl: https://hg.mozilla.org/integration/autoland/rev/3188e322f798 Use NS_GetFinalChannelURI instead of GetURI in SessionHistoryEntry r=rpl,smaug

Sandor Molnar[:smolnar]

Comment 12

•

6 months ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/3188e322f798

Status: NEW → RESOLVED

Closed: 6 months ago

status-firefox136: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → 136 Branch

Alex Cornestean

Comment 13

•

6 months ago

Verified as Fixed. Tested on the latest Nightly (136.0a1/20250119092932) under Windows 10 x64 and Ubuntu 24.04 LTS.

The extension page is now rendered upon page reload when both the extension manifest.json and extension .zip file are loaded and the "Access to the file was denied" error is no longer shown.

Status: RESOLVED → VERIFIED

status-firefox136: fixed → verified

Ryan VanderMeulen [:RyanVM]

Updated

•

6 months ago

status-firefox112: affected → wontfix

status-firefox113: affected → wontfix

Rob Wu [:robwu]

Assignee

Comment 14

•

6 months ago

Attached file Bug 1826867 - Use NS_GetFinalChannelURI instead of GetURI in SessionHistoryEntry — Details

When redirects are involved, DocumentLoadListener::DoOnStartRequest
may call ReplaceLoadingSessionHistoryEntryForLoad, which updates the
history entry with the destination of a redirect. But if the redirection
target is a moz-extension:-URL, the URL becomes a jar:file:/file: URL.
This is because SessionHistoryInfo (in SessionHistoryEntry.cpp) looks up
the URL with nsIChannel::GetURI. For moz-extension:-URLs, the
underlying channel has a jar:file: or file: URL, as provided by
ExtensionProtocolHandler (via SubstitutingProtocolHandler::NewChannel).
For details, see https://bugzilla.mozilla.org/show_bug.cgi?id=1826867#c7

To fix this, this patch switches to NS_GetFinalChannelURI instead. For
more history on this type of bug and SessionHistoryInfo, see
https://bugzilla.mozilla.org/show_bug.cgi?id=1826867#c9

Original Revision: https://phabricator.services.mozilla.com/D234333

Phabricator Automation

Updated

•

6 months ago

Attachment #9464132 - Flags: approval-mozilla-esr128?

Phabricator Automation

Comment 15

•

6 months ago

esr128 Uplift Approval Request

User impact if declined: Displayed URL in address bar / history does sometimes not match the actual moz-extension:-URL.
Code covered by automated testing: yes
Fix verified in Nightly: yes
Needs manual QE test: no
Steps to reproduce for manual QE testing: QE not needed; but if desired there are two test cases, one in bug 1826867 and another in bug 1940339.
Risk associated with taking this patch: Low
Explanation of risk level: Switched to the correct way to determine the URL, which was used before in other bugs (https://bugzilla.mozilla.org/show_bug.cgi?id=1826867#c8), along with a fall back to the original logic in case it somehow fails unexpectedly.
String changes made/needed: None
Is Android affected?: yes

Phabricator Automation

Updated

•

6 months ago

Attachment #9464132 - Flags: approval-mozilla-esr128? → approval-mozilla-esr128+

Pulsebot

Comment 16

•

6 months ago

uplift

https://hg.mozilla.org/releases/mozilla-esr128/rev/d81d36e86029

Lando Automation

Updated

•

6 months ago

status-firefox-esr128: --- → fixed

Alex Cornestean

Comment 17

•

5 months ago

Verified as Fixed. Tested on the latest ESR (128.8.0esr/20250208133428 from https://treeherder.mozilla.org/jobs?repo=mozilla-esr128&revision=d81d36e86029842297fd95a9ccfafc7a986e05aa) under Windows 10 and Ubuntu 24.04 LTS, using the extension attached in Comment 0.

The extension page is now rendered upon page reload when both the extension manifest.json and extension .zip file are loaded and the "Access to the file was denied" error is no longer shown.

status-firefox-esr128: fixed → verified

extension.zip 2 years ago kitrik 753 bytes, application/x-zip-compressed		Details
Bug 1826867 - Use NS_GetFinalChannelURI instead of GetURI in SessionHistoryEntry 6 months ago Rob Wu [:robwu] 48 bytes, text/x-phabricator-request		Details \| Review
Bug 1826867 - Use NS_GetFinalChannelURI instead of GetURI in SessionHistoryEntry 6 months ago Rob Wu [:robwu] 48 bytes, text/x-phabricator-request	phab-bot : approval-mozilla-esr128+	Details \| Review