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

Backport curl CVE fixes to stable #1269

Merged
merged 2 commits into from
Oct 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/security/2023-10-13-curl-backport.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- curl ([CVE-2023-38545](https://nvd.nist.gov/vuln/detail/CVE-2023-38545), [CVE-2023-38546](https://nvd.nist.gov/vuln/detail/CVE-2023-38546))
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
From 92fd36dd54de9ac845549944692eb33c5aee7343 Mon Sep 17 00:00:00 2001
From: Jay Satiro <[email protected]>
Date: Mon, 9 Oct 2023 17:15:44 -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.87.0 - 8.1.2. 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 | 2 +-
tests/data/test728 | 64 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 69 insertions(+), 5 deletions(-)
create mode 100644 tests/data/test728

diff --git a/lib/socks.c b/lib/socks.c
index d491e08..e7da5b4 100644
--- a/lib/socks.c
+++ b/lib/socks.c
@@ -539,9 +539,9 @@ static CURLproxycode do_SOCKS5(struct Curl_cfilter *cf,

/* 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))
@@ -882,7 +882,7 @@ static CURLproxycode do_SOCKS5(struct Curl_cfilter *cf,
}
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], sx->hostname, hostname_len); /* w/o NULL */
len += hostname_len;
}
diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc
index 3e0221a..64b11de 100644
--- a/tests/data/Makefile.inc
+++ b/tests/data/Makefile.inc
@@ -99,7 +99,7 @@ test679 test680 test681 test682 test683 test684 test685 \
\
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 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..05bcf28
--- /dev/null
+++ b/tests/data/test728
@@ -0,0 +1,64 @@
+<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_PROXY 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/%TESTNUMBER
+</command>
+</client>
+
+#
+# Verify data after the test has been "shot"
+<verify>
+<protocol crlf="yes">
+GET /%TESTNUMBER HTTP/1.1
+Host: %HOSTIP:%HTTPPORT
+User-Agent: curl/%VERSION
+Accept: */*
+
+</protocol>
+<errorcode>
+97
+</errorcode>
+# 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.
+<stderr mode="text">
+curl: (97) SOCKS5: the destination hostname is too long to be resolved remotely by the proxy.
+</stderr>
+</verify>
+</testcase>
--
2.7.4

Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
From 61275672b46d9abb3285740467b882e22ed75da8 Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <[email protected]>
Date: Thu, 14 Sep 2023 23:28:32 +0200
Subject: [PATCH] cookie: remove unnecessary struct fields

Plus: reduce the hash table size from 256 to 63. It seems unlikely to
make much of a speed difference for most use cases but saves 1.5KB of
data per instance.

Closes #11862
---
lib/cookie.c | 13 +------------
lib/cookie.h | 13 ++++---------
lib/easy.c | 4 +---
3 files changed, 6 insertions(+), 24 deletions(-)

diff -r -u -p curl-8.0.1/lib/cookie.c curl-8.0.1-patched/lib/cookie.c
--- curl-8.0.1/lib/cookie.c 2023-03-20 11:38:42.000000000 -0000
+++ curl-8.0.1-patched/lib/cookie.c 2023-10-13 11:42:44.820188193 -0000
@@ -119,7 +119,6 @@ static void freecookie(struct Cookie *co
free(co->name);
free(co->value);
free(co->maxage);
- free(co->version);
free(co);
}

@@ -726,11 +725,7 @@ Curl_cookie_add(struct Curl_easy *data,
}
}
else if((nlen == 7) && strncasecompare("version", namep, 7)) {
- strstore(&co->version, valuep, vlen);
- if(!co->version) {
- badcookie = TRUE;
- break;
- }
+ /* just ignore */
}
else if((nlen == 7) && strncasecompare("max-age", namep, 7)) {
/*
@@ -1174,7 +1169,6 @@ Curl_cookie_add(struct Curl_easy *data,
free(clist->path);
free(clist->spath);
free(clist->expirestr);
- free(clist->version);
free(clist->maxage);

*clist = *co; /* then store all the new data */
@@ -1238,9 +1232,6 @@ struct CookieInfo *Curl_cookie_init(stru
c = calloc(1, sizeof(struct CookieInfo));
if(!c)
return NULL; /* failed to get memory */
- c->filename = strdup(file?file:"none"); /* copy the name just in case */
- if(!c->filename)
- goto fail; /* failed to get memory */
/*
* Initialize the next_expiration time to signal that we don't have enough
* information yet.
@@ -1394,7 +1385,6 @@ static struct Cookie *dup_cookie(struct
CLONE(name);
CLONE(value);
CLONE(maxage);
- CLONE(version);
d->expires = src->expires;
d->tailmatch = src->tailmatch;
d->secure = src->secure;
@@ -1611,7 +1601,6 @@ void Curl_cookie_cleanup(struct CookieIn
{
if(c) {
unsigned int i;
- free(c->filename);
for(i = 0; i < COOKIE_HASH_SIZE; i++)
Curl_cookie_freelist(c->cookies[i]);
free(c); /* free the base struct as well */
diff -r -u -p curl-8.0.1/lib/cookie.h curl-8.0.1-patched/lib/cookie.h
--- curl-8.0.1/lib/cookie.h 2023-03-17 23:34:19.000000000 -0000
+++ curl-8.0.1-patched/lib/cookie.h 2023-10-13 11:47:39.693438491 -0000
@@ -36,11 +36,7 @@ struct Cookie {
char *domain; /* domain = <this> */
curl_off_t expires; /* expires = <this> */
char *expirestr; /* the plain text version */
-
- /* RFC 2109 keywords. Version=1 means 2109-compliant cookie sending */
- char *version; /* Version = <value> */
char *maxage; /* Max-Age = <value> */
-
bool tailmatch; /* whether we do tail-matching of the domain name */
bool secure; /* whether the 'secure' keyword was used */
bool livecookie; /* updated from a server, not a stored file */
@@ -61,13 +57,11 @@ struct Cookie {
struct CookieInfo {
/* linked list of cookies we know of */
struct Cookie *cookies[COOKIE_HASH_SIZE];
-
- char *filename; /* file we read from/write to */
- long numcookies; /* number of cookies in the "jar" */
+ curl_off_t next_expiration; /* the next time at which expiration happens */
+ int numcookies; /* number of cookies in the "jar" */
+ int lastct; /* last creation-time used in the jar */
bool running; /* state info, for cookie adding information */
bool newsession; /* new session, discard session cookies on load */
- int lastct; /* last creation-time used in the jar */
- curl_off_t next_expiration; /* the next time at which expiration happens */
};

/* This is the maximum line length we accept for a cookie line. RFC 2109
diff -r -u -p curl-8.0.1/lib/easy.c curl-8.0.1-patched/lib/easy.c
--- curl-8.0.1/lib/easy.c 2023-03-20 11:28:32.000000000 -0000
+++ curl-8.0.1-patched/lib/easy.c 2023-10-13 11:42:44.824188258 -0000
@@ -911,9 +911,7 @@ struct Curl_easy *curl_easy_duphandle(st
if(data->cookies) {
/* If cookies are enabled in the parent handle, we enable them
in the clone as well! */
- outcurl->cookies = Curl_cookie_init(data,
- data->cookies->filename,
- outcurl->cookies,
+ outcurl->cookies = Curl_cookie_init(data, NULL, outcurl->cookies,
data->set.cookiesession);
if(!outcurl->cookies)
goto fail;