From fb4415d8aee6c1045be932a34fe6107c2f5ed147 Mon Sep 17 00:00:00 2001 From: Jay Satiro Date: Wed, 11 Oct 2023 07:34:19 +0200 Subject: [PATCH] socks: return error if hostname too long for remote resolve Prior to this change the state machine attempted to change the remote resolve to a local resolve if the hostname was longer than 255 characters. Unfortunately that did not work as intended and caused a security issue. Upstream-Status: Backport [https://github.com/curl/curl/commit/fb4415d8aee6c1045be932a34fe6107c2f5ed147] CVE: CVE-2023-38545 Signed-off-by: Archana Polampalli --- lib/socks.c | 8 +++--- tests/data/Makefile.inc | 2 +- tests/data/test722 | 64 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 5 deletions(-) create mode 100644 tests/data/test722 diff --git a/lib/socks.c b/lib/socks.c index a014aa6..2215c02 100644 --- a/lib/socks.c +++ b/lib/socks.c @@ -536,9 +536,9 @@ CURLproxycode Curl_SOCKS5(const char *proxy_user, /* RFC1928 chapter 5 specifies max 255 chars for domain name in packet */ if(!socks5_resolve_local && hostname_len > 255) { - infof(data, "SOCKS5: server resolving disabled for hostnames of " - "length > 255 [actual len=%zu]", hostname_len); - socks5_resolve_local = TRUE; + failf(data, "SOCKS5: the destination hostname is too long to be " + "resolved remotely by the proxy."); + return CURLPX_LONG_HOSTNAME; } if(auth & ~(CURLAUTH_BASIC | CURLAUTH_GSSAPI)) @@ -879,7 +879,7 @@ CURLproxycode Curl_SOCKS5(const char *proxy_user, } else { socksreq[len++] = 3; - socksreq[len++] = (char) hostname_len; /* one byte address length */ + socksreq[len++] = (unsigned char) hostname_len; /* one byte length */ memcpy(&socksreq[len], hostname, hostname_len); /* address w/o NULL */ len += hostname_len; } diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc index 3064b39..47117b6 100644 --- a/tests/data/Makefile.inc +++ b/tests/data/Makefile.inc @@ -99,7 +99,7 @@ test670 test671 test672 test673 test674 test675 test676 test677 test678 \ \ test700 test701 test702 test703 test704 test705 test706 test707 test708 \ test709 test710 test711 test712 test713 test714 test715 test716 test717 \ -test718 test719 test720 test721 \ +test718 test719 test720 test721 test722 \ \ test800 test801 test802 test803 test804 test805 test806 test807 test808 \ test809 test810 test811 test812 test813 test814 test815 test816 test817 \ diff --git a/tests/data/test722 b/tests/data/test722 new file mode 100644 index 0000000..05bcf28 --- /dev/null +++ b/tests/data/test722 @@ -0,0 +1,64 @@ + + + +HTTP +HTTP GET +SOCKS5 +SOCKS5h +followlocation + + + +# +# Server-side + +# The hostname in this redirect is 256 characters and too long (> 255) for +# SOCKS5 remote resolve. curl must return error CURLE_PROXY in this case. + +HTTP/1.1 301 Moved Permanently +Location: http://AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/ +Content-Length: 0 +Connection: close + + + + +# +# Client-side + + +proxy + + +http +socks5 + + +SOCKS5h with HTTP redirect to hostname too long + + +--no-progress-meter --location --proxy socks5h://%HOSTIP:%SOCKSPORT http://%HOSTIP:%HTTPPORT/%TESTNUMBER + + + +# +# Verify data after the test has been "shot" + + +GET /%TESTNUMBER HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +User-Agent: curl/%VERSION +Accept: */* + + + +97 + +# the error message is verified because error code CURLE_PROXY (97) may be +# returned for any number of reasons and we need to make sure it is +# specifically for the reason below so that we know the check is working. + +curl: (97) SOCKS5: the destination hostname is too long to be resolved remotely by the proxy. + + + -- 2.40.0