222 lines
6.2 KiB
Diff
222 lines
6.2 KiB
Diff
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)
|