JordanMilne / Advocate

An SSRF-preventing wrapper around Python's requests library. Advocate is no longer maintained, please fork and rename if you would like to continue work on it.
Other
92 stars 17 forks source link

http://0177.0.0.1/ works when it shouldn't #18

Closed okyanusoz closed 2 years ago

okyanusoz commented 2 years ago

http://0177.0.0.1/ resolves to http://127.0.0.1, which is localhost. This module does not block this URL when it should.

ColdHeat commented 2 years ago

This issue is slightly alarmist. It is correct that http://0177.0.0.1/ isn't blocked but it doesn't seem to "work". Under Python 3.9.13:

>>> import advocate
>>> print(advocate.get("http://0177.0.0.1"))
0177.0.0.1 80
((<AddressFamily.AF_INET: 2>, <SocketKind.SOCK_STREAM: 1>, 6, '', (IPv4Address('177.0.0.1'), 80)),)
('177.0.0.1', 80)

As you can see this resolves down to 177.0.0.1 which isn't a private IP.

ColdHeat commented 2 years ago

I think since the decimal and hex versions of http://127.0.0.1 are blocked (2130706433, 0x7F000001), it would follow that the octal version should be blocked.

@spladug @JordanMilne are you still maintaining this library? Is it possible for someone at Reddit to maintain this since they make use of the lib? I might also be willing to maintain it if there's no one available.

JordanMilne commented 2 years ago

@okyanusoz Thanks for filing an issue! I'm not able to reproduce advocate.get("http://0177.0.0.1/") successfully connecting to localhost on my system, it raises a validation exception. Are you sure that advocate.get("http://0177.0.0.1/") is actually trying to connect to 127.0.0.1 on your system and not 177.0.0.1? What OS are you on, and how did you install Python? What does this output when pasted in a python shell?

import socket
socket.getaddrinfo("0177.0.0.1", 8000)
JordanMilne commented 2 years ago

Hi @ColdHeat, sorry for the delay here, unfortunately I haven't had much time to work on OSS recently :disappointed:. I'm in the middle of dropping support for Python 2 as part of cutting a 2.0 version of Advocate, and that's knocked loose a bunch of other TODOs like getting off of Travis and making the urllib3 patching less brittle. I expect to have a WIP branch up in the coming weeks though.

This issue is slightly alarmist. It is correct that http://0177.0.0.1/ isn't blocked but it doesn't seem to "work". [...] As you can see this resolves down to 177.0.0.1 which isn't a private IP.

Yep, though depending on your OS and what libc you're using, 0177.0.0.1 may either be interpreted as octal or decimal with a leading 0. On my system, the getaddrinfo() impl treats it as an octal address, so the socket.getaddrinfo() call returns 127.0.0.1 and advocate blocks it appropriately:

>>> import advocate
>>> advocate.get("http://0177.0.0.1")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/src/advocate/advocate/api.py", line 108, in get
    return request('get', url, **kwargs)
  File "/src/advocate/advocate/api.py", line 94, in request
    response = sess.request(method=method, url=url, **kwargs)
  File "/src/.virtualenvs/advocate/lib/python3.10/site-packages/requests/sessions.py", line 587, in request
    resp = self.send(prep, **send_kwargs)
  File "/src/.virtualenvs/advocate/lib/python3.10/site-packages/requests/sessions.py", line 701, in send
    r = adapter.send(request, **kwargs)
  File "/src/.virtualenvs/advocate/lib/python3.10/site-packages/requests/adapters.py", line 489, in send
    resp = conn.urlopen(
  File "/src/.virtualenvs/advocate/lib/python3.10/site-packages/urllib3/connectionpool.py", line 703, in urlopen
    httplib_response = self._make_request(
  File "/src/.virtualenvs/advocate/lib/python3.10/site-packages/urllib3/connectionpool.py", line 398, in _make_request
    conn.request(method, url, **httplib_request_kw)
  File "/src/.virtualenvs/advocate/lib/python3.10/site-packages/urllib3/connection.py", line 239, in request
    super(HTTPConnection, self).request(method, url, body=body, headers=headers)
  File "/usr/lib/python3.10/http/client.py", line 1282, in request
    self._send_request(method, url, body, headers, encode_chunked)
  File "/usr/lib/python3.10/http/client.py", line 1328, in _send_request
    self.endheaders(body, encode_chunked=encode_chunked)
  File "/usr/lib/python3.10/http/client.py", line 1277, in endheaders
    self._send_output(message_body, encode_chunked=encode_chunked)
  File "/usr/lib/python3.10/http/client.py", line 1037, in _send_output
    self.send(msg)
  File "/usr/lib/python3.10/http/client.py", line 975, in send
    self.connect()
  File "/src/.virtualenvs/advocate/lib/python3.10/site-packages/urllib3/connection.py", line 205, in connect
    conn = self._new_conn()
  File "/src/advocate/advocate/connection.py", line 152, in _validating_new_conn
    conn = conn_func(
  File "/src/advocate/advocate/connection.py", line 123, in validating_create_connection
    raise err
advocate.exceptions.UnacceptableAddressException: ('0177.0.0.1', 80)

>>> import socket
>>> socket.getaddrinfo("0177.0.0.1", 8000)
[(<AddressFamily.AF_INET: 2>, <SocketKind.SOCK_STREAM: 1>, 6, '', ('127.0.0.1', 8000)), (<AddressFamily.AF_INET: 2>, <SocketKind.SOCK_DGRAM: 2>, 17, '', ('127.0.0.1', 8000)), (<AddressFamily.AF_INET: 2>, <SocketKind.SOCK_RAW: 3>, 0, '', ('127.0.0.1', 8000))]
# ^ 0177.0.0.1 is equivalent to 127.0.0.1 on this system, so advocate refuses to connect.

I think since the decimal and hex versions of http://127.0.0.1 are blocked (2130706433, 0x7F000001), it would follow that the octal version should be blocked.

So Advocate doesn't really know anything about IP encodings, it just takes whatever host is provided as part of the URL and hands it off to socket.getaddrinfo(), which will do any DNS lookups and IP canonicalization as appropriate. It can't even really tell the difference between an IP or a hostname in the host portion of a URI.

Rather than play whack-a-mole with all the different possible IP representations (https://android.googlesource.com/platform/bionic/+/master/tests/arpa_inet_test.cpp#27 has a handful for musl libc, I'm sure there are more depending on the system,) we just focus on addresses that we know resolve to a banned address at the point the connection is being made per getaddrinfo(). Barring something truly bizarre, although all bets are off with computers, the IP returned by getaddrinfo() should be unambiguous, and that's ultimately what we use for validation and connection.

JordanMilne commented 2 years ago

Closing since this doesn't appear to be reproducible, please re-open if you have more details that could help reproduce!

For future readers, there are several system configurations (like OS X, perhaps on Linux with an unusual libc) where "0177.0.0.1" will resolve to "177.0.0.1" rather than "127.0.0.1". advocate will only block requests to "0177.0.0.1" on systems where it resolves to "127.0.0.1" because, by design, advocate only checks what an address resolves to at the point the request is being made.