From 4bd5b7991bf602a6c46dd0d65fc04d4b8d9667a6 Mon Sep 17 00:00:00 2001
From: Martin Blix Grydeland <martin@varnish-software.com>
Date: Wed, 30 Oct 2013 13:48:20 +0100
Subject: [PATCH] Make up our mind:  Any req.* we receive from the client with
 fundamental trouble gets failed back without VCL involvement.
References: https://www.varnish-cache.org/trac/ticket/1367
References: CVE-2013-4484
References: https://bugzilla.novell.com/show_bug.cgi?id=848451

Fixes	#1367
---
 bin/varnishd/cache_center.c      | 28 +++++++++++++++-------------
 bin/varnishd/cache_http.c        |  2 +-
 bin/varnishtest/tests/r01367.vtc | 30 ++++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 14 deletions(-)
 create mode 100644 bin/varnishtest/tests/r01367.vtc

diff --git a/bin/varnishd/cache_center.c b/bin/varnishd/cache_center.c
index 19eb2ce..fdf7cee 100644
--- a/bin/varnishd/cache_center.c
+++ b/bin/varnishd/cache_center.c
@@ -1474,9 +1474,12 @@ DOT start -> recv [style=bold,color=green]
 static int
 cnt_start(struct sess *sp)
 {
-	uint16_t done;
+	uint16_t err_code;
 	char *p;
-	const char *r = "HTTP/1.1 100 Continue\r\n\r\n";
+	const char *r_100 = "HTTP/1.1 100 Continue\r\n\r\n";
+	const char *r_400 = "HTTP/1.1 400 Bad Request\r\n\r\n";
+	const char *r_413 = "HTTP/1.1 413 Request Entity Too Large\r\n\r\n";
+	const char *r_417 = "HTTP/1.1 417 Expectation Failed\r\n\r\n";
 
 	CHECK_OBJ_NOTNULL(sp, SESS_MAGIC);
 	AZ(sp->restarts);
@@ -1499,10 +1502,14 @@ cnt_start(struct sess *sp)
 	sp->wrk->vcl = NULL;
 
 	http_Setup(sp->http, sp->ws);
-	done = http_DissectRequest(sp);
+	err_code = http_DissectRequest(sp);
 
 	/* If we could not even parse the request, just close */
-	if (done == 400) {
+	if (err_code == 400)
+		(void)write(sp->fd, r_400, strlen(r_400));
+	else if (err_code == 413)
+		(void)write(sp->fd, r_413, strlen(r_413));
+	if (err_code != 0) {
 		sp->step = STP_DONE;
 		vca_close_session(sp, "junk");
 		return (0);
@@ -1514,12 +1521,6 @@ cnt_start(struct sess *sp)
 	/* Catch original request, before modification */
 	HTTP_Copy(sp->http0, sp->http);
 
-	if (done != 0) {
-		sp->err_code = done;
-		sp->step = STP_ERROR;
-		return (0);
-	}
-
 	sp->doclose = http_DoConnection(sp->http);
 
 	/* XXX: Handle TRACE & OPTIONS of Max-Forwards = 0 */
@@ -1529,13 +1530,14 @@ cnt_start(struct sess *sp)
 	 */
 	if (http_GetHdr(sp->http, H_Expect, &p)) {
 		if (strcasecmp(p, "100-continue")) {
-			sp->err_code = 417;
-			sp->step = STP_ERROR;
+			(void)write(sp->fd, r_417, strlen(r_417));
+			sp->step = STP_DONE;
+			vca_close_session(sp, "junk");
 			return (0);
 		}
 
 		/* XXX: Don't bother with write failures for now */
-		(void)write(sp->fd, r, strlen(r));
+		(void)write(sp->fd, r_100, strlen(r_100));
 		/* XXX: When we do ESI includes, this is not removed
 		 * XXX: because we use http0 as our basis.  Believed
 		 * XXX: safe, but potentially confusing.
diff --git a/bin/varnishd/cache_http.c b/bin/varnishd/cache_http.c
index 8753acc..605975b 100644
--- a/bin/varnishd/cache_http.c
+++ b/bin/varnishd/cache_http.c
@@ -601,7 +601,7 @@ http_splitline(struct worker *w, int fd, struct http *hp,
 	hp->hd[h2].e = p;
 
 	if (!Tlen(hp->hd[h2]))
-		return (413);
+		return (400);
 
 	/* Skip SP */
 	for (; vct_issp(*p); p++) {
diff --git a/bin/varnishtest/tests/r01367.vtc b/bin/varnishtest/tests/r01367.vtc
new file mode 100644
index 0000000..e1de20a
--- /dev/null
+++ b/bin/varnishtest/tests/r01367.vtc
@@ -0,0 +1,30 @@
+varnishtest "blank GET"
+
+server s1 {
+	rxreq
+	txresp
+} -start
+
+varnish v1 -vcl+backend { 
+	sub vcl_error {
+		return (restart);
+	}
+} -start
+
+client c1 {
+	send "GET    \nHost: example.com\n\n"
+	rxresp
+	expect resp.status == 400
+} -run
+
+client c1 {
+	txreq -hdr "Expect: Santa-Claus"
+	rxresp
+	expect resp.status == 417
+} -run
+
+client c1 {
+	txreq
+	rxresp
+	expect resp.status == 200
+} -run
-- 
1.8.2