878922 - imgLoader::GetMimeTypeFromContent should support SVG images

Status: RESOLVED WONTFIX

(Core :: Graphics: ImageLib, defect)

Description

[Seth Fowler [:seth] [:s2h]]

imgLoader::GetMimeTypeFromContent currently can't detect SVG files. See bug 867755 comment 35 and bug 366324 comment 14.

Comment 1

[Seth Fowler [:seth] [:s2h]]

Depending on bug 867755 since we decided not to block that bug on fixing this issue, but we want to take the changes in that bug into account.

Comment 2

[Masatoshi Kimura [:emk]]

We shouldn't. It can easily lead a security hole.
At least the sniffing behavior must be spec'ed. Currently the spec is saying nothing about sniffing SVG.
http://mimesniff.spec.whatwg.org/#sniffing-in-an-image-context

Comment 3

[Seth Fowler [:seth] [:s2h]]

You may well be right, but let's keep this bug open until we've had more discussion. The problems this causes are significant and we need to handle them in some way, even if this isn't the right way.

(In reply to Masatoshi Kimura [:emk] from comment #2)
> We shouldn't. It can easily lead a security hole.

Are there specific reasons why sniffing SVG is more likely to lead to a security hole than the other kinds of sniffing we already do? It's not obvious to me.

> At least the sniffing behavior must be spec'ed. Currently the spec is saying
> nothing about sniffing SVG.
> http://mimesniff.spec.whatwg.org/#sniffing-in-an-image-context

That's true. However, it shouldn't be an obstacle to implementation; if we want to propose this addition to the spec it's preferable to have an implementation to discuss. (It is however good reason to keep this behind a pref until it's specified.)

Comment 4

[Daniel Holbert [:dholbert]]

Also note that the algorithm that you linked to assumes we have a "supplied MIME type" available. IIUC, this bug is about cases where we *don't* have a supplied mime type. (which is why we're trying to sniff it)  So I don't think that algorithm in the spec is really applicable.

Comment 5

[Daniel Holbert [:dholbert]]

(At least, we don't have the supplied mime type for the favicon cache situation from bug 366324. Not sure about the other case.)

Comment 6

[Masatoshi Kimura [:emk]]

Oh, I only saw the former case (bug 867755 comment 35).
> the .sjs returns an SVG file, but says the Content-Type is image/png.
Obviously we should fix the .sjs in this case.

Comment 7

[Masatoshi Kimura [:emk]]

SVG will not be sniffed even in the unknown mime type case.
http://mimesniff.spec.whatwg.org/#rules-for-identifying-an-unknown-mime-type
I can't even find the word "svg" in the spec.
Please file a spec bug and convince a spec editor first.

Comment 8

[Gordon P. Hemsley [:GPHemsley]]

I could add an SVG sniffing algorithm (similar to the "rules for distinguishing if a resource is a feed or HTML") to the "image type pattern matching algorithm", but I would need assurances that that wouldn't introduce any security issues.

And it would need to be filed as a spec bug, as Masatoshi says.

(Note: Properly tagged SVG files are never sniffed because they have an XML type.)

Comment 9

[Anne (:annevk)]

I'm going to close this as WONTFIX per the comments from emk. emk is correct that if this is a problem in some subsystems in Firefox we should fix those subsystems instead. We should not expand the amount of code we have to sniff.