1645796 - Empty Content-Type with XCTO: nosniff will not offer "Open in Nightly" for PDFs

Status: REOPENED

(Firefox :: File Handling, defect, P3)

Description

[giul.mus]

Attachment: Download window

Reproducible testcase: https://didattica.polito.it/pls/portal30/sviluppo.pkg_apply_admin.download_bando?p_id_bando=14836&p_tipo=bando

The server returns a PDF file, but with an empty Content-Type in the HTTP response. Firefox correctly identifies the file as a PDF, but otherwise acts as if it were a text file: it does not show an "Open in Nightly" option nor will it suggest to open it with PDF viewers (it will suggest a text editor). I verified that if I manually fix the Content-Type header with a proxy, the bug does not occur.

Furthermore, Nightly identifies the file as a PDF, but Release does not (it detects a text file instead). I believe this is an intended enhancement though.

HTTP response:

HTTP/1.1 200 OK
Date: Mon, 15 Jun 2020 12:23:26 GMT
Server: ATS/3.3.1
Cache-Control: max-age=0
Content-Length: 496941
Content-Disposition: attachment; filename="bando-det-disat-carbone-2-699.pdf"; title="bando-det-disat-carbone-2-699.pdf"
[misc...]
Content-Type: 
Content-Language: en
Age: 0
[misc...]

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

This was regressed by bug 1428473. Confirmed via disabling the pref introduced by bug 1570658.

Sebastian, is there some way that we can add content-type sniffing back for document navigation?

Comment 2

[:Gijs (he/him)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #1)

This was regressed by bug 1428473. Confirmed via disabling the pref introduced by bug 1570658.

Sebastian, is there some way that we can add content-type sniffing back for document navigation?

The server also sends (but this was elided from comment #0):

X-Content-Type-Options: nosniff

Not sniffing for navigation when the server sends an explicit nosniff header is kind of the point of that bug...

Really, this is a server issue, they shouldn't send both nosniff and then send no content-type header at all, even for downloads.

However, I would have expected this to work based on our own knowledge of the file extension. nosniff should govern the in-docshell response, but I would expect us to still try to determine what app to use based on the file extension / mimetype. Jared, do you know why we don't get the right pdf mimetype here?

Comment 3

[Sebastian Streich [:sstreich]]

I would expect us to still try to determine what app to use based on the file extension / mimetype

With nosniff enabled we skip the Uri-extension sniffing as well - the only thing we sniff if it might be text/plain; otherwise the sniffer returns application/octet-stream. Also don't think we should re-enable this in the sniffing step, as an attacker could then simply control the content type via the Uri.

But i agree that if we are already downloading the file, we might aswell have a try sniffing the uri.
What We could do is Before Choosing the action maybe something like

if (channel->loadInfo()->GetSkipContentSniffing() && MIMEType.EqualsLiteral("application/octet-stream")) {
  nsCOMPtr<nsIMIMEService> mimeService(do_GetService("@mozilla.org/mime;1"));
  mimeService->GetTypeFromURI(mSourceUrl,MIMEType);
}

Or we might ignore nosniff if Content-Disposition is also set 🤔, in that case there is no way we would load stuff into the docshell, right?

Comment 4

[:Gijs (he/him)]

(In reply to Sebastian Streich [:sstreich] from comment #3)

Or we might ignore nosniff if Content-Disposition is also set 🤔, in that case there is no way we would load stuff into the docshell, right?

It depends what you mean - part of the issue highlighted in comment #0 is that we do not show the "open in Nightly" option. If that option was available and selected by the user, we would load the resulting document into a docshell (after saving to disk)... Still, it's true that we will no longer load this channel instance into a docshell anymore.

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

I guess I should have verified comment #0 didn't leave out any headers.

In nsExternalHelperAppService.cpp we check the file extension to see if we should put "Portable Document Format" as the file description.

We don't get the right PDF mimetype because the nosniff code skips the sniffing, and then we inspect the characters of the first 512 bytes and see that they are all 'char' (https://searchfox.org/mozilla-central/rev/027893497316897b8f292bde48dbb6da2391a331/netwerk/streamconv/converters/nsUnknownDecoder.cpp#716-720), which this file is, and then the mimetype gets set as text/plain. Once we have a known mimetype in the HelperAppDlg code, we don't check for a different mimetype.

Comment 6

[Anne (:annevk)]

Please also test <a href=somepdf download=pdf> whereby somepdf does not have Content-Disposition set to see if this needs to apply to downloads generally.

And please add a comment documenting these oddities to https://github.com/whatwg/mimesniff/issues/82.

Comment 7

[kernp25]

Attachment: firefox_nSE0u8OvYb.png

Comment 8

[kernp25]

It will also not show for this url: https://3c.gmx.net/mail/client/attachment/download/tatt0_1---tmai1625114336a812cb/;jsessionid=13B319533C1B89F6C88E4AE02F605DF7-n3.bs62b

Comment 9

[kernp25]

Response Headers:

HTTP/1.1 200 200
Date: Mon, 27 Jul 2020 12:07:14 GMT
Server: Apache
Access-Control-Allow-Origin: *
Access-Control-Allow-Methods: POST, GET, OPTIONS
Access-Control-Max-Age: 1728000
X-Content-Type-Options: nosniff
X-Robots-Tag: noindex, nofollow
Content-Security-Policy: frame-ancestors *
Expires: Tue, 27 Jul 2021 12:07:14 GMT
Cache-Control: private, public, max-age=31536000
Pragma: cache
Content-Disposition: attachment; filename="SHIPMENT_LABEL.PDF"; filename*=UTF-8''SHIPMENT_LABEL.PDF
Keep-Alive: timeout=3, max=99
Connection: Keep-Alive
Transfer-Encoding: chunked
Content-Type: application/pdf

Comment 10

[kernp25]

https://bugzilla.mozilla.org/show_bug.cgi?id=1655457

Comment 11

[giul.mus]

There was a further small regression since I posted this bug. Now the files are no longer recognized as PDFs in the "which is:" field, it says "plain text document" instead.

Comment 12

[:Gijs (he/him)]

The examples in this bug all show an "open in nightly" option for me on both nightly and 96 beta, so I think this now just works?

Comment 13

[kernp25]

The server returns now: content-type; name=bando-det-disat-carbone-2-699.pdf; charset=Windows-1252

Instead of: Content-Type:

Maybe the bug still happens?

Comment 14

[:Gijs (he/him)]

(In reply to kernp25 from comment #13)

The server returns now: content-type; name=bando-det-disat-carbone-2-699.pdf; charset=Windows-1252

Instead of: Content-Type:

Maybe the bug still happens?

I'm sorry, can you clarify what you mean? That looks like a content-disposition header rather than a content-type header?

Comment 15

[kernp25]

Attachment: manifest.zip

Test add-on to reproduce the bug.

Comment 16

[kernp25]

Attachment: 2Oek8HoMYU.png

What the server is now returning.

Comment 17

[kernp25]

(In reply to :Gijs (he/him) from comment #14)

(In reply to kernp25 from comment #13)

The server returns now: content-type; name=bando-det-disat-carbone-2-699.pdf; charset=Windows-1252

Instead of: Content-Type:

Maybe the bug still happens?

I'm sorry, can you clarify what you mean? That looks like a content-disposition header rather than a content-type header?

The bug title says: Empty Content-Type with XCTO: nosniff will not offer "Open in Nightly" for PDFs

But the server does not return a X-Content-Type-Options: nosniff header anymore. So that's why it works now for https://didattica.polito.it/pls/portal30/sviluppo.pkg_apply_admin.download_bando?p_id_bando=14836&p_tipo=bando. But the bug still happens if you add the header back.

STR:

1. Install the test add-on.
2. Navigate to https://didattica.polito.it/pls/portal30/sviluppo.pkg_apply_admin.download_bando?p_id_bando=14836&p_tipo=bando

Note: If you uncomment the line contentTypeHeader.value = "application/pdf"; then it works as expected.

Comment 18

[:Gijs (he/him)]

(In reply to kernp25 from comment #17)

The bug title says: Empty Content-Type with XCTO: nosniff will not offer "Open in Nightly" for PDFs

But the server does not return a X-Content-Type-Options: nosniff header anymore. So that's why it works now for https://didattica.polito.it/pls/portal30/sviluppo.pkg_apply_admin.download_bando?p_id_bando=14836&p_tipo=bando.

OK, thanks for clarifying. I'm not really convinced how severe this is. Websites need to learn not to send XCTO: nosniff without sending a meaningful content-type, and it seems like this website now has indeed learned that (to some degree... would still be better if the content of the Content-Type header wasn't just a copy of Content-Disposition which makes no semantic sense, but one step at a time...).

I'm pretty nervous about doing stuff here at this point, also because on Nightly we now show PDF.js immediately for Content-Disposition: attachment PDFs, so sniffing into that flow and showing arbitrary content as a PDF in PDF.js in nightly is potentially risky, if there were an issue with pdf.js - though tbf, if there were a PDF.js bug, presumably it could similarly be exploited by uploading a "real" PDF somewhere, and considering PDF.js doesn't share an origin with the server the PDF comes from, it's hard to imagine how this adds any realistic risk of exploitation.

Freddy, care to give a second opinion of what should happen here?

Either way, lowering priority because there's no assignee at this point and the number of public webpages doing this seems to have decreased.

Comment 19

[Frederik Braun [:freddy]]

@Gijs: Generally agree with your sentiment. There's only so much we can do to help pages with incorrect or incomplete headers. I'd rather not have us force things more aggressively into PDF on a whim.

@kernp25: Thank you for providing a add-on for testing. That's useful! (Though I was surprised you used an add-on specifically)

Comment 20

[Tom Schuster (MoCo)]

I came across this while looking at bug 1768069. Per the mime sniff spec were are not supposed to sniff PDF files when X-Content-Type-Options: no-sniff is set. The test URL from comment 0 doesn't seem to work anymore, because it doesn't use the no-sniff header anymore.

Comment 21

[Ashley Lowe]

Attachment: same problem

Thanks from templonaphy1974.