tags 939333 + patch
thanks
On Tue, Sep 03, 2019 at 02:27:33PM +0200, Salvatore Bonaccorso wrote:
> See https://varnish-cache.org/security/VSV00003.html . A CVE does not
> seem yet to be assigned (but a request pending now).
I made a backport to 6.1.1 for stable. It consists of all changes between
6.2.0 and 6.2.1 in git, except:
- No bumping of version number or changelog.
- Removed some unrelated change of #include order in otherwise untouched
files.
- The tests are not included (probably should be; I removed them as part
of trying to backport in a slightly different fashion).
- bin/varnishtest/vtc_http.c needed additional changes from the 6.2.0 set
to have the end pointer vct_iscrlf() and vct_skipcrlf() now need;
since varnishtest is not meant to be run against untrusted servers,
I changed the patch to simply use the old, insecure versions
(now named vct_iscrlf_unsafe() and vct_skipcrlf_unsafe()).
- The diff has been refreshed to fix line numbers.
With this patch in debian/patches/, Varnish compiles and appears to run
normally (we already use it in production).
/* Steinar */
--
Homepage: https://www.sesse.net/
Index: varnish-6.1.1/bin/varnishd/cache/cache_http.c
===================================================================
--- varnish-6.1.1.orig/bin/varnishd/cache/cache_http.c
+++ varnish-6.1.1/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') &&
Index: varnish-6.1.1/bin/varnishd/http1/cache_http1_proto.c
===================================================================
--- varnish-6.1.1.orig/bin/varnishd/http1/cache_http1_proto.c
+++ varnish-6.1.1/bin/varnishd/http1/cache_http1_proto.c
@@ -111,6 +111,7 @@ http1_dissect_hdrs(struct http *hp, char
unsigned maxhdr)
{
char *q, *r, *s;
+ int i;
assert(p > htc->rxbuf_b);
assert(p <= htc->rxbuf_e);
@@ -121,31 +122,34 @@ http1_dissect_hdrs(struct http *hp, char
/* 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)) {
+ 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);
- if (r >= htc->rxbuf_e)
+ r += i;
+ assert(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 < r)
+ *q++ = ' ';
+ while (q < htc->rxbuf_e && vct_issp(*q))
*q++ = ' ';
}
@@ -206,11 +210,9 @@ http1_dissect_hdrs(struct http *hp, char
return (400);
}
}
- /* We cannot use vct_skipcrlf() we have to respect rxbuf_e */
- if (p+2 <= htc->rxbuf_e && p[0] == '\r' && p[1] == '\n')
- p += 2;
- else if (p+1 <= htc->rxbuf_e && p[0] == '\n')
- p += 1;
+ 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);
@@ -224,7 +226,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,26 +267,34 @@ http1_splitline(struct http *hp, struct
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 (; !vct_iscrlf(p); p++) {
- if (vct_isctl(*p) && !vct_issp(*p)) {
- hp->hd[hf[2]].b = NULL;
+ q = p;
+ for (; p < htc->rxbuf_e && !vct_iscrlf(p, htc->rxbuf_e); p++) {
+ 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_skipcrlf(p);
+ i = vct_iscrlf(p, htc->rxbuf_e);
+ if (!i)
+ return (400);
*p = '\0';
p += i;
Index: varnish-6.1.1/include/vct.h
===================================================================
--- varnish-6.1.1.orig/include/vct.h
+++ varnish-6.1.1/include/vct.h
@@ -30,6 +30,8 @@
/* from libvarnish/vct.c */
+#include "vas.h"
+
#define VCT_SP (1<<0)
#define VCT_CRLF (1<<1)
#define VCT_LWS (VCT_CRLF | VCT_SP)
@@ -76,7 +78,26 @@ 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));
+}
+
+#define vct_iscrlf_unsafe(p) (((p)[0] == 0x0d && (p)[1] == 0x0a) || (p)[0] == 0x0a)
+#define vct_skipcrlf_unsafe(p) ((p)[0] == 0x0d && (p)[1] == 0x0a ? 2 : 1)
+
Index: varnish-6.1.1/bin/varnishtest/vtc_http.c
===================================================================
--- varnish-6.1.1.orig/bin/varnishtest/vtc_http.c
+++ varnish-6.1.1/bin/varnishtest/vtc_http.c
@@ -428,20 +428,20 @@ http_splitheader(struct http *hp, int re
hh[n++] = p;
while (!vct_islws(*p))
p++;
- AZ(vct_iscrlf(p));
+ AZ(vct_iscrlf_unsafe(p));
*p++ = '\0';
/* URL/STATUS */
while (vct_issp(*p)) /* XXX: H space only */
p++;
- AZ(vct_iscrlf(p));
+ AZ(vct_iscrlf_unsafe(p));
hh[n++] = p;
while (!vct_islws(*p))
p++;
- if (vct_iscrlf(p)) {
+ if (vct_iscrlf_unsafe(p)) {
hh[n++] = NULL;
q = p;
- p += vct_skipcrlf(p);
+ p += vct_skipcrlf_unsafe(p);
*q = '\0';
} else {
*p++ = '\0';
@@ -449,26 +449,26 @@ http_splitheader(struct http *hp, int re
while (vct_issp(*p)) /* XXX: H space only */
p++;
hh[n++] = p;
- while (!vct_iscrlf(p))
+ while (!vct_iscrlf_unsafe(p))
p++;
q = p;
- p += vct_skipcrlf(p);
+ p += vct_skipcrlf_unsafe(p);
*q = '\0';
}
assert(n == 3);
while (*p != '\0') {
assert(n < MAX_HDR);
- if (vct_iscrlf(p))
+ if (vct_iscrlf_unsafe(p))
break;
hh[n++] = p++;
- while (*p != '\0' && !vct_iscrlf(p))
+ while (*p != '\0' && !vct_iscrlf_unsafe(p))
p++;
q = p;
- p += vct_skipcrlf(p);
+ p += vct_skipcrlf_unsafe(p);
*q = '\0';
}
- p += vct_skipcrlf(p);
+ p += vct_skipcrlf_unsafe(p);
assert(*p == '\0');
for (n = 0; n < 3 || hh[n] != NULL; n++) {
@@ -566,13 +566,13 @@ http_rxchunk(struct http *hp)
l = hp->prxbuf;
if (http_rxchar(hp, 2, 0) < 0)
return (-1);
- if (!vct_iscrlf(hp->rxbuf + l)) {
+ if (!vct_iscrlf_unsafe(hp->rxbuf + l)) {
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_unsafe(hp->rxbuf + l + 1)) {
vtc_log(hp->vl, hp->fatal,
"Wrong chunk tail[1] = %02x",
hp->rxbuf[l + 1] & 0xff);
