1662795 - The Open file prompt does not have the option to "Open in Firefox" SVG files with the application/octet-stream content-type

Status: VERIFIED FIXED

(Firefox :: Downloads Panel, defect, P1)

Description

[Rares Doghi, Desktop QA]

[Affected versions]:
Nightly 81.0b5

[Affected platforms]:
Platforms: Windows, Ubuntu, Mac OS

[Preconditions]
Reach about:config and set the following pref: browser.download.viewableInternally.enabledTypes = SVG,XML,PDF,WEBP,AVIF

[Steps to reproduce]:

1. Launch the Firefox browser and reach https://mime.ty.ax/
2. From the preBuilt section select the .svg application/octet-stream link.

Expected Result:
The Open file prompt should have the option to "Open files in Firefox"

Actual Results:
The "Open in Firefox" option is missing from the Open File prompt

Comment 1

[Rares Doghi, Desktop QA]

I think the severity for this issue should be an S3 since the option to Open in Firefox is missing from the file prompt.

Comment 2

[Robert Longson [:longsonr]]

treating files that are octet-stream as anything other than a binary blob has led to no end of security issues in the past. Are we really sure we want to do this?

Comment 3

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

(In reply to Robert Longson [:longsonr] from comment #2)

treating files that are octet-stream as anything other than a binary blob has led to no end of security issues in the past. Are we really sure we want to do this?

This is about what to do with these files after saving them locally. We normally offer to launch them in an external viewer, based generally just on the extension. For application/octet-stream .pdf we offer the ability to view it internally from a file:// URL instead, and bug 1639067 extended that to SVG and a few other types. Are the issues you're thinking of present for files on the local filesystem as well?

I'll look into why the file type detection isn't showing the radio button here.

Comment 4

[Robert Longson [:longsonr]]

If you load an SVG file from the filesystem and that SVG file contains script within the file itself then yes, browsers will run that script.

Comment 5

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

The check for the radio button uses the MIME type alone, it doesn't also pass in the primary extension, which in the case of application/octet-stream stays as the original file's extension (not sure where the logic for this is). So this can be fixed by passing in primaryExtension.

But now I'm not so sure if this is wise from a security perspective. It was always possible to open a .svg on the filesystem in Firefox, but this would make it much easier to do when served as application/octet-stream.

The primary point of bug 1639067 was to enable viewing downloaded files internally when clicked from the download list, this now works for application/octet-stream: by design, the launch check uses either MIME type or extension.

This feature has a pref that allows changing the set of types (MIME type + extension) treated this way, so given that SVG can contain JS maybe it's a bigger risk than the others? XML may be another one, XHTML or SVG also run off the filesystem with .xml. It would be good to use when content-type is specified, though, and I wouldn't want to break the old support for application/octet-stream PDFs, so this would have to be reworked to not use extensions in some cases.

:longsonr, would you argue that we shouldn't be making it easy to open .svg in Firefox based only on the extension by either of these user interface methods: "Open in Firefox" radio button in unknown content type dialog, activate from in download list? Or just the former since that's easier to fool someone into just ok-ing through (which would mean leaving the situation as-is)? :Gijs, what do you think?

Comment 6

[Robert Longson [:longsonr]]

What I was mainly concerned about initially was having an SVG image served as octet-stream being displayed on an HTML page. We should add a check that we don't do that unless we're sure the new code can't possibly enable that.

SVG is simply XML so with the right namespace that can contain XHTML or SVG either of which can run script. I'd be just as wary of making that work with unknown content. PDF is inert as far as I'm aware and all the raster image types are inert.

Are we doing this for Chrome compatibility? If so what does it do when it downloads xml or svg extensions? If it does display them directly, does it run script in them?

If somehow we forced the SVG to be displayed as an image rather than as a main document that would stop it running script so that would be much better. Not sure how feasible that is given that if you load an SVG from the filesystem it's going to be treated as a document rather than an image but I guess you could, with sufficient effort construct an img element or use a background-image to display the document.

Maybe I'm worrying over nothing, after all even with script it's certainly limited the amount of maliciousness you can cause here.

Comment 7

[:Gijs (he/him)]

Given our changes to our file same-origin model last year (bug 803143) there isn't a lot script from such a file could do in this case that it couldn't do already. The application/octet-stream part also doesn't really offer any protection here (ie if I were a malicious attacker I could serve the file with image/svg+xml if that made us offer this behaviour - the scripts would run either way as the mimetype from the http server doesn't influence our behaviour once we've downloaded the file to disk), and yes, AIUI other browsers already associate with the same file types.

Freddy and I already discussed this type of attack vector on slack a few days back, and we're comfortable with the level of risk.

If we wanted to mitigate this further, we could ensure that svg files loaded on their own (from the file system) get put in ImageDocument the same way raster images are (in fact, I'm not 100% sure why they aren't already?), in which case we wouldn't run script - though of course that would break people who are deliberately using script in their SVG files. Not sure how large that group is and if we therefore care. Anyway, that's a separate bug. Even if we did that, similar concerns would apply to xml and html files and we can't mitigate those in the same way, so I don't think the mitigation for SVG is high priority or needs to block moving forward with this bug.

(In reply to Robert Longson [:longsonr] from comment #2)

treating files that are octet-stream as anything other than a binary blob has led to no end of security issues in the past. Are we really sure we want to do this?

I think this is true for executables (esp. exe/bat/cmd files on Windows) but "load this document into the browser" is not quite as scary, as a determined attacker could probably do the same on the web, without having the user jump through the download hoop. :-)

Comment 8

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Ok, I'll limit this to just fixing the issue from the summary for now, thanks all for thinking through this with me.

Comment 9

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: Bug 1662795 - Use extension to decide whether to offer view internally. r?Gijs!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4c7f58f8b662
Use extension to decide whether to offer view internally. r=Gijs

Comment 11

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/4c7f58f8b662

Comment 12

[Rares Doghi, Desktop QA]

This issue is verified as fixed in our Latest Beta build 82.0b2 on Windows 10, Mac OSx 10.15 and Ubuntu