Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow whitelisting mediaTypes used in resources.GetRemote #10286

Closed
dirtymew opened this issue Sep 16, 2022 · 10 comments · Fixed by #10973
Closed

Allow whitelisting mediaTypes used in resources.GetRemote #10286

dirtymew opened this issue Sep 16, 2022 · 10 comments · Fixed by #10973
Assignees
Milestone

Comments

@dirtymew
Copy link
Contributor

What version of Hugo are you using (hugo version)?

$ hugo version
hugo v0.103.0+extended darwin/amd64 BuildDate=unknown

Does this issue reproduce with the latest release?

yes

Hi there

I got a build error - resources.GetRemote : failed to resolve media type for remote resource

while trying to get remote resources with content types of MS Office files:

  • .doc application/msword
  • .docx application/vnd.openxmlformats-officedocument.wordprocessingml.document
  • .xls application/vnd.ms-excel
  • .xlsx application/vnd.openxmlformats-officedocument.spreadsheetml.sheet
  • .ppt application/vnd.ms-powerpoint
  • .pptx application/vnd.openxmlformats-officedocument.presentationml.presentation

MediaType are also added to config file and with other additional MediaTypes like zip application/zip resources.GetRemote works ok. Trying to add Options with "Content-Type" to GetRemote request have no effect.

P.S.
also found mistake in docs
correct mime type for PowerPoint application/vnd.ms-powerpoint, not application/vnd.mspowerpoint

@cyberdean
Copy link

I encounter the same problem, on hugo 0.108.0, with image/avif media type. (.avif)

Adding it to config.yaml does not help (same error message)

mediaTypes:
  image/avif:
    suffixes:
      - avif

@bep bep changed the title resources.GetRemote: failed to resolve media type for remote resource Allow setting mediaType as an option in resources.GetRemote Dec 9, 2022
@bep
Copy link
Member

bep commented Dec 9, 2022

The mime TYPE detection in GetRemote is a little bit restrictive, we don't just look at the suffix. This comes from security concerns, but that means that in its current form there will always be reports like the above.

So I suggest that we allow setting the MediaType directly:

{{ $r := resources.GetRemote "https://example.org" (dict "mediaType" "application/msword") }}

@jmooring
Copy link
Member

jmooring commented Dec 9, 2022

@bep Would you consider this?

[security.http]
methods = ['(?i)GET|POST']
urls = ['.*']
mediaTypes = ['image/avif','^application/vnd.openxmlformats']

This would represent additional media types, so the naming may not be great.

@bep
Copy link
Member

bep commented Dec 9, 2022

@jmooring yea, that would probably be a better idea. I don't have a better name on the tip of my tongue.

@bep bep changed the title Allow setting mediaType as an option in resources.GetRemote Allow whitelisting mediaTypes used in resources.GetRemote Dec 9, 2022
@bep
Copy link
Member

bep commented May 19, 2023

@jmooring OK, thanks for bringing this to my attention. Looking back at now, I don't see why we couldn't simply do:

{{ $r := resources.GetRemote "https://example.org" (dict "mediaType" "application/msword") }}

I don't see any obvious security implications in the above???

@bep bep added this to the v0.112.0 milestone May 19, 2023
@jmooring
Copy link
Member

If we do this...

{{ $r := resources.GetRemote "https://example.org" (dict "mediaType" "application/msword") }}

...we would have to know the expected media type in the response header before making the request. That wouldn't work with this (contrived) example:

content/about.md

+++
title = 'About'
download = 'https://example.org/a.foo'
+++
{{ with .Params.download }}
  {{ if (urls.Parse .).IsAbs }}
    {{ with resources.GetRemote . }}
      <a href="{{ .RelPermalink }}" download>Download</a>
    {{ end }}
  {{ else }}
    ...
  {{ end }}
{{ end }}

So I'm back to...

[security.http]
methods = ['(?i)GET|POST']
urls = ['.*']
mediaTypes = ['image/avif','^application/vnd.openxmlformats']

or similar.

@bep
Copy link
Member

bep commented May 20, 2023

...we would have to know the expected media type in the response header before making the request. That wouldn't work with this (contrived) example:

OK, now I think I get it; so in the examples above:

  1. The response header is set to a sensible mime type.
  2. But the content sniffing fails to resolve to that same mime type?

Is that correct? So an additional security config would whitelist some types saying we trust the response header value?

@jmooring
Copy link
Member

Is that correct?

Yes.

So an additional security config would whitelist some types saying we trust the response header value?

Yes.

But I think we need to remember that, based on past behavior, someone will inevitably will do this:

[security.http]
mediaTypes = ['.*']

But if you aim for your own foot, I think you deserve the consequences.

@bep
Copy link
Member

bep commented May 20, 2023

But if you aim for your own foot, I think you deserve the consequences.

I said this before, but: The security config is there mainly to avoid ... spring-guns.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants