Skip to content
This repository was archived by the owner on Feb 26, 2025. It is now read-only.

Link mime-type notes to issue 105 (re CSP)#119

Closed
mikesamuel wants to merge 2 commits intoWICG:masterfrom
mikesamuel:patch-1
Closed

Link mime-type notes to issue 105 (re CSP)#119
mikesamuel wants to merge 2 commits intoWICG:masterfrom
mikesamuel:patch-1

Conversation

@mikesamuel
Copy link
Copy Markdown

This incorporates the conclusion at the end of #105 (comment)

We should probably not accept any JSON MIME type.

based on the assumptions made by existing hosting providers and JSON-producing web services as to what can be safely hosted on origins that appear on CSP source white lists.

This incorporates the conclusion at the end of WICG#105 (comment)

> We should probably not accept any JSON MIME type.

based on the assumptions made by existing hosting providers and JSON-producing web services as to what can be safely hosted on origins that appear on CSP source white lists.
Per @Malvoz's observation:

> It seems that the convention for MIME-type extensions would suggest
> application/importmap+json rather than application/json+importmap
> (src: https://www.iana.org/assignments/media-types/media-types.xhtml)
@ljharb
Copy link
Copy Markdown

ljharb commented May 3, 2019

If ti has its own mime type, should it have its own extension as well? (I’m aware browsers wouldn’t pay attention to the extension)

@mikesamuel
Copy link
Copy Markdown
Author

@ljharb Having a separate extension would not help security-wise.

I'd be worried about this scenario:

  1. a web service hosts uploaded files checks and uses mime.lookup to reject uploads of HTML, SVG, and JS files in accordance with recommendations.
  2. the web service uses mime.lookup] to compute the Content-type header when serving an uploaded file
  3. import-maps rolls out in browser with a .importmap extension
  4. The maintainer updates to a new version of mime.lookup which maps .importmap to application/importmap+json.
  5. An attacker uploads payload.importmap and exploits it via an injection vulnerability.

It's been best practice to not host third-party content on a sensitive origin for some time, but increasing the set of Content-type values that existing applications may unintentionally attach to hosted content is a risk factor.

@ljharb
Copy link
Copy Markdown

ljharb commented May 3, 2019

@mikesamuel an extension is the typical way that servers decide which mime type to serve a file with; i'm not suggesting it would help security-wise, but i don't see how it'd hurt. (given that browsers would, just like with everything else, ignore the extension and only switch on mime type)

@mikesamuel
Copy link
Copy Markdown
Author

@ljharb

given that browsers would, just like with everything else, ignore the extension and only switch on mime type

Major frameworks fill in Content-type based on extension, so these are not orthogonal concerns. For example, the widely used send package uses extension checking. I believe that ends up being used to compute the mime-type for a lot of express apps, at least those that use serve-static.

but i don't see how it'd hurt.

I was trying to explain how in the scenario I just outlined. Was that not clear? Not convincing?

I don't think it's a big enough problem to warrant not recommending an extension.
But I also don't want to complicate this PR with something that will probably involve a lot of bikeshedding.

@ljharb
Copy link
Copy Markdown

ljharb commented May 3, 2019

Fair that it doesn’t need to be in scope for the PR :-) but no, your example doesn’t seem any less problematic to me than if it uses the json extension.

@mikesamuel
Copy link
Copy Markdown
Author

@ljharb I don't think we disagree on any "... (should|must) ..." claims so unless you want me to expand on anything, have a great weekend.

@mikesamuel
Copy link
Copy Markdown
Author

Fair that it doesn’t need to be in scope for the PR :-) but no, your example doesn’t seem any less problematic to me than if it uses the json extension.

@ljharb
I think you're right. AFAICT, my cases all assume things I've assumed are outside the threat model I outlined at #105 (comment) : e.g. that there are few cases where an attacker can inject <script type=importmap ... but not <script>....
Sorry for the confusion.

domenic added a commit that referenced this pull request Jun 25, 2019
Now that the actual specification (spec.bs) has become more solid, we can clean up some tentative notes, and delete most of spec.md.

Closes #119 by including the appropriate explanatory text in the README, instead of in spec.md.
@domenic domenic closed this in #144 Jun 26, 2019
domenic added a commit that referenced this pull request Jun 26, 2019
Now that the actual specification (spec.bs) has become more solid, we can clean up some tentative notes, and delete most of spec.md.

Closes #119 by including the appropriate explanatory text in the README, instead of in spec.md.
hiroshige-g pushed a commit to hiroshige-g/import-maps that referenced this pull request Jul 11, 2019
Now that the actual specification (spec.bs) has become more solid, we can clean up some tentative notes, and delete most of spec.md.

Closes WICG#119 by including the appropriate explanatory text in the README, instead of in spec.md.
@hiroshige-g
Copy link
Copy Markdown
Collaborator

#105 (comment)

We should probably not accept any JSON MIME type.

@mikesamuel, Does this mean that "if a MIME type is accepted as a JSON, then it shouldn't be accepted as an import map (and vice versa)"?
If so, I noticed that application/importmap+json is a JSON MIME type and thus we should consider another MIME type.
https://mimesniff.spec.whatwg.org/#json-mime-type

(Or just meant that "not all JSON MIME types should be accepted as import maps"?)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants