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-3033 (LTS-2022) #1278

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
1 change: 1 addition & 0 deletions changelog/security/2023-10-16-2023-10-16-curl-backports.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
Expand Up @@ -99,6 +99,9 @@ MULTILIB_CHOST_TOOLS=(
PATCHES=(
"${FILESDIR}"/${PN}-7.30.0-prefix.patch
"${FILESDIR}"/${PN}-respect-cflags-3.patch

"${FILESDIR}"/${P}-CVE-2023-38545.patch
"${FILESDIR}"/${P}-CVE-2023-38546.patch
)

src_prepare() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
From e478fb5f5f2e26e2b060a51939dd2746041459ab Mon Sep 17 00:00:00 2001
From: Jay Satiro <raysatiro@yahoo.com>
Date: Mon, 9 Oct 2023 16:34:27 -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.78.0 - 7.80.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 | 2 +-
tests/data/test728 | 67 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 72 insertions(+), 5 deletions(-)
create mode 100644 tests/data/test728

diff --git a/lib/socks.c b/lib/socks.c
index 91c4223..222660c 100644
--- a/lib/socks.c
+++ b/lib/socks.c
@@ -535,9 +535,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))
@@ -857,7 +857,7 @@ CURLproxycode 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)",
diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc
index 1636969..1067005 100644
--- a/tests/data/Makefile.inc
+++ b/tests/data/Makefile.inc
@@ -94,7 +94,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 \
+test718 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..8673613
--- /dev/null
+++ b/tests/data/test728
@@ -0,0 +1,67 @@
+<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>
+<strippart>
+s/\r\n$/\n/
+</strippart>
+<protocol>
+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 be1d056dba65e1ffd14242cd8caa688ce880ab49 Mon Sep 17 00:00:00 2001
From: Sayan Chowdhury <schowdhury@microsoft.com>
Date: Fri, 13 Oct 2023 20:54:44 +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 | 14 ++++----------
lib/easy.c | 4 +---
3 files changed, 6 insertions(+), 25 deletions(-)

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

@@ -666,11 +665,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)) {
/*
@@ -1091,7 +1086,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 */
@@ -1169,9 +1163,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.
@@ -1322,7 +1313,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;
@@ -1532,7 +1522,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 0ffe08e63..0b0c0cfe8 100644
--- a/lib/cookie.h
+++ b/lib/cookie.h
@@ -34,11 +34,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 */
@@ -54,18 +50,16 @@ 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 2aca93845..812a0dc36 100644
--- a/lib/easy.c
+++ b/lib/easy.c
@@ -848,9 +848,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