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

hostname tests are skipped due to incorrect type of `canonname` #26

Open dealbreaker973 opened 1 year ago

dealbreaker973 commented 1 year ago

Hi, when I try to run the pytest, I noticed that hostname-related tests are skipped even when the nameserver supports the AI_CANONNAME flag. This was possibly due to the incorrect type of the expected result when testing for the AI_CANONNAME flag: https://github.com/JordanMilne/Advocate/blob/master/test/test_advocate.py#L55

The cannonname returned by socket.getaddrinfo is a string now. ref: https://docs.python.org/3/library/socket.html#socket.getaddrinfo

When running hostname-related tests, HostnameTests.test_idn failed:

======================================= FAILURES =======================================
________________________________ HostnameTests.test_idn ________________________________
self = <test.test_advocate.HostnameTests testMethod=test_idn>

    def test_idn(self):
        # test some basic globs
>       self.assertFalse(self._is_hostname_allowed(
            u"中国.example.org",
            fake_lookup=True,
            hostname_blacklist={"*.org"}
        ))

test/test_advocate.py:359:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
test/test_advocate.py:350: in _is_hostname_allowed
    if validator.is_addrinfo_allowed(res):
advocate/addrvalidator.py:58: in wrapper
    return func(self, *args, **kwargs)
advocate/addrvalidator.py:319: in is_addrinfo_allowed
    if not self.is_hostname_allowed(canonname):
advocate/addrvalidator.py:259: in is_hostname_allowed
    if self._hostname_matches_pattern(hostname, pattern):
advocate/addrvalidator.py:221: in _hostname_matches_pattern
    hostname = canonicalize_hostname(hostname)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

hostname = b'xn--fiqs8s.example.org'

    def canonicalize_hostname(hostname):
        """Lowercase and punycodify a hostname"""
        # We do the lowercasing after IDNA encoding because we only want to
        # lowercase the *ASCII* chars.
        # TODO: The differences between IDNA2003 and IDNA2008 might be relevant
        # to us, but both specs are damn confusing.
>       return str(hostname.encode("idna").lower(), 'utf-8')
E       AttributeError: 'bytes' object has no attribute 'encode'

advocate/addrvalidator.py:23: AttributeError
=============================== short test summary info ================================
FAILED test/test_advocate.py::HostnameTests::test_idn - AttributeError: 'bytes' object has no attribute 'encode'

Similar to the cannoname bug, this is also introduced by feeding bytes as the cannoname in the _is_hostname_allowed function: https://github.com/JordanMilne/Advocate/blob/master/test/test_advocate.py#L346

I can make a PR later for a quick fix of the issues mentioned above