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

Block direct requests to private IPs #529

Merged
merged 4 commits into from
Sep 13, 2023

Conversation

M4tthewDE
Copy link
Contributor

@M4tthewDE M4tthewDE commented Sep 11, 2023

Pull request checklist:

  • CHANGELOG.md was updated, if applicable

Description

@M4tthewDE M4tthewDE changed the title Block private IPs Block requests to private IPs Sep 11, 2023
Copy link
Contributor

@KararTY KararTY left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Mm2PL
Copy link
Contributor

Mm2PL commented Sep 11, 2023

This will still resolve hosts pointed to by DNS records. For example if we'd have an A record saying localhost.example.com -> 127.0.0.1, this won't ignore 127.0.0.1.

@M4tthewDE
Copy link
Contributor Author

That should be handled via the ignored hosts imo. It might be worthwhile to add a config option for ignored hosts, since as of now they can only be added by patching the source.

@pajlada
Copy link
Member

pajlada commented Sep 11, 2023

Go's HTTP client has a way to customize parts of the transport layer, see https://pkg.go.dev/net/http#Client and https://pkg.go.dev/net/http#RoundTripper - with this we should be able to intercept the request after the domain resolving has happened & stop it mid-transit instead
Happy to discuss if this feels like a bad idea

@M4tthewDE
Copy link
Contributor Author

I was not able to find a way to intercept requests after the domain resolving. If this is not feasible, we could instead make the ignored hosts configurable and maybe add common hosts (e.g. localhost) as a default entry.

@pajlada pajlada changed the title Block requests to private IPs Block direct requests to private IPs Sep 13, 2023
Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good first effort, so I will go ahead and merge this in as-is, but I won't close #480
Thanks!

@pajlada pajlada enabled auto-merge (squash) September 13, 2023 19:12
@pajlada pajlada merged commit 6716075 into Chatterino:master Sep 13, 2023
8 checks passed
@M4tthewDE M4tthewDE deleted the feature/block-private-ips branch September 13, 2023 19:51
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.

4 participants