Do not allow forwarding of authorization headers on redirect. #89
Conversation
There is now a flag `auth_on_redirect` that can be set if you need to pass authorization headers. This is similar to https://curl.se/docs/CVE-2018-1000007.html and https://nvd.nist.gov/vuln/detail/CVE-2021-31879
|
Having this same issue with redirects and the HTTPrb client, maybe you can update the PR to include that client? |
|
which backend? from what i can tell the net_http backend is the only one that implements redirects as part of this library. I would assume that's a bug/issue for the client library |
|
Thanks for the pull request. It seems that this will apply only on |
|
Sure, I'll take a crack at it |
|
@janko does the fix for HTTPrb and HTTPX need to be done here or on their respective repos? |
lib/down/net_http.rb
Outdated
| uri.user = nil unless auth_on_redirect | ||
| uri.password = nil unless auth_on_redirect |
There was a problem hiding this comment.
do i need this? or asked another way, should credentials stay when it's a relative redirect?
There was a problem hiding this comment.
going to remove if it's not a relative redirect
| @@ -155,6 +157,10 @@ def open_uri(uri, options, follows_remaining:) | |||
| raise ResponseError.new("Invalid Redirect URI: #{exception.uri}", response: response) | |||
| end | |||
|
|
|||
| # do not leak credentials on redirect | |||
| options.delete("Authorization") unless auth_on_redirect | |||
| options.delete(:http_basic_authentication) unless auth_on_redirect | |||
There was a problem hiding this comment.
same question in a different place - should there be some logic to keep the creds if it's a relative redirect?
There was a problem hiding this comment.
there's not a good way to preserve on a relative redirect - i can check to see if the host is the same, but that doesn't help from a test perspective. for now, i'm keping the logic as-is.
… works for #open as #download does not have the same informtion
|
@janko would you mind taking another look at this pr? |
|
Fwiw httpx does this already (do try with a recent version). |
There is now a flag
auth_on_redirectthat can be set if you need to pass authorization headers. This is similar tohttps://curl.se/docs/CVE-2018-1000007.html
and
https://nvd.nist.gov/vuln/detail/CVE-2021-31879