AlyoshaVasilieva / luminous-ttv

Avoid Twitch ads by grabbing video playlists from Russia
GNU General Public License v3.0
108 stars 7 forks source link

Real proxy IP can be exposed even when redacted_ip is enabled #15

Closed zGato closed 11 months ago

zGato commented 11 months ago

So, if you build the project using all the features, thus enabling the 1.1.1.1 redacted_ip, clients making a successful connection will have their m3u8 requests exposing the "1.1.1.1" IP.

The original IP is passed to Twitch, thus making that any failed request will expose the whole usher request. This has been tested on my own proxies and on yours. I won't explain specifically how to trigger this error, but it is possible.

This is not a critial issue, and perhaps not even consider making any change as you're using VPNs, so your proxies are not technically exposed.

(Just so you know it's possible, EU and EU2 is using Lithuania starting with 194. , it's obviously a VPN IP and AS is using China starting with 184., aswell a VPN, tho this provider also hosts dedi servers)

A way to handle this would be to just make 3xx (or greater) errors not expose any debug info, as it's currently doing so.

AlyoshaVasilieva commented 11 months ago

I can reproduce, thank you. As you noticed it's not a big deal for me since it just exposes VPN IPs, but for anyone self-hosting I might as well fix it. I will try to fix it without removing debug info first.

(The original rationale was mostly 'make DDoS by weirdos harder'.)

zGato commented 11 months ago

Yeah, the 1.1.1.1 redacted_ip module is really helpful, but in reality there's no one who will do a DDoS attack to a Twitch proxy, why would you in the first place haha.

I've already patched it on my end as I use nginx in front of the proxies, and there doesn't seem to be anyone publicly exposing their luminousttv instance anywhere, so I don't think it's really important.

AlyoshaVasilieva commented 11 months ago

Should be fixed by v0.5.5 / e99b311d7c3c1c0b1886fbe1e198354e5d31740c, going through CI now. Do you think it would be useful to preserve the query parameters like player_version, warp, player_backend and so on while only dropping the useless/leaky things like token? I didn't bother with it in this version since I don't have any logging enabled on my servers beyond what country Twitch has detected.

zGato commented 11 months ago

I don't have any other logging either. The token and sig paramenters IIRC are only used for VODs, and no one proxies those (except for mobile users, but I'm not entirely sure as I don't personally use those)

As otherwise tested here, luminousttv can be used with mobile platforms using the following syntax: https://eu.luminous.dev/playlist/$channel.m3u8%3Facmb%3Dtwitchlosers specifying acmb as Twitch requires that "token" but can be random.

So I don't feel it's required to remove those from the extension completely, but making them an optional module could be useful.

AlyoshaVasilieva commented 11 months ago

I do actually proxy VODs in my extension, but I suspect I'm the only person using it with everyone else using using TTV-LOL 1.x or 2 since that's much better to use.

Well, in my tests IP is no longer leaked in errors since query string is wiped, so I'll close this. Thanks again for the report.