Conversation
|
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? |
As far as I know, no. 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 |
|
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 |
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.
To override other header, the usual way is by sending 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): invidious/src/invidious/videos.cr Lines 362 to 374 in 0c07e9d |
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.


Currently, each invidious process is fixed to it's
domainconfig 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
domainto the Tor address, and the same goes for I2P.On some places,
HOST_URLis still used and it shouldn't be changed, like for example:invidious/src/invidious/helpers/utils.cr
Line 297 in adcdb8c
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.