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

libcurl: Restrict redirect schemes #4094

Closed
wants to merge 3 commits into from

Conversation

linosgian
Copy link

All protocols except for CURLPROTO_FILE/CURLPROTO_SMB and their TLS counterpart were allowed for redirects. This vastly broadens the exploitation surface in case of a vulnerability such as SSRF,
where libcurl-based clients are forced to make requests to arbitrary (usually internal) hosts.

For instance, CURLPROTO_GOPHER can be used to smuggle any TCP-based
protocol by URL-encoding a payload in the URI. Gopher will open a TCP connection and send the payload.

An example of what an adversary can do looks like the following:
gopher://some.elasticsearch.node:9200/_DELETE%20/some_index%20HTTP%2F1.0%0A
What this will do is delete the selected (e.g. some_index) elasticsearch index from the elasticsearch node.

For more information about gopher and other protocols used in exploitation, refer to this.
Although the official documentation states some issues around these protocols, I believe that the convenience of allowing such redirects is outweighed by the importance they hold for exploitation.

This PR flips the blacklisting logic of only stating which protocols should be denied and makes this a whitelist of only HTTP/HTTPS and FTP/FTPS. This is also "future-proof", so that other newly supported protocols in the future won't bite any unsuspected user, through redirects.
All other protocols have to be explicitly enabled for redirects through CURLOPT_REDIR_PROTOCOLS.

For context there is already a discussion on the curl-library mailing list about this.

Awaiting your comments on this.

All protocols except for CURLPROTO_FILE/CURLPROTO_SMB and their TLS
counterpart were allowed for redirect. This vastly broadens the
exploitation surface in case of a vulnerability such as SSRF [1],
where libcurl-based clients are forced to make requests to arbitrary
hosts.

For instance, CURLPROTO_GOPHER can be used to smuggle any TCP-based
protocol by URL-encoding a payload in the URI. Gopher will open a TCP
connection and send the payload.

Only HTTP/HTTPS and FTP/FTPS are allowed. All other protocols have to be
explicitly enabled for redirects through CURLOPT_REDIR_PROTOCOLS.

[1]: https://www.acunetix.com/blog/articles/server-side-request-forgery-vulnerability/

Signed-off-by: Linos Giannopoulos <lgian@skroutz.gr>
@jay
Copy link
Member

jay commented Jul 5, 2019

Ref: https://curl.haxx.se/mail/lib-2019-07/0007.html

Though I'm typically don't-break-anything-ever I can't think of a use case for allowing, say, an http redirect to telnet:// or pop3:// etc. Regardless we should address gopher.

@danielgustafsson
Copy link
Member

I am very much in favor of whitelisting like this rather than the previous blacklisting approach.

lib/url.c Outdated
~(CURLPROTO_FILE | CURLPROTO_SCP | CURLPROTO_SMB |
CURLPROTO_SMBS);
set->redir_protocols = CURLPROTO_HTTP | CURLPROTO_HTTPS | CURLPROTO_FTP
| CURLPROTO_FTPS;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since no browser ever supported FTPS and since most FTPS services actually will require FTP + upgrade, I think we can leave out FTPS as well and only allow HTTP, HTTPS and FTP by default.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I've pushed a fixup reflecting your comment

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since no browser ever supported FTPS and since most FTPS services actually will require FTP + upgrade, I think we can leave out FTPS as well and only allow HTTP, HTTPS and FTP by default.

A little late on this but I disagree. curl allowed it and for all we know some server is redirecting to sftp or ftps for file transfer.

The testcase ensures that redirects to CURLPROTO_GOPHER won't be
allowed, by default, in the future. Also, curl is being used
for convenience while keeping the testcases DRY.

The expected error code is CURLE_UNSUPPORTED_PROTOCOL when the client is
redirected to CURLPROTO_GOPHER

Signed-off-by: Linos Giannopoulos <lgian@skroutz.gr>
@linosgian
Copy link
Author

I've also added a testcase to make sure that redirects to CURLPROTO_GOPHER won't be enabled (by default) again. We could do the same for several other protocols too.

@bagder
Copy link
Member

bagder commented Jul 14, 2019

Thanks!

@bagder bagder closed this in 6080ea0 Jul 14, 2019
jay added a commit that referenced this pull request Jul 17, 2019
- Allow FTPS on redirect.

- Update default allowed redirect protocols in documentation.

Follow-up to 6080ea0.

Ref: #4094

Closes #4115
caraitto pushed a commit to caraitto/curl that referenced this pull request Jul 23, 2019
All protocols except for CURLPROTO_FILE/CURLPROTO_SMB and their TLS
counterpart were allowed for redirect. This vastly broadens the
exploitation surface in case of a vulnerability such as SSRF [1], where
libcurl-based clients are forced to make requests to arbitrary hosts.

For instance, CURLPROTO_GOPHER can be used to smuggle any TCP-based
protocol by URL-encoding a payload in the URI. Gopher will open a TCP
connection and send the payload.

Only HTTP/HTTPS and FTP are allowed. All other protocols have to be
explicitly enabled for redirects through CURLOPT_REDIR_PROTOCOLS.

[1]: https://www.acunetix.com/blog/articles/server-side-request-forgery-vulnerability/

Signed-off-by: Linos Giannopoulos <lgian@skroutz.gr>

Closes curl#4094
caraitto pushed a commit to caraitto/curl that referenced this pull request Jul 23, 2019
- Allow FTPS on redirect.

- Update default allowed redirect protocols in documentation.

Follow-up to 6080ea0.

Ref: curl#4094

Closes curl#4115
@lock lock bot locked as resolved and limited conversation to collaborators Oct 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

4 participants