Skip to content

Commit

Permalink
Address "CVE-2023-38545" in bundled curl library (v7.69).
Browse files Browse the repository at this point in the history
The patch is applied during the CMake preparation step, before compiling the bundled  curl source code.
Closes davix#107
  • Loading branch information
mpatrascoiu committed Oct 13, 2023
1 parent c3330db commit 8058b4b
Show file tree
Hide file tree
Showing 2 changed files with 141 additions and 1 deletion.
2 changes: 1 addition & 1 deletion cmake/modules/buildCurl.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ macro(buildCurl)
SOURCE_DIR "${CMAKE_SOURCE_DIR}/deps/curl"
BINARY_DIR "${CMAKE_BINARY_DIR}/deps/curl"
PREFIX "${CMAKE_BINARY_DIR}/deps/curl"
PATCH_COMMAND bash -c "(test -f .git && git checkout -q lib/setopt.c || true) && set -x && git apply ${CMAKE_SOURCE_DIR}/curl-CVE-2022-32221.patch"
PATCH_COMMAND bash -c "(test -f .git && git checkout -q lib/setopt.c lib/socks.c tests/data/Makefile.inc || true) && set -x && git apply ${CMAKE_SOURCE_DIR}/curl-CVE-2022-32221.patch ${CMAKE_SOURCE_DIR}/curl-CVE-2023-38545_7.69.0.patch"
CONFIGURE_COMMAND bash -c "${CMAKE_COMMAND} -DCMAKE_INSTALL_PREFIX=/usr/ -DCMAKE_INSTALL_LIBDIR=lib -DHTTP_ONLY=ON -DBUILD_CURL_EXE=OFF -DBUILD_TESTING=OFF -DBUILD_SHARED_LIBS=OFF -DCMAKE_POSITION_INDEPENDENT_CODE=ON -DCMAKE_USE_LIBSSH2=OFF ${SECURE_TRANSPORT_FLAGS} ${CMAKE_SOURCE_DIR}/deps/curl && ${CMAKE_SOURCE_DIR}/patch-curl-clock-gettime.sh"
BUILD_COMMAND make
INSTALL_COMMAND make DESTDIR=${CMAKE_BINARY_DIR}/deps/curl-install install
Expand Down
140 changes: 140 additions & 0 deletions curl-CVE-2023-38545_7.69.0.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
From c736c1e678d1b81d895281e419e59d9c4e853071 Mon Sep 17 00:00:00 2001
From: Jay Satiro <raysatiro@yahoo.com>
Date: Mon, 9 Oct 2023 15:37:31 -0400
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.

This patch applies to curl versions 7.69.0 - 7.72.0. Other versions
that are affected take a different patch. Refer to the CVE advisory
for more information.

Bug: https://curl.se/docs/CVE-2023-38545.html
---
lib/socks.c | 8 +++---
tests/data/Makefile.inc | 1 +
tests/data/test728 | 71 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 76 insertions(+), 4 deletions(-)
create mode 100644 tests/data/test728

diff --git a/lib/socks.c b/lib/socks.c
index 0fb97e1..74064fe 100644
--- a/lib/socks.c
+++ b/lib/socks.c
@@ -521,9 +521,9 @@ CURLcode 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(conn->data, "SOCKS5: server resolving disabled for hostnames of "
- "length > 255 [actual len=%zu]\n", hostname_len);
- socks5_resolve_local = TRUE;
+ failf(data, "SOCKS5: the destination hostname is too long to be "
+ "resolved remotely by the proxy.");
+ return CURLE_COULDNT_RESOLVE_HOST;
}

if(auth & ~(CURLAUTH_BASIC | CURLAUTH_GSSAPI))
@@ -835,7 +835,7 @@ CURLcode Curl_SOCKS5(const char *proxy_user,

if(!socks5_resolve_local) {
socksreq[len++] = 3; /* ATYP: domain name = 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;
infof(data, "SOCKS5 connect to %s:%d (remotely resolved)\n",
diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc
index dfc7432..1281c39 100644
--- a/tests/data/Makefile.inc
+++ b/tests/data/Makefile.inc
@@ -89,6 +89,7 @@ test662 test663 \
\
test700 test701 test702 test703 test704 test705 test706 test707 test708 \
test709 test710 test711 test712 test713 test714 test715 test716 test717 \
+test728 \
\
test800 test801 test802 test803 test804 test805 test806 test807 test808 \
test809 test810 test811 test812 test813 test814 test815 test816 test817 \
diff --git a/tests/data/test728 b/tests/data/test728
new file mode 100644
index 0000000..04dc2c7
--- /dev/null
+++ b/tests/data/test728
@@ -0,0 +1,71 @@
+<testcase>
+<info>
+<keywords>
+HTTP
+HTTP GET
+SOCKS5
+SOCKS5h
+followlocation
+</keywords>
+</info>
+
+#
+# Server-side
+<reply>
+# The hostname in this redirect is 256 characters and too long (> 255) for
+# SOCKS5 remote resolve. curl must return error CURLE_COULDNT_RESOLVE_HOST in
+# this case.
+<data>
+HTTP/1.1 301 Moved Permanently
+Location: http://AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/
+Content-Length: 0
+Connection: close
+
+</data>
+</reply>
+
+#
+# Client-side
+<client>
+<features>
+proxy
+</features>
+<server>
+http
+socks5
+</server>
+ <name>
+SOCKS5h with HTTP redirect to hostname too long
+ </name>
+ <command>
+--no-progress-meter --location --proxy socks5h://%HOSTIP:%SOCKSPORT http://%HOSTIP:%HTTPPORT/728
+</command>
+</client>
+
+#
+# Verify data after the test has been "shot"
+<verify>
+<strip>
+^User-Agent:.*
+</strip>
+<strippart>
+s/\r\n$/\n/
+</strippart>
+<protocol>
+GET /728 HTTP/1.1
+Host: %HOSTIP:%HTTPPORT
+Accept: */*
+
+</protocol>
+<errorcode>
+6
+</errorcode>
+# the error message is verified because error code
+# CURLE_COULDNT_RESOLVE_HOST (6) 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.
+<stderr mode="text">
+curl: (6) SOCKS5: the destination hostname is too long to be resolved remotely by the proxy.
+</stderr>
+</verify>
+</testcase>
--
2.7.4

0 comments on commit 8058b4b

Please sign in to comment.