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 to flatcar-3510 (LTS-2023) #1277

Merged
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
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,133 @@
From 92fd36dd54de9ac845549944692eb33c5aee7343 Mon Sep 17 00:00:00 2001
From: Jay Satiro <raysatiro@yahoo.com>
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,134 @@
From 021b69e6a42e12ea088c328c2363b2ddc6bf54de Mon Sep 17 00:00:00 2001
From: Sayan Chowdhury <schowdhury@microsoft.com>
Date: Mon, 16 Oct 2023 17:37:22 +0530
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

Signed-off-by: Sayan Chowdhury <schowdhury@microsoft.com>
---
lib/cookie.c | 13 +------------
lib/cookie.h | 13 ++++---------
lib/easy.c | 4 +---
3 files changed, 6 insertions(+), 24 deletions(-)

diff --git a/lib/cookie.c b/lib/cookie.c
index bccf2e888..30c1bf9cc 100644
--- a/lib/cookie.c
+++ b/lib/cookie.c
@@ -118,7 +118,6 @@ static void freecookie(struct Cookie *co)
free(co->name);
free(co->value);
free(co->maxage);
- free(co->version);
free(co);
}

@@ -717,11 +716,7 @@ Curl_cookie_add(struct Curl_easy *data,
}
}
else if(strcasecompare("version", name)) {
- strstore(&co->version, whatptr);
- if(!co->version) {
- badcookie = TRUE;
- break;
- }
+ /* just ignore */
}
else if(strcasecompare("max-age", name)) {
/*
@@ -1176,7 +1171,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 */
@@ -1240,9 +1234,6 @@ struct CookieInfo *Curl_cookie_init(struct Curl_easy *data,
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.
@@ -1396,7 +1387,6 @@ static struct Cookie *dup_cookie(struct Cookie *src)
CLONE(name);
CLONE(value);
CLONE(maxage);
- CLONE(version);
d->expires = src->expires;
d->tailmatch = src->tailmatch;
d->secure = src->secure;
@@ -1612,7 +1602,6 @@ void Curl_cookie_cleanup(struct CookieInfo *c)
{
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 --git a/lib/cookie.h b/lib/cookie.h
index abc0a2e8a..3b715a982 100644
--- a/lib/cookie.h
+++ b/lib/cookie.h
@@ -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 */
@@ -56,18 +52,17 @@ struct Cookie {
#define COOKIE_PREFIX__SECURE (1<<0)
#define COOKIE_PREFIX__HOST (1<<1)

-#define COOKIE_HASH_SIZE 256
+#define COOKIE_HASH_SIZE 63

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 --git a/lib/easy.c b/lib/easy.c
index d7f93be1e..c4a1ce390 100644
--- a/lib/easy.c
+++ b/lib/easy.c
@@ -905,9 +905,7 @@ struct Curl_easy *curl_easy_duphandle(struct Curl_easy *data)
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;
--
2.40.0

Loading