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

Bug#1053102: marked as done (bookworm-pu: package curl/7.88.1-10+deb12u3)



Your message dated Sat, 07 Oct 2023 09:59:43 +0000
with message-id <E1qp463-00A4JL-Jq@coccia.debian.org>
and subject line Released with 12.2
has caused the Debian Bug report #1053102,
regarding bookworm-pu: package curl/7.88.1-10+deb12u3
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.)


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

[ Reason ]
A vulnerability was discovered and reported to Curl upstream [1] with the
following CVE ID: CVE-2023-38039.

The description of the CVE is:

> When curl retrieves an HTTP response, it stores the incoming headers
> so that they can be accessed later via the libcurl headers API.
> However, curl did not have a limit in how many or how large headers it
> would accept in a response, allowing a malicious server to stream an
> endless series of headers and eventually cause curl to run out of heap
> memory.

This proposed updated is meant to fix the vulnerability, but it also has
another commit to fix the autopkgtest [2] (LDAP related test).

[ Impact ]
As the vulnerability is present in bookworm's curl code, it can be
exploited by malicious actors.

[ Tests ]
Automatic tests were executed (from the curl test suite) during build
time and also via autopkgtest in salsa [3]. Everything passed after
the changes were introduced.

I also conducted a test to see if the vulnerability was fixed. In order
to do so, I've downloaded the exploit PoC [4], compiled and tested in a
bookworm container. The default bookworm curl version is vulnerable, but
this new one is not (i.e. curl stops to store headers and breaks the
connection so my notebook doesn't end up completely unusable due to
memory comsuption :-).

[ Risks ]
The changes weren't big because the delta between bookworm's version and
current upstream are not that large (Debian 12 launched 3/4 months ago).
Though they exist so I did a backport of the patch (obviously there is a
chance of introducing bugs here, but we are using the tests to spot it)
and samueloph reviewed everything.

[ 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 (old)stable
  [x] the issue is verified as fixed in unstable

[ Changes ]
Here is a list of the commits applied to this pu release:

commit 3ccd62e24c87727d9b15181f2adbdd122170dbe5 
Author: Carlos Henrique Lima Melara <charlesmelara@riseup.net>
Date:   Fri Sep 15 22:34:28 2023 +0530

    Finalize changelog for 7.88.1-10+deb12u3 bookworm upload

commit ea3bd6b36d41636b268a0d0649f4486617a46ce1
Author: Carlos Henrique Lima Melara <charlesmelara@riseup.net>
Date:   Wed Sep 13 01:36:35 2023 +0530

    d/p/CVE-2023-38039.patch: backport patch

commit 044cac2e1278aefd53a39c19ccd2b6dc0ae385d1
Author: Carlos Henrique Lima Melara <charlesmelara@riseup.net>
Date:   Tue Sep 12 23:56:22 2023 +0530

    d/patches: import upstream patch to fix CVE-2023-38039

commit b943f101c182d4e14cce8f7614b300e222b73be1
Author: Andreas Hasenack <andreas.hasenack@canonical.com>
Date:   Mon Aug 7 19:18:28 2023 -0300

    Move ldap-test to a script and add retry logic

    d/t/control, d/t/curl-ldapi-test:

    Move test-command to an actual test script and add a retry logic, and
    show extra debug information if it fails.

[ Other info ]
Links:

[1] https://curl.se/docs/CVE-2023-38039.html
[2] https://salsa.debian.org/debian/curl/-/commit/b943f101c182d4e14cce8f7614b300e222b73be1
[3] https://salsa.debian.org/debian/curl/-/jobs/4557368
[4] https://hackerone.com/reports/2072338

Cheers,
Charles
diff -Nru curl-7.88.1/debian/changelog curl-7.88.1/debian/changelog
--- curl-7.88.1/debian/changelog	2023-07-25 20:11:34.000000000 +0800
+++ curl-7.88.1/debian/changelog	2023-09-16 01:01:23.000000000 +0800
@@ -1,3 +1,16 @@
+curl (7.88.1-10+deb12u3) bookworm; urgency=medium
+
+  * Team upload.
+
+  [ Andreas Hasenack ]
+  * Move ldap-test to a script and add retry logic.
+
+  [ Carlos Henrique Lima Melara ]
+  * Fix CVE-2023-38039: HTTP headers eat all memory.
+      - Done by debian/patches/CVE-2023-38039.patch.
+
+ -- Carlos Henrique Lima Melara <charlesmelara@riseup.net>  Fri, 15 Sep 2023 22:31:23 +0530
+
 curl (7.88.1-10+deb12u2) bookworm; urgency=medium
 
   * Team upload.
diff -Nru curl-7.88.1/debian/patches/CVE-2023-38039.patch curl-7.88.1/debian/patches/CVE-2023-38039.patch
--- curl-7.88.1/debian/patches/CVE-2023-38039.patch	1970-01-01 07:30:00.000000000 +0730
+++ curl-7.88.1/debian/patches/CVE-2023-38039.patch	2023-09-16 01:01:23.000000000 +0800
@@ -0,0 +1,211 @@
+From 3ee79c1674fd6f99e8efca52cd7510e08b766770 Mon Sep 17 00:00:00 2001
+From: Daniel Stenberg <daniel@haxx.se>
+Date: Wed, 2 Aug 2023 23:34:48 +0200
+Subject: [PATCH] http: return error when receiving too large header set
+
+To avoid abuse. The limit is set to 300 KB for the accumulated size of
+all received HTTP headers for a single response. Incomplete research
+suggests that Chrome uses a 256-300 KB limit, while Firefox allows up to
+1MB.
+
+Closes #11582
+
+Backport to Debian by Carlos Henrique Lima Melara <charlesmelara@riseup.net>
+---
+ lib/c-hyper.c    | 12 +++++++-----
+ lib/http.c       | 35 +++++++++++++++++++++++++++++++----
+ lib/http.h       |  8 ++++++++
+ lib/http_proxy.c |  4 +++-
+ lib/pingpong.c   |  4 +++-
+ lib/urldata.h    | 13 ++++++-------
+ 6 files changed, 58 insertions(+), 18 deletions(-)
+
+diff --git a/lib/c-hyper.c b/lib/c-hyper.c
+index 9c7632d..28f64ef 100644
+--- a/lib/c-hyper.c
++++ b/lib/c-hyper.c
+@@ -174,8 +174,11 @@ static int hyper_each_header(void *userdata,
+     }
+   }
+ 
+-  data->info.header_size += (curl_off_t)len;
+-  data->req.headerbytecount += (curl_off_t)len;
++  result = Curl_bump_headersize(data, len, FALSE);
++  if(result) {
++    data->state.hresult = result;
++    return HYPER_ITER_BREAK;
++  }
+   return HYPER_ITER_CONTINUE;
+ }
+ 
+@@ -305,9 +308,8 @@ static CURLcode status_line(struct Curl_easy *data,
+     if(result)
+       return result;
+   }
+-  data->info.header_size += (curl_off_t)len;
+-  data->req.headerbytecount += (curl_off_t)len;
+-  return CURLE_OK;
++  result = Curl_bump_headersize(data, len, FALSE);
++  return result;
+ }
+ 
+ /*
+diff --git a/lib/http.c b/lib/http.c
+index 6f71a51..8cb077f 100644
+--- a/lib/http.c
++++ b/lib/http.c
+@@ -3749,6 +3749,29 @@ static CURLcode verify_header(struct Curl_easy *data)
+   return CURLE_OK;
+ }
+ 
++CURLcode Curl_bump_headersize(struct Curl_easy *data,
++                              size_t delta,
++                              bool connect_only)
++{
++  size_t bad = 0;
++  if(delta < MAX_HTTP_RESP_HEADER_SIZE) {
++    if(!connect_only)
++      data->req.headerbytecount += (unsigned int)delta;
++    data->info.header_size += (unsigned int)delta;
++    if(data->info.header_size > MAX_HTTP_RESP_HEADER_SIZE)
++      bad = data->info.header_size;
++  }
++  else
++    bad = data->info.header_size + delta;
++  if(bad) {
++    failf(data, "Too large response headers: %zu > %zu",
++          bad, MAX_HTTP_RESP_HEADER_SIZE);
++    return CURLE_RECV_ERROR;
++  }
++  return CURLE_OK;
++}
++
++
+ /*
+  * Read any HTTP header lines from the server and pass them to the client app.
+  */
+@@ -3996,8 +4019,9 @@ CURLcode Curl_http_readwrite_headers(struct Curl_easy *data,
+       if(result)
+         return result;
+ 
+-      data->info.header_size += (long)headerlen;
+-      data->req.headerbytecount += (long)headerlen;
++      result = Curl_bump_headersize(data, headerlen, FALSE);
++      if(result)
++        return result;
+ 
+       /*
+        * When all the headers have been parsed, see if we should give
+@@ -4303,8 +4327,11 @@ CURLcode Curl_http_readwrite_headers(struct Curl_easy *data,
+     if(result)
+       return result;
+ 
+-    data->info.header_size += Curl_dyn_len(&data->state.headerb);
+-    data->req.headerbytecount += Curl_dyn_len(&data->state.headerb);
++    result = Curl_bump_headersize(data, Curl_dyn_len(&data->state.headerb),
++                                  FALSE);
++    if(result)
++      return result;
++
+ 
+     Curl_dyn_reset(&data->state.headerb);
+   }
+diff --git a/lib/http.h b/lib/http.h
+index 444abc0..86459b9 100644
+--- a/lib/http.h
++++ b/lib/http.h
+@@ -60,6 +60,9 @@ extern const struct Curl_handler Curl_handler_wss;
+ #endif
+ #endif /* websockets */
+ 
++CURLcode Curl_bump_headersize(struct Curl_easy *data,
++                              size_t delta,
++                              bool connect_only);
+ 
+ /* Header specific functions */
+ bool Curl_compareheader(const char *headerline,  /* line to check */
+@@ -176,6 +179,11 @@ CURLcode Curl_http_auth_act(struct Curl_easy *data);
+ #define EXPECT_100_THRESHOLD (1024*1024)
+ #endif
+ 
++/* MAX_HTTP_RESP_HEADER_SIZE is the maximum size of all response headers
++   combined that libcurl allows for a single HTTP response, any HTTP
++   version. This count includes CONNECT response headers. */
++#define MAX_HTTP_RESP_HEADER_SIZE (300*1024)
++
+ #endif /* CURL_DISABLE_HTTP */
+ 
+ #ifdef USE_NGHTTP3
+diff --git a/lib/http_proxy.c b/lib/http_proxy.c
+index fdd092d..2f68a5f 100644
+--- a/lib/http_proxy.c
++++ b/lib/http_proxy.c
+@@ -584,7 +584,9 @@ static CURLcode recv_CONNECT_resp(struct Curl_cfilter *cf,
+         return result;
+     }
+ 
+-    data->info.header_size += (long)perline;
++    result = Curl_bump_headersize(data, perline, TRUE);
++    if(result)
++      return result;
+ 
+     /* Newlines are CRLF, so the CR is ignored as the line isn't
+        really terminated until the LF comes. Treat a following CR
+diff --git a/lib/pingpong.c b/lib/pingpong.c
+index 2f4aa1c..189a0b6 100644
+--- a/lib/pingpong.c
++++ b/lib/pingpong.c
+@@ -341,7 +341,9 @@ CURLcode Curl_pp_readresp(struct Curl_easy *data,
+       ssize_t clipamount = 0;
+       bool restart = FALSE;
+ 
+-      data->req.headerbytecount += (long)gotbytes;
++      result = Curl_bump_headersize(data, gotbytes, FALSE);
++      if(result)
++        return result;
+ 
+       pp->nread_resp += gotbytes;
+       for(i = 0; i < gotbytes; ptr++, i++) {
+diff --git a/lib/urldata.h b/lib/urldata.h
+index 069ffb2..68d42f8 100644
+--- a/lib/urldata.h
++++ b/lib/urldata.h
+@@ -619,17 +619,16 @@ struct SingleRequest {
+   curl_off_t bytecount;         /* total number of bytes read */
+   curl_off_t writebytecount;    /* number of bytes written */
+ 
+-  curl_off_t headerbytecount;   /* only count received headers */
+-  curl_off_t deductheadercount; /* this amount of bytes doesn't count when we
++  curl_off_t pendingheader;      /* this many bytes left to send is actually
++                                    header and not body */
++  struct curltime start;         /* transfer started at this time */
++  unsigned int headerbytecount;  /* only count received headers */
++  unsigned int deductheadercount; /* this amount of bytes doesn't count when we
+                                    check if anything has been transferred at
+                                    the end of a connection. We use this
+                                    counter to make only a 100 reply (without a
+                                    following second response code) result in a
+                                    CURLE_GOT_NOTHING error code */
+-
+-  curl_off_t pendingheader;      /* this many bytes left to send is actually
+-                                    header and not body */
+-  struct curltime start;         /* transfer started at this time */
+   enum {
+     HEADER_NORMAL,              /* no bad header at all */
+     HEADER_PARTHEADER,          /* part of the chunk is a bad header, the rest
+@@ -1072,7 +1071,6 @@ struct PureInfo {
+   int httpversion; /* the http version number X.Y = X*10+Y */
+   time_t filetime; /* If requested, this is might get set. Set to -1 if the
+                       time was unretrievable. */
+-  curl_off_t header_size;  /* size of read header(s) in bytes */
+   curl_off_t request_size; /* the amount of bytes sent in the request(s) */
+   unsigned long proxyauthavail; /* what proxy auth types were announced */
+   unsigned long httpauthavail;  /* what host auth types were announced */
+@@ -1080,6 +1078,7 @@ struct PureInfo {
+   char *contenttype; /* the content type of the object */
+   char *wouldredirect; /* URL this would've been redirected to if asked to */
+   curl_off_t retry_after; /* info from Retry-After: header */
++  unsigned int header_size;  /* size of read header(s) in bytes */
+ 
+   /* PureInfo members 'conn_primary_ip', 'conn_primary_port', 'conn_local_ip'
+      and, 'conn_local_port' are copied over from the connectdata struct in
diff -Nru curl-7.88.1/debian/patches/series curl-7.88.1/debian/patches/series
--- curl-7.88.1/debian/patches/series	2023-07-25 20:11:34.000000000 +0800
+++ curl-7.88.1/debian/patches/series	2023-09-16 01:01:23.000000000 +0800
@@ -28,6 +28,9 @@
 CVE-2023-32001.patch
 Use-OpenLDAP-specific-functionality.patch
 
+# Patches from 8.3.0.
+CVE-2023-38039.patch
+
 # Do not add patches below.
 # Used to generate packages for the other crypto libraries.
 90_gnutls.patch
diff -Nru curl-7.88.1/debian/tests/control curl-7.88.1/debian/tests/control
--- curl-7.88.1/debian/tests/control	2023-07-25 20:11:34.000000000 +0800
+++ curl-7.88.1/debian/tests/control	2023-09-16 01:01:23.000000000 +0800
@@ -10,6 +10,6 @@
 Depends: @builddeps@
 Restrictions: allow-stderr
 
-Test-Command: gcc debian/tests/LDAP-bindata.c $(pkgconf --cflags --libs ldap libcurl) -o "$AUTOPKGTEST_TMP"/ldap-test && "$AUTOPKGTEST_TMP"/ldap-test
+Tests: curl-ldapi-test
 Depends: gcc, libc-dev, libcurl4-openssl-dev | libcurl-dev, libldap-dev, slapd, pkgconf
 Restrictions: allow-stderr, isolation-container, needs-root
diff -Nru curl-7.88.1/debian/tests/curl-ldapi-test curl-7.88.1/debian/tests/curl-ldapi-test
--- curl-7.88.1/debian/tests/curl-ldapi-test	1970-01-01 07:30:00.000000000 +0730
+++ curl-7.88.1/debian/tests/curl-ldapi-test	2023-09-16 01:01:23.000000000 +0800
@@ -0,0 +1,24 @@
+#!/bin/sh
+
+set -e
+
+cleanup() {
+    if [ $? -ne 0 ]; then
+        set +e
+        echo "## Something failed, gathering logs"
+        echo
+        echo "## syslog"
+        tail -n 50 /var/log/syslog
+        echo
+        echo "## slapd journal"
+        journalctl -u slapd
+    fi
+}
+
+trap cleanup EXIT
+
+echo "## Building ldap-test app"
+gcc debian/tests/LDAP-bindata.c $(pkgconf --cflags --libs ldap libcurl) -o "$AUTOPKGTEST_TMP"/ldap-test
+
+echo "## calling ldap-test"
+"$AUTOPKGTEST_TMP"/ldap-test
diff -Nru curl-7.88.1/debian/tests/LDAP-bindata.c curl-7.88.1/debian/tests/LDAP-bindata.c
--- curl-7.88.1/debian/tests/LDAP-bindata.c	2023-07-25 20:11:34.000000000 +0800
+++ curl-7.88.1/debian/tests/LDAP-bindata.c	2023-09-16 01:01:23.000000000 +0800
@@ -117,12 +117,18 @@
 		ldap_perror(p, "Failed to initialize libldap");
 		exit(EXIT_FAILURE);
 	}
-	if(p = ldap_connect(ldp)) {
-		ldap_perror(p, "Failed to connect to slapd over UNIX domain socket");
-		if(p = ldap_unbind_ext(ldp, NULL, NULL)) {
-			ldap_perror(p, "Failed to deinitialize libldap");
+	unsigned int counter = 0;
+	while (p = ldap_connect(ldp)) {
+		counter++;
+		fprintf(stderr, "ldapi:// connection failed, retrying (count=%u)\n", counter);
+		if (counter >= 10) {
+			ldap_perror(p, "Failed to connect to slapd over UNIX domain socket");
+			if (p = ldap_unbind_ext(ldp, NULL, NULL)) {
+				ldap_perror(p, "Failed to deinitialize libldap");
+			}
+			exit(EXIT_FAILURE);
 		}
-		exit(EXIT_FAILURE);
+		sleep(1);
 	}
 	if(p = ldap_sasl_bind_s(ldp, u8"CN=admin,DC=example,DC=com", LDAP_SASL_SIMPLE, &(struct berval){.bv_len = strlen(u8"Password"), .bv_val = u8"Password"}, NULL, NULL, NULL)) {
 		ldap_perror(p, "Failed to bind to directory server");

--- End Message ---
--- Begin Message ---
Version: 12.2

The upload requested in this bug has been released as part of 12.2.

--- End Message ---

Reply to: