Fix CVE-2019-15892

This commit is contained in:
wang_yue111 2021-01-19 11:36:56 +08:00
parent d45d322d9a
commit d87d4c4387
9 changed files with 534 additions and 1 deletions

221
CVE-2019-15892-1.patch Normal file
View File

@ -0,0 +1,221 @@
From 177e17c8f129c58daeeb98055761ee65ab5c3dfc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alf-Andr=C3=A9=20Walla?= <fwsgonzo@hotmail.com>
Date: Tue, 13 Aug 2019 12:52:39 +0200
Subject: [PATCH] Add bounds-checking to vct_iscrlf and vct_skipcrlf
The macros vct_iscrlf() and vct_skipcrlf() may look at one or two bytes
after its pointer value, causing OOB reads. This would allow
http1_dissect_hdrs to wrongly see a CRLF when one wasn't there (the last
LF left over in the bufer from the previous request).
Change the macros to inline functions, and harden them by always sending
the end pointer so that they can't overflow.
vct_iscrlf() will return an int value of 0 for no [CR]LF, 1 for LF and 2
for CRLF.
vct_skipcrlf() will return the pointer having been skipped 0, 1 or 2
bytes.
---
bin/varnishd/http1/cache_http1_proto.c | 16 +++++++++-------
bin/varnishtest/vtc_http.c | 26 ++++++++++++++------------
include/vct.h | 19 +++++++++++++++++--
bin/varnishtest/vtc_subr.c | 3 ++-
4 files changed, 42 insertions(+), 22 deletions(-)
diff --git a/bin/varnishd/http1/cache_http1_proto.c b/bin/varnishd/http1/cache_http1_proto.c
index dd81863d32..5d99da47a8 100644
--- a/bin/varnishd/http1/cache_http1_proto.c
+++ b/bin/varnishd/http1/cache_http1_proto.c
@@ -121,31 +121,31 @@ http1_dissect_hdrs(struct http *hp, char *p, struct http_conn *htc,
/* Find end of next header */
q = r = p;
- if (vct_iscrlf(p))
+ if (vct_iscrlf(p, htc->rxbuf_e))
break;
while (r < htc->rxbuf_e) {
if (!vct_isctl(*r) || vct_issp(*r)) {
r++;
continue;
}
- if (!vct_iscrlf(r)) {
+ if (!vct_iscrlf(r, htc->rxbuf_e)) {
VSLb(hp->vsl, SLT_BogoHeader,
"Header has ctrl char 0x%02x", *r);
return (400);
}
q = r;
assert(r < htc->rxbuf_e);
- r += vct_skipcrlf(r);
+ r = vct_skipcrlf(r, htc->rxbuf_e);
if (r >= htc->rxbuf_e)
break;
- if (vct_iscrlf(r))
+ if (vct_iscrlf(r, htc->rxbuf_e))
break;
/* If line does not continue: got it. */
if (!vct_issp(*r))
break;
/* Clear line continuation LWS to spaces */
- while (vct_islws(*q))
+ while (q < htc->rxbuf_e && vct_islws(*q))
*q++ = ' ';
}
@@ -275,7 +275,7 @@ http1_splitline(struct http *hp, struct http_conn *htc, const int *hf,
hp->hd[hf[2]].b = p;
/* Third field is optional and cannot contain CTL except TAB */
- for (; !vct_iscrlf(p); p++) {
+ for (; p < htc->rxbuf_e && !vct_iscrlf(p, htc->rxbuf_e); p++) {
if (vct_isctl(*p) && !vct_issp(*p)) {
hp->hd[hf[2]].b = NULL;
return (400);
@@ -284,7 +284,9 @@ http1_splitline(struct http *hp, struct http_conn *htc, const int *hf,
hp->hd[hf[2]].e = p;
/* Skip CRLF */
- i = vct_skipcrlf(p);
+ i = vct_iscrlf(p, htc->rxbuf_e);
+ if (!i)
+ return (400);
*p = '\0';
p += i;
diff --git a/bin/varnishtest/vtc_http.c b/bin/varnishtest/vtc_http.c
index 616cb459e1..e17643f8eb 100644
--- a/bin/varnishtest/vtc_http.c
+++ b/bin/varnishtest/vtc_http.c
@@ -409,6 +409,7 @@ http_splitheader(struct http *hp, int req)
char *p, *q, **hh;
int n;
char buf[20];
+ const char* rxbuf_e = &hp->rxbuf[hp->prxbuf];
CHECK_OBJ_NOTNULL(hp, HTTP_MAGIC);
if (req) {
@@ -428,20 +429,20 @@ http_splitheader(struct http *hp, int req)
hh[n++] = p;
while (!vct_islws(*p))
p++;
- AZ(vct_iscrlf(p));
+ AZ(vct_iscrlf(p, rxbuf_e));
*p++ = '\0';
/* URL/STATUS */
while (vct_issp(*p)) /* XXX: H space only */
p++;
- AZ(vct_iscrlf(p));
+ AZ(vct_iscrlf(p, rxbuf_e));
hh[n++] = p;
while (!vct_islws(*p))
p++;
- if (vct_iscrlf(p)) {
+ if (vct_iscrlf(p, rxbuf_e)) {
hh[n++] = NULL;
q = p;
- p += vct_skipcrlf(p);
+ p = vct_skipcrlf(p, rxbuf_e);
*q = '\0';
} else {
*p++ = '\0';
@@ -449,26 +450,26 @@ http_splitheader(struct http *hp, int req)
while (vct_issp(*p)) /* XXX: H space only */
p++;
hh[n++] = p;
- while (!vct_iscrlf(p))
+ while (!vct_iscrlf(p, rxbuf_e))
p++;
q = p;
- p += vct_skipcrlf(p);
+ p = vct_skipcrlf(p, rxbuf_e);
*q = '\0';
}
assert(n == 3);
while (*p != '\0') {
assert(n < MAX_HDR);
- if (vct_iscrlf(p))
+ if (vct_iscrlf(p, rxbuf_e))
break;
hh[n++] = p++;
- while (*p != '\0' && !vct_iscrlf(p))
+ while (*p != '\0' && !vct_iscrlf(p, rxbuf_e))
p++;
q = p;
- p += vct_skipcrlf(p);
+ p = vct_skipcrlf(p, rxbuf_e);
*q = '\0';
}
- p += vct_skipcrlf(p);
+ p = vct_skipcrlf(p, rxbuf_e);
assert(*p == '\0');
for (n = 0; n < 3 || hh[n] != NULL; n++) {
@@ -564,15 +565,16 @@ http_rxchunk(struct http *hp)
vtc_dump(hp->vl, 4, "chunk", hp->rxbuf + l, i);
}
l = hp->prxbuf;
+
if (http_rxchar(hp, 2, 0) < 0)
return (-1);
- if (!vct_iscrlf(hp->rxbuf + l)) {
+ if (!vct_iscrlf(&hp->rxbuf[l], &hp->rxbuf[hp->prxbuf])) {
vtc_log(hp->vl, hp->fatal,
"Wrong chunk tail[0] = %02x",
hp->rxbuf[l] & 0xff);
return (-1);
}
- if (!vct_iscrlf(hp->rxbuf + l + 1)) {
+ if (!vct_iscrlf(&hp->rxbuf[l + 1], &hp->rxbuf[hp->prxbuf])) {
vtc_log(hp->vl, hp->fatal,
"Wrong chunk tail[1] = %02x",
hp->rxbuf[l + 1] & 0xff);
diff --git a/include/vct.h b/include/vct.h
index 24143a3322..1b7ffbd4f5 100644
--- a/include/vct.h
+++ b/include/vct.h
@@ -76,7 +76,22 @@ vct_is(int x, uint16_t y)
#define vct_isxmlname(x) vct_is(x, VCT_XMLNAMESTART | VCT_XMLNAME)
#define vct_istchar(x) vct_is(x, VCT_ALPHA | VCT_DIGIT | VCT_TCHAR)
-#define vct_iscrlf(p) (((p)[0] == 0x0d && (p)[1] == 0x0a) || (p)[0] == 0x0a)
+static inline int
+vct_iscrlf(const char* p, const char* end)
+{
+ assert(p <= end);
+ if (p == end)
+ return (0);
+ if ((p[0] == 0x0d && (p+1 < end) && p[1] == 0x0a)) // CR LF
+ return (2);
+ if (p[0] == 0x0a) // LF
+ return (1);
+ return (0);
+}
/* NB: VCT always operate in ASCII, don't replace 0x0d with \r etc. */
-#define vct_skipcrlf(p) ((p)[0] == 0x0d && (p)[1] == 0x0a ? 2 : 1)
+static inline char*
+vct_skipcrlf(char* p, const char* end)
+{
+ return (p + vct_iscrlf(p, end));
+}
diff --git a/bin/varnishtest/vtc_subr.c b/bin/varnishtest/vtc_subr.c
index 2c1439a..f200981 100644
--- a/bin/varnishtest/vtc_subr.c
+++ b/bin/varnishtest/vtc_subr.c
@@ -33,10 +33,11 @@
#include <string.h>
#include <stdint.h>
+#include "vtc.h"
+
#include "vct.h"
#include "vnum.h"
#include "vre.h"
-#include "vtc.h"
struct vsb *
vtc_hex_to_bin(struct vtclog *vl, const char *arg)

25
CVE-2019-15892-2.patch Normal file
View File

@ -0,0 +1,25 @@
From f98c250300bd7303bb7b706384ec153101a3eab0 Mon Sep 17 00:00:00 2001
From: Martin Blix Grydeland <martin@varnish-software.com>
Date: Thu, 15 Aug 2019 10:44:00 +0200
Subject: [PATCH] Allow a NULL value in http_Proto
The proto field is optional in HTTP, so it may not be set. Set the proto
to 0 also for a NULL value instead of segfaulting if it were NULL.
---
bin/varnishd/cache/cache_http.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/bin/varnishd/cache/cache_http.c b/bin/varnishd/cache/cache_http.c
index 23eaa0b183..070ead2e6b 100644
--- a/bin/varnishd/cache/cache_http.c
+++ b/bin/varnishd/cache/cache_http.c
@@ -212,7 +212,8 @@ http_Proto(struct http *to)
fm = to->hd[HTTP_HDR_PROTO].b;
- if ((fm[0] == 'H' || fm[0] == 'h') &&
+ if (fm != NULL &&
+ (fm[0] == 'H' || fm[0] == 'h') &&
(fm[1] == 'T' || fm[1] == 't') &&
(fm[2] == 'T' || fm[2] == 't') &&
(fm[3] == 'P' || fm[3] == 'p') &&

48
CVE-2019-15892-3.patch Normal file
View File

@ -0,0 +1,48 @@
From 3dc8c15adc23456f494fd23455b2251efe275eda Mon Sep 17 00:00:00 2001
From: Martin Blix Grydeland <martin@varnish-software.com>
Date: Thu, 15 Aug 2019 10:56:58 +0200
Subject: [PATCH] Fix http1_splitline parsing of 2 field HTTP proto lines using
NLNL
When parsing a request like this, "GET /\n\n", the first NL would be
overwritten by nul guard inserted after the 2nd field, and the second NL
would be overwritten by the nul guard after the missing 3rd field. This
would cause http1_dissect_hdrs to attempt to decode the body as headers.
---
bin/varnishd/http1/cache_http1_proto.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/bin/varnishd/http1/cache_http1_proto.c b/bin/varnishd/http1/cache_http1_proto.c
index 5d99da47a8..af9ca3898c 100644
--- a/bin/varnishd/http1/cache_http1_proto.c
+++ b/bin/varnishd/http1/cache_http1_proto.c
@@ -224,7 +224,7 @@ static uint16_t
http1_splitline(struct http *hp, struct http_conn *htc, const int *hf,
unsigned maxhdr)
{
- char *p;
+ char *p, *q;
int i;
assert(hf == HTTP1_Req || hf == HTTP1_Resp);
@@ -265,14 +265,19 @@ http1_splitline(struct http *hp, struct http_conn *htc, const int *hf,
hp->hd[hf[1]].e = p;
if (!Tlen(hp->hd[hf[1]]))
return (400);
- *p++ = '\0';
/* Skip SP */
+ q = p;
for (; vct_issp(*p); p++) {
if (vct_isctl(*p))
return (400);
}
hp->hd[hf[2]].b = p;
+ if (q < p)
+ *q = '\0'; /* Nul guard for the 2nd field. If q == p
+ * (the third optional field is not
+ * present), the last nul guard will
+ * cover this field. */
/* Third field is optional and cannot contain CTL except TAB */
for (; p < htc->rxbuf_e && !vct_iscrlf(p, htc->rxbuf_e); p++) {

46
CVE-2019-15892-4.patch Normal file
View File

@ -0,0 +1,46 @@
From 73befed1a6950f5312e3a422dde82a7bb5a8bbe3 Mon Sep 17 00:00:00 2001
From: Martin Blix Grydeland <martin@varnish-software.com>
Date: Thu, 15 Aug 2019 11:16:22 +0200
Subject: [PATCH] Do not set the proto txt.b value when third field is missing
In http1_splitline, if the third field is missing, we would still set the
txt.b value to where the field would have been, with a NULL txt.e
entry. This would cause http_Proto to attempt to parse the values
there. Fix this by only setting the .b and .e if the third field was
present.
---
bin/varnishd/http1/cache_http1_proto.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/bin/varnishd/http1/cache_http1_proto.c b/bin/varnishd/http1/cache_http1_proto.c
index af9ca3898c..e55555bf19 100644
--- a/bin/varnishd/http1/cache_http1_proto.c
+++ b/bin/varnishd/http1/cache_http1_proto.c
@@ -272,7 +272,6 @@ http1_splitline(struct http *hp, struct http_conn *htc, const int *hf,
if (vct_isctl(*p))
return (400);
}
- hp->hd[hf[2]].b = p;
if (q < p)
*q = '\0'; /* Nul guard for the 2nd field. If q == p
* (the third optional field is not
@@ -280,13 +279,15 @@ http1_splitline(struct http *hp, struct http_conn *htc, const int *hf,
* cover this field. */
/* Third field is optional and cannot contain CTL except TAB */
+ q = p;
for (; p < htc->rxbuf_e && !vct_iscrlf(p, htc->rxbuf_e); p++) {
- if (vct_isctl(*p) && !vct_issp(*p)) {
- hp->hd[hf[2]].b = NULL;
+ if (vct_isctl(*p) && !vct_issp(*p))
return (400);
- }
}
- hp->hd[hf[2]].e = p;
+ if (p > q) {
+ hp->hd[hf[2]].b = q;
+ hp->hd[hf[2]].e = p;
+ }
/* Skip CRLF */
i = vct_iscrlf(p, htc->rxbuf_e);

41
CVE-2019-15892-5.patch Normal file
View File

@ -0,0 +1,41 @@
From 3eb7a04587d235bec5a312d3eae652abd8a63a14 Mon Sep 17 00:00:00 2001
From: Martin Blix Grydeland <martin@varnish-software.com>
Date: Thu, 15 Aug 2019 11:19:41 +0200
Subject: [PATCH] Be stricter on final [CR]LF parsing in http1_dissect_hdrs
The end of http1_dissect_hdrs ends with skipping over the final [CR]LF
that marks then end of the headers. Currently that skip is optional, that
is, it is skipped if it was present.
This patch adds an assert if the final [CR]LF is not found when finishing
the parsing. HTTP1_Complete guarantees that it is there, if not we would
not have started parsing the request or response in the first place, and
if it is missing, there must be an error in the parsing leading up to it.
---
bin/varnishd/http1/cache_http1_proto.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/bin/varnishd/http1/cache_http1_proto.c b/bin/varnishd/http1/cache_http1_proto.c
index e55555bf19..e5203a94ec 100644
--- a/bin/varnishd/http1/cache_http1_proto.c
+++ b/bin/varnishd/http1/cache_http1_proto.c
@@ -117,6 +117,7 @@ http1_dissect_hdrs(struct http *hp, char
unsigned maxhdr)
{
char *q, *r;
+ int i;
assert(p > htc->rxbuf_b);
assert(p <= htc->rxbuf_e);
@@ -213,8 +214,9 @@ http1_dissect_hdrs(struct http *hp, char
break;
}
}
- if (p < htc->rxbuf_e)
- p += vct_skipcrlf(p);
+ i = vct_iscrlf(p, htc->rxbuf_e);
+ assert(i > 0); /* HTTP1_Complete guarantees this */
+ p += i;
HTC_RxPipeline(htc, p);
htc->rxbuf_e = p;
return (0);

60
CVE-2019-15892-6.patch Normal file
View File

@ -0,0 +1,60 @@
From bf18bb21ef9c269edadac549b7b7d43fdb87051c Mon Sep 17 00:00:00 2001
From: Martin Blix Grydeland <martin@varnish-software.com>
Date: Thu, 15 Aug 2019 12:54:50 +0200
Subject: [PATCH] Fix HTTP header line continuation in http1_dissect_hdrs
When clearing the [CR]LF in a line continuation, we would continue
replacing any [CR|LF|HT|SP] characters up until the end of the buffer,
possibly overwriting later [CR]LFs. Fix this by only unconditionally
overwrite one [CR]LF, and then only replace [HT|SP] with SP to keep with
previous behaviour.
Update r00494.vtc to include multiple line continuations to make sure they
are parsed.
---
bin/varnishd/http1/cache_http1_proto.c | 4 +++-
bin/varnishtest/tests/r00494.vtc | 11 +++++++++++
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/bin/varnishd/http1/cache_http1_proto.c b/bin/varnishd/http1/cache_http1_proto.c
index e5203a94ec..e373d7d5d5 100644
--- a/bin/varnishd/http1/cache_http1_proto.c
+++ b/bin/varnishd/http1/cache_http1_proto.c
@@ -146,7 +146,9 @@ http1_dissect_hdrs(struct http *hp, char *p, struct http_conn *htc,
break;
/* Clear line continuation LWS to spaces */
- while (q < htc->rxbuf_e && vct_islws(*q))
+ while (q < r)
+ *q++ = ' ';
+ while (q < htc->rxbuf_e && vct_issp(*q))
*q++ = ' ';
}
diff --git a/bin/varnishtest/tests/r00494.vtc b/bin/varnishtest/tests/r00494.vtc
index cb0bbe8d7b..e0db8a4bf8 100644
--- a/bin/varnishtest/tests/r00494.vtc
+++ b/bin/varnishtest/tests/r00494.vtc
@@ -6,6 +6,11 @@ server s1 {
rxreq
txresp -hdr {Foo: bar,
barf: fail} -body "xxx"
+
+ rxreq
+ txresp -hdr {Foo: bar,
+
+ barf: fail} -body "xxx"
} -start
varnish v1 -vcl+backend {
@@ -21,4 +26,10 @@ client c1 {
expect resp.http.bar == "bar, barf: fail"
expect resp.http.barf == <undef>
expect resp.http.foo == <undef>
+
+ txreq -url /2
+ rxresp
+ expect resp.http.bar == "bar, barf: fail"
+ expect resp.http.barf == <undef>
+ expect resp.http.foo == <undef>
} -run

45
CVE-2019-15892-7.patch Normal file
View File

@ -0,0 +1,45 @@
From ea1d09b3b8ee8ad667b9d680013ed9448e0727dc Mon Sep 17 00:00:00 2001
From: Martin Blix Grydeland <martin@varnish-software.com>
Date: Thu, 15 Aug 2019 14:06:00 +0200
Subject: [PATCH] Add a test case covering some HTTP/1 parsing corner cases
---
bin/varnishtest/tests/b00067.vtc | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
create mode 100644 bin/varnishtest/tests/b00067.vtc
diff --git a/bin/varnishtest/tests/b00067.vtc b/bin/varnishtest/tests/b00067.vtc
new file mode 100644
index 0000000000..2167c9483f
--- /dev/null
+++ b/bin/varnishtest/tests/b00067.vtc
@@ -0,0 +1,29 @@
+varnishtest "HTTP/1 parsing checks"
+
+# Some tricky requests that have been known to cause parsing errors in the past.
+
+server s1 {
+ rxreq
+ txresp
+} -start
+
+varnish v1 -vcl+backend {
+} -start
+
+# This test checks a bug that was dependent on the contents of the buffer left behind
+# by the previous request
+client c1 {
+ send "GET / HTTP/1.1\r\nHost: asdf.com\r\nFoo: baar\r\n\r\n\r\n\r\n\r\n"
+ rxresp
+ send "GET / HTTP/1.1\r\nHost: asdf.com\r\nAsdf: b\n \r\n\r\nSj\r"
+ rxresp
+ expect resp.status == 200
+} -run
+
+# This tests that the line continuation handling doesn't clear out the end of headers
+# [CR]LF
+client c1 {
+ send "GET / HTTP/1.1\r\nHost: asdf.com\r\nAsdf: b\n \r\n\r\nSj"
+ rxresp
+ expect resp.status == 200
+} -run

36
CVE-2019-15892-8.patch Normal file
View File

@ -0,0 +1,36 @@
From 6da64a47beff44ecdb45c82b033811f2d19819af Mon Sep 17 00:00:00 2001
From: Martin Blix Grydeland <martin@varnish-software.com>
Date: Fri, 23 Aug 2019 13:53:42 +0200
Subject: [PATCH] Avoid some code duplication
Apply some adjustments to recent patches based off of review by Nils
Goroll at UPLEX (@nigoroll)
---
bin/varnishd/http1/cache_http1_proto.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/bin/varnishd/http1/cache_http1_proto.c b/bin/varnishd/http1/cache_http1_proto.c
index 61563b8ead..31c75ed88d 100644
--- a/bin/varnishd/http1/cache_http1_proto.c
+++ b/bin/varnishd/http1/cache_http1_proto.c
@@ -128,15 +128,16 @@ http1_dissect_hdrs(struct http *hp, char *p, struct http_conn *htc,
r++;
continue;
}
- if (!vct_iscrlf(r, htc->rxbuf_e)) {
+ i = vct_iscrlf(r, htc->rxbuf_e);
+ if (i == 0) {
VSLb(hp->vsl, SLT_BogoHeader,
"Header has ctrl char 0x%02x", *r);
return (400);
}
q = r;
- assert(r < htc->rxbuf_e);
- r = vct_skipcrlf(r, htc->rxbuf_e);
- if (r >= htc->rxbuf_e)
+ r += i;
+ assert(r <= htc->rxbuf_e);
+ if (r == htc->rxbuf_e)
break;
if (vct_iscrlf(r, htc->rxbuf_e))
break;

View File

@ -1,7 +1,7 @@
Name: varnish
Summary: A web application accelerator
Version: 6.0.0
Release: 5
Release: 6
License: BSD
URL: https://www.varnish-cache.org/
Source0: http://varnish-cache.org/_downloads/varnish-%{version}.tgz
@ -11,6 +11,14 @@ Source1: https://github.com/varnishcache/pkg-varnish-cache/archive/0ad2
Patch0001: varnish-5.1.1.fix_ld_library_path_in_doc_build.patch
Patch0002: gcc-9-stricter-on-NULL-arguments-for-printf.patch
Patch0003: CVE-2019-15892-1.patch
Patch0004: CVE-2019-15892-2.patch
Patch0005: CVE-2019-15892-3.patch
Patch0006: CVE-2019-15892-4.patch
Patch0007: CVE-2019-15892-5.patch
Patch0008: CVE-2019-15892-6.patch
Patch0009: CVE-2019-15892-7.patch
Patch0010: CVE-2019-15892-8.patch
BuildRequires: python3-sphinx python3-docutils pkgconfig make graphviz nghttp2 systemd-units
BuildRequires: ncurses-devel pcre-devel libedit-devel
@ -158,6 +166,9 @@ test -f /etc/varnish/secret || (uuidgen > /etc/varnish/secret && chmod 0600 /etc
%{_mandir}/man7/*.7*
%changelog
* Tue Jan 19 2021 wangyue <wangyue92@huawei.com> - 6.0.0-6
- Fix CVE-2019-15892
* Tue Jun 2 2020 chengzihan <chengzihan2@huawei.com> - 6.0.0-5
- Fix the error of using parameter ("%s", NULL) for 'printf' when built by gcc-9