1294411 - Update XCTO: nosniff implementation to accept images which content type starts with 'image/'

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Description

[Christoph Kerschbaumer [:ckerschb]]

The spec [1] slightly changed and images should be allowed to load if the content type starts with 'image/'. E.g. loading a png with type 'image/foo' should be allowed to load.

[1] https://fetch.spec.whatwg.org/#x-content-type-options-header

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

Attachment: bug_1294411_xcto_should_allow_images_with_type_image.patch

Comment 2

[Daniel Veditz [:dveditz]]

Comment on attachment 8780081 [details] [diff] [review]
bug_1294411_xcto_should_allow_images_with_type_image.patch

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

This is fine. Apart from an extra console message the old code was effectively fine, too: either way we've already made the network load and are not displaying image/unknown. This does match the updated spec.

r=dveditz

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/faa7e3750138
Update XCTO: nosniff implementation to accept images which content type starts with 'image/'. r=dveditz

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8780081 [details] [diff] [review]
bug_1294411_xcto_should_allow_images_with_type_image.patch

Approval Request Comment
The spec for XCTO: nosniff changed and we should not block images that start with a mime type of "image/". The original implementation blocks all image loads which MIME type Firefox does not support.

[Feature/regressing bug #]:
Bug 471020 - Add X-Content-Type-Options: nosniff support to Firefox
[User impact if declined]:

[Describe test coverage new/current, TreeHerder]:
We have mochitests as well as web platform tests for the feature, they worked before the change and still work after this change.

[Risks and why]: 
low, in fact this change makes XCTO: nosniff less aggressive and blocks less images.

[String/UUID change made/needed]:
no

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/faa7e3750138

Comment 6

[Liz Henry (:lizzard) (relman/hg->git project)]

Does this affect 49? I notice bug 471020 landed for 50, so maybe you intended this for aurora (50) uplift.

Comment 7

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #6)
> Does this affect 49? I notice bug 471020 landed for 50, so maybe you
> intended this for aurora (50) uplift.

Liz, yes this is intended to be uplifted to aurora (50) - thanks for checking!

Comment 8

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8780081 [details] [diff] [review]
bug_1294411_xcto_should_allow_images_with_type_image.patch

Please see comment 4 and also comment 7.

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8780081 [details] [diff] [review]
bug_1294411_xcto_should_allow_images_with_type_image.patch

Makes Firefox more spec compliant, stabilized on Nightly for a few days, Aurora50+

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a03522ef5152