[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1053781: marked as done (unblock: curl/8.3.0-3)

Your message dated Thu, 12 Oct 2023 08:57:45 +0200
with message-id <af5fc4e8-74f6-494d-8a9a-657474638e76@debian.org>
and subject line Re: Bug#1053781: unblock: curl/8.3.0-3
has caused the Debian Bug report #1053781,
regarding unblock: curl/8.3.0-3
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
1053781: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1053781
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message --- 
Package: release.debian.org
Control: affects -1 + src:curl
X-Debbugs-Cc: curl@packages.debian.org
User: release.debian.org@packages.debian.org
Usertags: unblock
X-Debbugs-Cc: sergiodj@debian.org, charlesmelara@riseup.net, samueloph@debian.org
Severity: important

Please unblock package curl

[ Reason ]
The new package on unstable is fixing two just released CVEs, one low and one
high severity.

The high severity CVE can void the privacy of proxy users, by allowing remote
servers to trigger a local DNS resolution on the user's side.

These 2 CVEs have been fixed on bullseye-security, bullseye-backports,
bookworm-security, bookworm-backports and unstable.
I would like the fix to migrate to testing before running all CI tests and
waiting for the bake period.

[ Impact ]
The fix of the privacy-breaching CVE takes longer to reach testing, and
possibly longer to reach our derivatives as well.

CVE details: 
CVE-2023-38545 (High sev)
https://curl.se/mail/lib-2023-10/0018.html
CVE-2023-38546 (Low sev)
https://curl.se/mail/lib-2023-10/0019.html

[ Tests ]
Curl's own testsuite ran and had no errors (we run that on dh_auto_test).

[ Risks ]
There were no considerable changes required to backport the patches, the only
change was a makefile adjustment due to the context having been changed. This
was for the new test and I've confirmed it is running.

The difference between the package on testing (revision -2) and unstable
(revision -3) is minimal too (only the two new patches).

[ Checklist ]
  [x] all changes are documented in the d/changelog
  [x] I reviewed all changes and I approve them
  [x] attach debdiff against the package in testing

[ Other info ]
I purposedly backported the fix instead of packaging 8.4.0 to allow for a
faster migration to testing. Once it migrates, I'll push the pacckaging for
8.4.0.

The security team is about to release the fixes for bookworm-sec and
bullseye-sec. I have pushed the unstable, bookworm-bpo and bullseye-bpo just
now.

unblock curl/8.3.0-3

diff -Nru curl-8.3.0/debian/changelog curl-8.3.0/debian/changelog
--- curl-8.3.0/debian/changelog	2023-10-01 15:01:42.000000000 +0100
+++ curl-8.3.0/debian/changelog	2023-10-05 22:26:40.000000000 +0100
@@ -1,3 +1,9 @@
+curl (8.3.0-3) unstable; urgency=high
+
+  * Add patches to fix CVE-2023-38545 and CVE-2023-38546
+
+ -- Samuel Henrique <samueloph@debian.org>  Thu, 05 Oct 2023 22:26:40 +0100
+
 curl (8.3.0-2) unstable; urgency=medium
 
   * d/rules: Add test 3102 to TESTS_FAILS_ON_IPV6_ONLY_MACHINES
diff -Nru curl-8.3.0/debian/patches/CVE-2023-38545.patch curl-8.3.0/debian/patches/CVE-2023-38545.patch
--- curl-8.3.0/debian/patches/CVE-2023-38545.patch	1970-01-01 01:00:00.000000000 +0100
+++ curl-8.3.0/debian/patches/CVE-2023-38545.patch	2023-10-05 22:26:40.000000000 +0100
@@ -0,0 +1,130 @@
+From a6c541e709096a09eb3df6a8fbbe058239d63a55 Mon Sep 17 00:00:00 2001
+From: Jay Satiro <raysatiro@yahoo.com>
+Date: Sat, 30 Sep 2023 03:40:02 -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 too long. Unfortunately
+that did not always work as intended, leading to a security issue. And
+when it did it's a privacy violation for users of socks5h that may
+expect their DNS requests will not leak.
+
+Bug: https://curl.se/docs/CVE-2023-38545.html
+
+Backported by: Samuel Henrique <samueloph@debian.org>
+
+---
+ lib/socks.c             | 13 +++++----
+ tests/data/Makefile.inc |  2 +-
+ tests/data/test728      | 64 +++++++++++++++++++++++++++++++++++++++++
+ 3 files changed, 73 insertions(+), 6 deletions(-)
+ create mode 100644 tests/data/test728
+
+--- a/lib/socks.c
++++ b/lib/socks.c
+@@ -585,11 +585,14 @@ static CURLproxycode do_SOCKS5(struct Cu
+       infof(data, "SOCKS5: connecting to HTTP proxy %s port %d",
+             sx->hostname, sx->remote_port);
+ 
+-    /* RFC1928 chapter 5 specifies max 255 chars for domain name in packet */
++    /* RFC1928 chapter 5 specifies max 255 chars for domain name in packet.
++       If remote resolve is enabled and the host is too long then error.
++       The user's resolve setting is not overridden because that could be a
++       privacy violation and unexpected. */
+     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))
+@@ -903,7 +906,7 @@ CONNECT_RESOLVE_REMOTE:
+       }
+       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;
+       }
+--- a/tests/data/Makefile.inc
++++ b/tests/data/Makefile.inc
+@@ -101,7 +101,7 @@ 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\
+ \
+ test799 test800 test801 test802 test803 test804 test805 test806 test807 \
+ test808 test809 test810 test811 test812 test813 test814 test815 test816 \
+--- /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>
diff -Nru curl-8.3.0/debian/patches/CVE-2023-38546.patch curl-8.3.0/debian/patches/CVE-2023-38546.patch
--- curl-8.3.0/debian/patches/CVE-2023-38546.patch	1970-01-01 01:00:00.000000000 +0100
+++ curl-8.3.0/debian/patches/CVE-2023-38546.patch	2023-10-05 22:26:40.000000000 +0100
@@ -0,0 +1,128 @@
+From 61275672b46d9abb3285740467b882e22ed75da8 Mon Sep 17 00:00:00 2001
+From: Daniel Stenberg <daniel@haxx.se>
+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 --git a/lib/cookie.c b/lib/cookie.c
+index 4345a84c6fd9d..e39c89a94a960 100644
+--- a/lib/cookie.c
++++ b/lib/cookie.c
+@@ -119,7 +119,6 @@ static void freecookie(struct Cookie *co)
+   free(co->name);
+   free(co->value);
+   free(co->maxage);
+-  free(co->version);
+   free(co);
+ }
+ 
+@@ -718,11 +717,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)) {
+           /*
+@@ -1160,7 +1155,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 */
+@@ -1224,9 +1218,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.
+@@ -1378,7 +1369,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;
+@@ -1595,7 +1585,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 b3c0063b2cfb2..41e9e7a6914e0 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,17 +52,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 */
+ };
+ 
+ /* The maximum sizes we accept for cookies. RFC 6265 section 6.1 says
+diff --git a/lib/easy.c b/lib/easy.c
+index 16bbd35251d40..03195481f9780 100644
+--- a/lib/easy.c
++++ b/lib/easy.c
+@@ -925,9 +925,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;
diff -Nru curl-8.3.0/debian/patches/series curl-8.3.0/debian/patches/series
--- curl-8.3.0/debian/patches/series	2023-10-01 15:01:42.000000000 +0100
+++ curl-8.3.0/debian/patches/series	2023-10-05 22:26:40.000000000 +0100
@@ -5,10 +5,12 @@
 11_omit-directories-from-config.patch
 Remove-curl-s-LDFLAGS-from-curl-config-static-libs.patch
 
-# Upstream patches.
+# Patches from 8.4.0.
 test650_fix_an_end_tag_typo.patch
 tests_increase_the_default_server_logs_lock_timeout.patch
 lib_use_wrapper_for_curl_mime_data_fseek_callback.patch
+CVE-2023-38545.patch
+CVE-2023-38546.patch
 
 # Do not add patches below.
 # Used to generate packages for the other crypto libraries.

--- End Message ---
--- Begin Message --- 
Hi,

On 11-10-2023 08:28, Samuel Henrique wrote:

I would like the fix to migrate to testing before running all CI tests and
waiting for the bake period.
As CI has run everywhere except on ppc64el (which unfortunately has a big backlog at the moment) and no regressions were found (I've manually (thus jumping the queue) scheduled two successfully tests on ppc64el to at least confirm it's not totally broken there), I have added both a force-skiptest hint and an urgent hint. curl should migrate in the next migration run. 

Paul

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature


--- End Message ---
Reply to: