Closed Bug 1458449 Opened 2 years ago Closed 2 years ago

Firefox blocks xml documents on ftp servers from loading subresources like xslt files

Categories

(Core :: DOM: Security, defect, P1)

61 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox59 --- unaffected
firefox60 --- unaffected
firefox61 + verified

People

(Reporter: felix.bau, Assigned: evilpie)

References

Details

(Keywords: nightly-community, regression, site-compat, Whiteboard: [domsecurity-active])

Attachments

(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:61.0) Gecko/20100101 Firefox/61.0
Build ID: 20180430220108

Steps to reproduce:

Opened ftp://ftp.fu-berlin.de/doc/rfc/authors/rfc7430.xml from Google


Actual results:

I was slapped in the face by a wall of text, no for real:
Firefox blocked the stylesheet (xslt file) from loading.
console log:
"Loading FTP subresource within http(s) page not allowed (Blocked loading of: “ftp://ftp.fu-berlin.de/doc/rfc/authors/rfc2629.xslt”)"

My problem: it's within an ftp page/document and not an http page
Is this a bug or desired?
If it's desired, then the error needs to be adjusted.


Expected results:

I'm not sure if it makes sense, to block stylesheets, if the xml or even html is located on the same ftp server already.

If you don't want this, then you should block html/xml from ftp all together instead of showing the user incomplete documents.

Also: Downloading the xml file (whole website), doesn't download the xslt file, so opening it locally doesn't work without downloading the files one by one (taking a look at the source file to get required URLs)
Which means: you cannot read those documents unless you get a real FTP client or learn xml.

Another bug:
Firefox tries to render the xslt file for some reason, thereby complicating the download, since I have to click "view source" in the context menu first, to be able to save the page.
STRG+S on the open xslt file doesn't select the correct file name and tries to save it as html.
I could open another bug report for this, if you think it's different enough.
I can confirm the bug. Chrome Unstable doesn't block these ressources and displays the page styled. Tom, is this the desired behaviour given that my understanding of bug #1404744 was that we were aligning our behaviour to Chrome's?
Blocks: 1404744
Status: UNCONFIRMED → NEW
Component: Untriaged → DOM: Security
Ever confirmed: true
Flags: needinfo?(evilpies)
Product: Firefox → Core
I don't really care about how much we break FTP, but this was not really intentional. I guess we could allow loading FTP subresources in FTP pages.
Flags: needinfo?(evilpies)
Assignee: nobody → evilpies
I wasn't quite sure if using TriggeringPrincipal would be better here, but I think the LoadingPrinicpal allows the load in more cases, judging from the comment in nsILoadInfo.idl.
Attachment #8972538 - Flags: review?(ckerschb)
Keywords: site-compat
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [domsecurity-active]
Comment on attachment 8972538 [details] [diff] [review]
Allow FTP subresource in FTP documents

Review of attachment 8972538 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/security/nsContentSecurityManager.cpp
@@ +191,5 @@
>      return NS_OK;
>    }
>  
> +  // Allow loading FTP subresources in FTP documents, like XML.
> +  nsIPrincipal* loadingPrincipal = loadInfo->LoadingPrincipal();

While in that case the difference is more semantical I would like us to use the triggeringPrincipal. The triggeringPricipal is the principal that actually triggers the load and hence the principal we should use for the check here.
Attachment #8972538 - Flags: review?(ckerschb)
Okay. Seems like I need to spend a bit of time in the future figuring out those differences.
Attachment #8972538 - Attachment is obsolete: true
Attachment #8972638 - Flags: review?(ckerschb)
Comment on attachment 8972638 [details] [diff] [review]
v2 Allow FTP subresource in FTP documents

Review of attachment 8972638 [details] [diff] [review]:
-----------------------------------------------------------------

thanks.r=me
Attachment #8972638 - Flags: review?(ckerschb) → review+
Pushed by evilpies@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/33db56e4ee57
Allow FTP subresource in FTP documents. r=ckerschb
https://hg.mozilla.org/mozilla-central/rev/33db56e4ee57
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Can we land an automated test for this?
Flags: qe-verify+
Flags: needinfo?(evilpies)
Flags: in-testsuite?
I think we can't test FTP pages, but I am not 100% sure.
Flags: needinfo?(evilpies) → needinfo?(ckerschb)
Hey Valentin, do we have any tests relying on FTP? If so, could you guide us to them?
Flags: needinfo?(ckerschb) → needinfo?(valentin.gosu)
I don't think we have any :(
Flags: needinfo?(valentin.gosu)
Thank you for taking a look and fixing it so quickly :)
I can open and view the file correctly again since 61.0a1 (2018-05-03)

I created a separate bug report for the other bug I discovered along the way:
https://bugzilla.mozilla.org/show_bug.cgi?id=1459057

Regards
Managed to reproduce the issue on Firefox Nightly (2018-05-01) on Windows 10 x64, Mac OS X 10.12 and Ubuntu 16.04 x64.

The issue cannot be reproduced on the latest Nightly Firefox 61.0a1. (20180503220110 / 2018-05-03) on Windows 10 x64, Mac OS X 10.12 and Ubuntu 16.04 x64, thus I will close the issue as VERIFIED FIXED.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.