Skip to content

Replace HOST_URL by Host header #5195

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

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Conversation

Fijxu
Copy link
Member

@Fijxu Fijxu commented Feb 28, 2025

Currently, each invidious process is fixed to it's domain config value which makes it really difficult for anyone to host Invidious using multiple domains, Tor and I2P.

If someone wants to host Invidious on Tor, a dedicated Invidious instance for it needs to be made setting domain to the Tor address, and the same goes for I2P.

On some places, HOST_URL is still used and it shouldn't be changed, like for example:

"hub.callback" => "#{HOST_URL}/feed/webhook/v1:#{time}:#{nonce}:#{OpenSSL::HMAC.hexdigest(:sha1, key, signature)}",

If this get merged when it's finished, it shouldn't break the setup of anyone using NGINX since the invidious documentation already mentions the use of the Host header on the NGINX configuration: https://github.com/iv-org/documentation/blob/65ecfccb10f15b4b9ce95c03b0d6d6ebd88bca7a/docs/nginx.md?plain=1#L24

Consider this a WIP since it's a big change that could break a lot of things, however, most of the changes made on the first commit are already applied to my Invidious fork at https://git.nadeko.net/Fijxu/invidious for a very very long time already, multiple domains works fine and Tor works fine.

@FibreFoX
Copy link

Just asking out of curiosity: This would change something from a know source to an "attacker controlled" source, right? Does this need any additional sanitizing?
Would env.response.headers["Location"] = "#{env.request.headers["Host"]}/api/v1/auth/playlists/#{playlist.id}" then be open to some kind of header injection attack via adding newlines (and therefor foreign headers) or something?

@Fijxu
Copy link
Member Author

Fijxu commented Mar 28, 2025

Just asking out of curiosity: This would change something from a know source to an "attacker controlled" source, right? Does this need any additional sanitizing? Would env.response.headers["Location"] = "#{env.request.headers["Host"]}/api/v1/auth/playlists/#{playlist.id}" then be open to some kind of header injection attack via adding newlines (and therefor foreign headers) or something?

As far as I know, no. env.request.headers["Host"] is a string and imagine you do a request to Invidious with a custom Host header, if you try to set any input on it like -H "Host: �e(�����-�J���hc��u�9���͹��{��1���bE��XQ�", you will only get an invalid URL (an URL that is not pointing to any Invidious host for example) of the input of the Host header and the webserver will return to you 404 Not Found (or any other error related to a non valid Host).

I may be wrong, but I can't think of any way to exploit this.

Ref: https://crystal-lang.org/reference/1.15/tutorials/basics/40_strings.html#interpolation
Example:
image

@FibreFoX
Copy link

I was more thinking of something like having a newline in there :) not just invalid binary stuff, just saw that "external controlled" data gets written directly back, just wanted to make sure

@Fijxu
Copy link
Member Author

Fijxu commented Mar 28, 2025

I was more thinking of something like having a newline in there :)

I see, Crystal handles it well

image

Fijxu added 2 commits March 28, 2025 16:37
Having to check if the cookie is inside a list of allowed domains on
invidious doesn't seem really useful because a reverse proxy like NGINX
and HAProxy will only send the client request to Invidious if the Host header
that the client sent to the server, matches with the `hdr(host)` (haproxy) or
`server_name` (nginx) set by the server configuration.
@SamantazFox
Copy link
Member

SamantazFox commented Apr 5, 2025

Does this need any additional sanitizing?

From my previous research on this subject, many implementations have a "trust 'Host' header" option for that purpose, as well as a "Trust the first N headers" to allow more than one level of proxying, and make sure that none of the attacker-controlled headers gets parsed.

Though some sanitization never hurts.

I may be wrong, but I can't think of any way to exploit this.

To override other header, the usual way is by sending Cookie: FOO=bar\nHost: evil.com.

But you can also do some XSS injections!

@Fijxu Some places like the thumbnails would require a significative rewrite, as they never see the HTTP context of the request (and worse, it ends up in the cache):

def build_thumbnails(id)
return {
{host: HOST_URL, height: 720, width: 1280, name: "maxres", url: "maxres"},
{host: HOST_URL, height: 720, width: 1280, name: "maxresdefault", url: "maxresdefault"},
{host: HOST_URL, height: 480, width: 640, name: "sddefault", url: "sddefault"},
{host: HOST_URL, height: 360, width: 480, name: "high", url: "hqdefault"},
{host: HOST_URL, height: 180, width: 320, name: "medium", url: "mqdefault"},
{host: HOST_URL, height: 90, width: 120, name: "default", url: "default"},
{host: HOST_URL, height: 90, width: 120, name: "start", url: "1"},
{host: HOST_URL, height: 90, width: 120, name: "middle", url: "2"},
{host: HOST_URL, height: 90, width: 120, name: "end", url: "3"},
}
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants