Bug#931610: stretch-pu: package pound/2.7-1.3+deb9u1
Control: tags -1 - confirmed
Hi Adam,
> On Sat, 2019-07-13 at 12:36 +0200, Carsten Leonhardt wrote:
>> Control: tags -1 - moreinfo
>>
>> Hi,
>>
>> attached is a new debdiff, the only change is that I removed some
>> cruft
>> from the "Origin" field in the patch metadata.
>>
>> I've deployed this version on live servers this morning and tested
>> them.
>>
>
> Please go ahead; thanks.
longer testing revealed a regression (CPU load built up slowly, finally
reaching 100%).
I found a fix and have applied it, the fixed version is running on live
servers since at least a week now, without a sign of abnormal CPU load.
To see just the fix:
https://salsa.debian.org/debian/pound/commit/bdd20196df7ff52f65c57c83c1ae5a56e74bca03
A full debdiff is attached.
Sorry for the complication, I should have written earlier.
Regards,
Carsten
diff -Nru pound-2.7/debian/changelog pound-2.7/debian/changelog
--- pound-2.7/debian/changelog 2017-02-19 14:13:02.000000000 +0000
+++ pound-2.7/debian/changelog 2019-07-07 21:44:04.000000000 +0000
@@ -1,3 +1,10 @@
+pound (2.7-1.3+deb9u1) stretch; urgency=medium
+
+ * Fix request smuggling via crafted headers, CVE-2016-10711
+ (Closes: #888786).
+
+ -- Carsten Leonhardt <leo@debian.org> Sun, 07 Jul 2019 23:44:04 +0200
+
pound (2.7-1.3) unstable; urgency=medium
* Non-maintainer upload.
diff -Nru pound-2.7/debian/patches/0003-CVE-2016-1071.patch pound-2.7/debian/patches/0003-CVE-2016-1071.patch
--- pound-2.7/debian/patches/0003-CVE-2016-1071.patch 1970-01-01 00:00:00.000000000 +0000
+++ pound-2.7/debian/patches/0003-CVE-2016-1071.patch 2019-07-07 21:44:04.000000000 +0000
@@ -0,0 +1,210 @@
+Description: Backport fix for CVE-2016-10711
+Author: Robert Segall
+Origin: upstream, http://www.apsis.ch/pound/Pound-2.8a.tgz
+Last-Update: 2019-07-07
+--- a/http.c
++++ b/http.c
+@@ -31,7 +31,8 @@
+ static char *h500 = "500 Internal Server Error",
+ *h501 = "501 Not Implemented",
+ *h503 = "503 Service Unavailable",
+- *h414 = "414 Request URI too long";
++ *h414 = "414 Request URI too long",
++ *h400 = "Bad Request";
+
+ static char *err_response = "HTTP/1.0 %s\r\nContent-Type: text/html\r\nContent-Length: %d\r\nExpires: now\r\nPragma: no-cache\r\nCache-control: no-cache,no-store\r\n\r\n%s";
+
+@@ -83,7 +84,7 @@
+ safe_url, safe_url);
+ snprintf(rep, sizeof(rep),
+ "HTTP/1.0 %d %s\r\nLocation: %s\r\nContent-Type: text/html\r\nContent-Length: %d\r\n\r\n",
+- code, code_msg, safe_url, strlen(cont));
++ code, code_msg, safe_url, (int)strlen(cont));
+ BIO_write(c, rep, strlen(rep));
+ BIO_write(c, cont, strlen(cont));
+ BIO_flush(c);
+@@ -126,11 +127,11 @@
+ get_line(BIO *const in, char *const buf, const int bufsize)
+ {
+ char tmp;
+- int i, n_read;
++ int i, n_read, seen_cr;
+
+ memset(buf, 0, bufsize);
+- for(n_read = 0;;)
+- switch(BIO_gets(in, buf + n_read, bufsize - n_read - 1)) {
++ for(i = 0, seen_cr = 0; i < bufsize - 1; i++)
++ switch(BIO_read(in, &tmp, 1)) {
+ case -2:
+ /* BIO_gets not implemented */
+ return -1;
+@@ -138,24 +139,49 @@
+ case -1:
+ return 1;
+ default:
+- for(i = n_read; i < bufsize && buf[i]; i++)
+- if(buf[i] == '\n' || buf[i] == '\r') {
+- buf[i] = '\0';
++ if(seen_cr)
++ if(tmp != '\n') {
++ /* we have CR not followed by NL */
++ do {
++ if(BIO_read(in, &tmp, 1) < 0)
++ return 1;
++ } while(tmp != '\n');
++ return 1;
++ } else {
++ buf[i - 1] = '\0';
+ return 0;
+ }
+- if(i < bufsize) {
+- n_read = i;
++
++ if(!iscntrl(tmp) || tmp == '\t') {
++ buf[i] = tmp;
++ continue;
++ }
++
++ if(tmp == '\r') {
++ seen_cr = 1;
+ continue;
+ }
+- logmsg(LOG_NOTICE, "(%lx) line too long: %s", pthread_self(), buf);
+- /* skip rest of "line" */
+- tmp = '\0';
+- while(tmp != '\n')
+- if(BIO_read(in, &tmp, 1) != 1)
++
++ if(tmp == '\n') {
++ /* line ends in NL only (no CR) */
++ buf[i] = 0;
++ return 0;
++ }
++
++ /* all other control characters cause an error */
++ do {
++ if(BIO_read(in, &tmp, 1) < 0)
+ return 1;
+- break;
++ } while(tmp != '\n');
++ return 1;
+ }
+- return 0;
++
++ /* line too long */
++ do {
++ if(BIO_read(in, &tmp, 1) < 0)
++ return 1;
++ } while(tmp != '\n');
++ return 1;
+ }
+
+ /*
+@@ -393,22 +419,16 @@
+
+ /* HTTP/1.1 allows leading CRLF */
+ memset(buf, 0, MAXBUF);
+- while((res = BIO_gets(in, buf, MAXBUF - 1)) > 0) {
+- has_eol = strip_eol(buf);
++ while((res = get_line(in, buf, MAXBUF)) == 0)
+ if(buf[0])
+ break;
+- }
+
+- if(res <= 0) {
++ if(res < 0) {
+ /* this is expected to occur only on client reads */
+ /* logmsg(LOG_NOTICE, "headers: bad starting read"); */
+ return NULL;
+- } else if(!has_eol) {
+- /* check for request length limit */
+- logmsg(LOG_WARNING, "(%lx) e414 headers: request URI too long", pthread_self());
+- err_reply(cl, h414, lstn->err414);
+- return NULL;
+ }
++
+ if((headers = (char **)calloc(MAXHEADERS, sizeof(char *))) == NULL) {
+ logmsg(LOG_WARNING, "(%lx) e500 headers: out of memory", pthread_self());
+ err_reply(cl, h500, lstn->err500);
+@@ -426,8 +446,10 @@
+ for(n = 1; n < MAXHEADERS; n++) {
+ if(get_line(in, buf, MAXBUF)) {
+ free_headers(headers);
++ /* this is not necessarily an error, EOF/timeout are possible
+ logmsg(LOG_WARNING, "(%lx) e500 can't read header", pthread_self());
+ err_reply(cl, h500, lstn->err500);
++ */
+ return NULL;
+ }
+ if(!buf[0])
+@@ -713,23 +735,39 @@
+ conn_closed = 1;
+ break;
+ case HEADER_TRANSFER_ENCODING:
+- if(cont >= L0)
+- headers_ok[n] = 0;
+- else if(!strcasecmp("chunked", buf))
+- if(chunked)
+- headers_ok[n] = 0;
+- else
+- chunked = 1;
++ if(!strcasecmp("chunked", buf))
++ chunked = 1;
++ else {
++ addr2str(caddr, MAXBUF - 1, &from_host, 1);
++ logmsg(LOG_NOTICE, "(%lx) e400 multiple Transfer-encoding \"%s\" from %s", pthread_self(), url, caddr);
++ err_reply(cl, h400, "Bad request: multiple Transfer-encoding values");
++ free_headers(headers);
++ clean_all();
++ return;
++ }
+ break;
+ case HEADER_CONTENT_LENGTH:
+- if(chunked || cont >= 0L)
+- headers_ok[n] = 0;
+- else {
+- if((cont = ATOL(buf)) < 0L)
+- headers_ok[n] = 0;
+- if(is_rpc == 1 && (cont < 0x20000L || cont > 0x80000000L))
+- is_rpc = -1;
++ if(cont != L_1 || strchr(buf, ',')) {
++ addr2str(caddr, MAXBUF - 1, &from_host, 1);
++ logmsg(LOG_NOTICE, "(%lx) e400 multiple Content-length \"%s\" from %s", pthread_self(), url, caddr);
++ err_reply(cl, h400, "Bad request: multiple Content-length values");
++ free_headers(headers);
++ clean_all();
++ return;
+ }
++ for(mh = buf; *mh; mh++)
++ if(!isdigit(*mh)) {
++ addr2str(caddr, MAXBUF - 1, &from_host, 1);
++ logmsg(LOG_NOTICE, "(%lx) e400 Content-length bad value \"%s\" from %s", pthread_self(), url, caddr);
++ err_reply(cl, h400, "Bad request: Content-length bad value");
++ free_headers(headers);
++ clean_all();
++ return;
++ }
++ if((cont = ATOL(buf)) < 0L)
++ headers_ok[n] = 0;
++ if(is_rpc == 1 && (cont < 0x20000L || cont > 0x80000000L))
++ is_rpc = -1;
+ break;
+ case HEADER_EXPECT:
+ /*
+@@ -787,6 +825,16 @@
+ }
+ }
+
++ /* check for possible request smuggling attempt */
++ if(chunked != 0 && cont != L_1) {
++ addr2str(caddr, MAXBUF - 1, &from_host, 1);
++ logmsg(LOG_NOTICE, "(%lx) e501 Transfer-encoding and Content-length \"%s\" from %s", pthread_self(), url, caddr);
++ err_reply(cl, h400, "Bad request: Transfer-encoding and Content-length headers present");
++ free_headers(headers);
++ clean_all();
++ return;
++ }
++
+ /* possibly limited request size */
+ if(lstn->max_req > L0 && cont > L0 && cont > lstn->max_req && is_rpc != 1) {
+ addr2str(caddr, MAXBUF - 1, &from_host, 1);
diff -Nru pound-2.7/debian/patches/0004-bugfix-BIO_read pound-2.7/debian/patches/0004-bugfix-BIO_read
--- pound-2.7/debian/patches/0004-bugfix-BIO_read 1970-01-01 00:00:00.000000000 +0000
+++ pound-2.7/debian/patches/0004-bugfix-BIO_read 2019-07-07 21:44:04.000000000 +0000
@@ -0,0 +1,37 @@
+Description: Bugfix
+ * http.c: Stop if BIO_read returns <= 0
+ fixes a bug in the fix for CVE-2016-1071 that causes pound to use 100% CPU
+ see http://www.apsis.ch/pound/pound_list/archive/2018/2018-06/1529070189000
+Author: Sergey Poznyakoff <gray@gnu.org>
+Date: Wed Feb 28 13:44:01 2018 +0000
+Origin: https://github.com/graygnuorg/pound/commit/c5a95780e2233a05ab3fb8b4eb8a9550f0c3b53c
+
+--- a/http.c
++++ b/http.c
+@@ -143,7 +143,7 @@
+ if(tmp != '\n') {
+ /* we have CR not followed by NL */
+ do {
+- if(BIO_read(in, &tmp, 1) < 0)
++ if(BIO_read(in, &tmp, 1) <= 0)
+ return 1;
+ } while(tmp != '\n');
+ return 1;
+@@ -170,7 +170,7 @@
+
+ /* all other control characters cause an error */
+ do {
+- if(BIO_read(in, &tmp, 1) < 0)
++ if(BIO_read(in, &tmp, 1) <= 0)
+ return 1;
+ } while(tmp != '\n');
+ return 1;
+@@ -178,7 +178,7 @@
+
+ /* line too long */
+ do {
+- if(BIO_read(in, &tmp, 1) < 0)
++ if(BIO_read(in, &tmp, 1) <= 0)
+ return 1;
+ } while(tmp != '\n');
+ return 1;
diff -Nru pound-2.7/debian/patches/series pound-2.7/debian/patches/series
--- pound-2.7/debian/patches/series 2017-02-19 14:13:02.000000000 +0000
+++ pound-2.7/debian/patches/series 2019-07-07 21:44:04.000000000 +0000
@@ -1,2 +1,4 @@
0001-Add-MKCALENDAR-to-xHTTP-2-and-above.patch
0002-add-support-openssl1.1-dhparam.patch
+0003-CVE-2016-1071.patch
+0004-bugfix-BIO_read
Reply to: