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

Some URL can make httpx use URL with wrong info #2184

Closed
lebr0nli opened this issue Apr 21, 2022 · 11 comments
Closed

Some URL can make httpx use URL with wrong info #2184

lebr0nli opened this issue Apr 21, 2022 · 11 comments

Comments

@lebr0nli
Copy link
Contributor

lebr0nli commented Apr 21, 2022

After some research, I found that httpx.Client and httpx.Proxy may implicit parse wrong URL because of the improper implement of httpx.URL().copy_with.

And this issue may lead to some blacklist bypass.

For example:

  • httpx.Client
user_input_from_http_request = 'http:////admin-dashboard/secret'
u = httpx.URL(user_input_from_http_request)
assert u.host.lower() != 'admin-dashboard'
resp = httpx.Client(base_url=u).get('/') # SSRF to http://admin-dashboard/secret
print(resp.text) # sensitive data leak
  • httpx.Proxy
user_input_from_http_request = 'http://x@//internal-proxy:8082/'
u = httpx.URL(user_input_from_http_request)
assert u.host.lower() != 'internal-proxy'
# httpx.Proxy(u).url.netloc == b'internal-proxy:8082'
resp = httpx.Client(proxies=u).get('http://localhost/secret') # will request via http proxy at internal-proxy:8082

Main reason:

return URL(self._uri_reference.copy_with(**kwargs).unsplit())

copy_with parse self._uri_reference.copy_with(**kwargs).unsplit() before returning the new URL, but the new URL string return by unsplit may make some unintended changes on the new URL.

For example:

print(httpx.URL('http://[invalid string!!!!]//localhost/test!'). _uri_reference.unsplit())
# output: http://localhost/test!
u = httpx.URL('http://[invalid string!!!!]//localhost/test!').copy_with(scheme='https') # I only want to change scheme
print(u.host)
# output: localhost
# oops, host changed!

So if a function is using copy_with, it may have the same issue as httpx.Client and httpx.Proxy, too (For example, copy_set_param).

I also made a patch PR for this issue by replacing return URL(self._uri_reference.copy_with(**kwargs).unsplit()) to:

        new_url = URL(self)
        new_url._uri_reference = self._uri_reference.copy_with(**kwargs)
        if new_url.is_absolute_url:
            new_url._uri_reference = new_url._uri_reference.normalize()
        return URL(new_url)

By the way, I think this issue is similar to CWE-172 and CWE-20.

If you want to request a CVE id for this issue to remind httpx's user, you can use these categories.

Updated: This has been assigned as CVE-2021-41945.

@Kludex
Copy link
Member

Kludex commented May 2, 2022

@br3ndonland
Copy link

GitHub has added this to their advisory database, and it is now being picked up by Dependabot dependency scanning.

@dashk
Copy link

dashk commented May 7, 2022

👋 When is the next release scheduled? Would love to pick up #2185, since it addresses CVE-2021-41945 🙏

@tomchristie
Copy link
Member

Release pull request... #2214

@yan12125
Copy link

This may be useful for Linux distro packagers: I created a manual backport https://github.com/archlinux/svntogit-community/blob/packages/python-httpx/trunk/CVE-2021-41945.diff as commit e9b0c85 cannot be applied directly onto version 0.22.0.

@hugovk
Copy link
Contributor

hugovk commented May 16, 2022

PR #2185 has been merged, can this be closed?

@erdnaxeli
Copy link

A stable version with this fix has not been released yet, this can't be closed.

@mittomen
Copy link

This looks a bit odd to me that this critical vulnerability hasn't been released, even though the fix has been merged more than a month now. This project looks pretty nice, but then people must also pay attention on both resolving and releasing critical problems immediately. Such long-lasting response may turn users away from using your work as we are considering that within our organisation too...

@tomchristie
Copy link
Member

Now released as 0.23.0

@tomchristie
Copy link
Member

tomchristie commented May 23, 2022

If anyone knows the process for updating those advisories, then marking them as resolved would be very much welcome.

@Bibo-Joshi
Copy link

For GHSA-h8pj-cxx2-jfg2, there is a "Suggest improvements" which apparently opens a PR. Leaving that here as FYI since a PR was already submitted: github/advisory-database#322.

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

No branches or pull requests

10 participants