You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
you have some program running behind your firewall (e.g., the code running Github, or Wordpress)
it wants to make an outgoing HTTP request, but the destination URL is at least somewhat under the control of an untrusted user (e.g., Github webhooks)
you want to make sure that user doesn't direct the request at some sensitive behind-the-firewall service and pwn you
so you have some check, like you forbid any URL that says localhost in it
but doing this correctly is extremely tricky, so everyone screws it up, and they get pwned.
The only correct place to do this check is after you've done the final conversion to a (normalized) IP address, and even there it has subtleties (e.g. sock.connect(0.0.0.0, port) seems to connect to localhost for me, and ipv4-mapped addresses are a thing that someone could potentially put in an AAAA record if they wanted to be perverse). If you're using open_tcp_stream – and we'd hope that HTTP clients on top of trio will use open_tcp_stream! – then that means that the checking has to happen inside open_tcp_stream.
One option would be to provide a forbidden_targets=... argument, that gives a list of ipaddress.IPv{4,6}Network objects, and for each resolved address checks if they fall into any of the given networks, and raises an error if so. The ipaddress module also provides mechanisms to check if an address is private, reserved, loopback, etc., which might be useful here.
Or I guess we could be even more minimal, and let the user pass in a validate_target=... function, that gets called on all the targets, and can raise an error or return False or whatever to indicate a bad target.
We should check with urllib3/requests maintainers whether this is something they would actually use.
The text was updated successfully, but these errors were encountered:
I was reading https://www.blackhat.com/docs/us-17/thursday/us-17-Tsai-A-New-Era-Of-SSRF-Exploiting-URL-Parser-In-Trending-Programming-Languages.pdf , and there are a few examples that follow this pattern:
localhost
in itThe only correct place to do this check is after you've done the final conversion to a (normalized) IP address, and even there it has subtleties (e.g.
sock.connect(0.0.0.0, port)
seems to connect to localhost for me, and ipv4-mapped addresses are a thing that someone could potentially put in an AAAA record if they wanted to be perverse). If you're usingopen_tcp_stream
– and we'd hope that HTTP clients on top of trio will useopen_tcp_stream
! – then that means that the checking has to happen insideopen_tcp_stream
.One option would be to provide a
forbidden_targets=...
argument, that gives a list ofipaddress.IPv{4,6}Network
objects, and for each resolved address checks if they fall into any of the given networks, and raises an error if so. Theipaddress
module also provides mechanisms to check if an address is private, reserved, loopback, etc., which might be useful here.Or I guess we could be even more minimal, and let the user pass in a
validate_target=...
function, that gets called on all the targets, and can raise an error or returnFalse
or whatever to indicate a bad target.We should check with urllib3/requests maintainers whether this is something they would actually use.
The text was updated successfully, but these errors were encountered: