[varnish] Added a patch fixing a crash on bogus content-length header,

Ingvar Hagelund ingvar at fedoraproject.org
Mon Mar 16 12:50:37 UTC 2015


commit a6d6ad761fe77b0412771df85458e4e9cfcff31e
Author: Ingvar Hagelund <ingvar at lardal.localdomain>
Date:   Mon Mar 16 13:50:34 2015 +0100

    Added a patch fixing a crash on bogus content-length header,
    
      closing #1200034

 varnish-4.0.3_fix_content_length_bug.patch | 274 +++++++++++++++++++++++++++++
 varnish.spec                               |   8 +-
 2 files changed, 281 insertions(+), 1 deletion(-)
---
diff --git a/varnish-4.0.3_fix_content_length_bug.patch b/varnish-4.0.3_fix_content_length_bug.patch
new file mode 100644
index 0000000..5fe7101
--- /dev/null
+++ b/varnish-4.0.3_fix_content_length_bug.patch
@@ -0,0 +1,274 @@
+This patch is a rebase of commit 9d61ea4d722549a984d912603902fccfac473824
+Author: Martin Blix Grydeland <martin at varnish-software.com>
+Date:   Fri Mar 13 15:23:15 2015 +0100
+
+    Fail fetch on malformed Content-Length header
+    
+    Add a common content length parser that is being used by both client
+    and backend side.
+    
+    Original patch by: fgs
+    
+    Fixes: #1691
+
+diff -Nur varnish-4.0.3.nofix/bin/varnishd/cache/cache.h varnish-4.0.3/bin/varnishd/cache/cache.h
+--- varnish-4.0.3.nofix/bin/varnishd/cache/cache.h	2015-02-18 15:14:11.000000000 +0100
++++ varnish-4.0.3/bin/varnishd/cache/cache.h	2015-03-13 16:22:42.943549723 +0100
+@@ -208,7 +208,7 @@
+  *
+  */
+ 
+-typedef ssize_t htc_read(struct http_conn *, void *, size_t);
++typedef ssize_t htc_read(struct http_conn *, void *, ssize_t);
+ 
+ struct http_conn {
+ 	unsigned		magic;
+@@ -560,7 +560,7 @@
+ 
+ 	struct pool_task	fetch_task;
+ 
+-	char			*h_content_length;
++	ssize_t			content_length;
+ 
+ #define BO_FLAG(l, r, w, d) unsigned	l:1;
+ #include "tbl/bo_flags.h"
+@@ -1014,6 +1014,7 @@
+ int http_GetHdrField(const struct http *hp, const char *hdr,
+     const char *field, char **ptr);
+ double http_GetHdrQ(const struct http *hp, const char *hdr, const char *field);
++ssize_t http_GetContentLength(const struct http *hp);
+ uint16_t http_GetStatus(const struct http *hp);
+ void http_SetStatus(struct http *to, uint16_t status);
+ const char *http_GetReq(const struct http *hp);
+@@ -1040,7 +1041,7 @@
+     unsigned maxbytes, unsigned maxhdr);
+ enum htc_status_e HTTP1_Reinit(struct http_conn *htc);
+ enum htc_status_e HTTP1_Rx(struct http_conn *htc);
+-ssize_t HTTP1_Read(struct http_conn *htc, void *d, size_t len);
++ssize_t HTTP1_Read(struct http_conn *htc, void *d, ssize_t len);
+ enum htc_status_e HTTP1_Complete(struct http_conn *htc);
+ uint16_t HTTP1_DissectRequest(struct req *);
+ uint16_t HTTP1_DissectResponse(struct http *sp, const struct http_conn *htc);
+diff -Nur varnish-4.0.3.nofix/bin/varnishd/cache/cache_http1_fetch.c varnish-4.0.3/bin/varnishd/cache/cache_http1_fetch.c
+--- varnish-4.0.3.nofix/bin/varnishd/cache/cache_http1_fetch.c	2015-02-18 15:14:11.000000000 +0100
++++ varnish-4.0.3/bin/varnishd/cache/cache_http1_fetch.c	2015-03-13 16:22:42.944549727 +0100
+@@ -43,29 +43,6 @@
+ #include "vtcp.h"
+ #include "vtim.h"
+ 
+-/*--------------------------------------------------------------------
+- * Convert a string to a size_t safely
+- */
+-
+-static ssize_t
+-vbf_fetch_number(const char *nbr, int radix)
+-{
+-	uintmax_t cll;
+-	ssize_t cl;
+-	char *q;
+-
+-	if (*nbr == '\0')
+-		return (-1);
+-	cll = strtoumax(nbr, &q, radix);
+-	if (q == NULL || *q != '\0')
+-		return (-1);
+-
+-	cl = (ssize_t)cll;
+-	if((uintmax_t)cl != cll) /* Protect against bogusly large values */
+-		return (-1);
+-	return (cl);
+-}
+-
+ /*--------------------------------------------------------------------*/
+ 
+ static enum vfp_status __match_proto__(vfp_pull_f)
+@@ -167,7 +144,6 @@
+ V1F_Setup_Fetch(struct busyobj *bo)
+ {
+ 	struct http_conn *htc;
+-	ssize_t cl;
+ 
+ 	CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
+ 	htc = &bo->htc;
+@@ -176,13 +152,15 @@
+ 
+ 	switch(htc->body_status) {
+ 	case BS_EOF:
++		assert(bo->content_length == -1);
+ 		VFP_Push(bo, v1f_pull_eof, 0);
+ 		return(-1);
+ 	case BS_LENGTH:
+-		cl = vbf_fetch_number(bo->h_content_length, 10);
+-		VFP_Push(bo, v1f_pull_straight, cl);
+-		return (cl);
++		assert(bo->content_length > 0);
++		VFP_Push(bo, v1f_pull_straight, bo->content_length);
++		return (bo->content_length);
+ 	case BS_CHUNKED:
++		assert(bo->content_length == -1);
+ 		VFP_Push(bo, v1f_pull_chunked, -1);
+ 		return (-1);
+ 	default:
+diff -Nur varnish-4.0.3.nofix/bin/varnishd/cache/cache_http1_fsm.c varnish-4.0.3/bin/varnishd/cache/cache_http1_fsm.c
+--- varnish-4.0.3.nofix/bin/varnishd/cache/cache_http1_fsm.c	2015-02-18 15:14:11.000000000 +0100
++++ varnish-4.0.3/bin/varnishd/cache/cache_http1_fsm.c	2015-03-13 16:22:42.944549727 +0100
+@@ -262,22 +262,22 @@
+ static enum req_body_state_e
+ http1_req_body_status(struct req *req)
+ {
+-	char *ptr, *endp;
++	ssize_t cl;
+ 
+ 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
+ 
+-	if (http_GetHdr(req->http, H_Content_Length, &ptr)) {
+-		AN(ptr);
+-		if (*ptr == '\0')
+-			return (REQ_BODY_FAIL);
+-		req->req_bodybytes = strtoul(ptr, &endp, 10);
+-		if (*endp != '\0' && !vct_islws(*endp))
+-			return (REQ_BODY_FAIL);
+-		if (req->req_bodybytes == 0)
+-			return (REQ_BODY_NONE);
++	req->req_bodybytes = 0;
++	cl = http_GetContentLength(req->http);
++	if (cl == -2)
++		return (REQ_BODY_FAIL);
++	else if (cl == 0)
++		return (REQ_BODY_NONE);
++	else if (cl > 0) {
++		req->req_bodybytes = cl;
+ 		req->h1.bytes_yet = req->req_bodybytes - req->h1.bytes_done;
+ 		return (REQ_BODY_PRESENT);
+ 	}
++	assert(cl == -1);	/* No Content-Length header */
+ 	if (http_HdrIs(req->http, H_Transfer_Encoding, "chunked")) {
+ 		req->chunk_ctr = -1;
+ 		return (REQ_BODY_CHUNKED);
+diff -Nur varnish-4.0.3.nofix/bin/varnishd/cache/cache_http1_proto.c varnish-4.0.3/bin/varnishd/cache/cache_http1_proto.c
+--- varnish-4.0.3.nofix/bin/varnishd/cache/cache_http1_proto.c	2015-02-18 15:14:11.000000000 +0100
++++ varnish-4.0.3/bin/varnishd/cache/cache_http1_proto.c	2015-03-13 16:22:42.944549727 +0100
+@@ -191,14 +191,15 @@
+  * Read up to len bytes, returning pipelined data first.
+  */
+ 
+-ssize_t
+-HTTP1_Read(struct http_conn *htc, void *d, size_t len)
++ssize_t __match_proto__(htc_read)
++HTTP1_Read(struct http_conn *htc, void *d, ssize_t len)
+ {
+ 	size_t l;
+ 	unsigned char *p;
+ 	ssize_t i = 0;
+ 
+ 	CHECK_OBJ_NOTNULL(htc, HTTP_CONN_MAGIC);
++	assert(len > 0);
+ 	l = 0;
+ 	p = d;
+ 	if (htc->pipeline.b) {
+diff -Nur varnish-4.0.3.nofix/bin/varnishd/cache/cache_http.c varnish-4.0.3/bin/varnishd/cache/cache_http.c
+--- varnish-4.0.3.nofix/bin/varnishd/cache/cache_http.c	2015-02-18 15:14:11.000000000 +0100
++++ varnish-4.0.3/bin/varnishd/cache/cache_http.c	2015-03-13 16:22:42.943549723 +0100
+@@ -488,6 +488,35 @@
+ 	return (i);
+ }
+ 
++/*--------------------------------------------------------------------*/
++
++ssize_t
++http_GetContentLength(const struct http *hp)
++{
++	ssize_t cl, cll;
++	char *b;
++
++	CHECK_OBJ_NOTNULL(hp, HTTP_MAGIC);
++
++	if (!http_GetHdr(hp, H_Content_Length, &b))
++		return (-1);
++	cl = 0;
++	if (!vct_isdigit(*b))
++		return (-2);
++	for (;vct_isdigit(*b); b++) {
++		cll = cl;
++		cl *= 10;
++		cl += *b - '0';
++		if (cll != cl / 10)
++			return (-2);
++	}
++	while (vct_islws(*b))
++		b++;
++	if (*b != '\0')
++		return (-2);
++	return (cl);
++}
++
+ /*--------------------------------------------------------------------
+  * XXX: redo with http_GetHdrField() ?
+  */
+diff -Nur varnish-4.0.3.nofix/bin/varnishd/cache/cache_rfc2616.c varnish-4.0.3/bin/varnishd/cache/cache_rfc2616.c
+--- varnish-4.0.3.nofix/bin/varnishd/cache/cache_rfc2616.c	2015-02-18 15:14:11.000000000 +0100
++++ varnish-4.0.3/bin/varnishd/cache/cache_rfc2616.c	2015-03-13 16:22:42.944549727 +0100
+@@ -188,6 +188,7 @@
+ RFC2616_Body(struct busyobj *bo, struct dstat *stats)
+ {
+ 	struct http *hp;
++	ssize_t cl;
+ 	char *b;
+ 
+ 	hp = bo->beresp;
+@@ -199,6 +200,8 @@
+ 	else
+ 		bo->should_close = 0;
+ 
++	bo->content_length = -1;
++
+ 	if (!strcasecmp(http_GetReq(bo->bereq), "head")) {
+ 		/*
+ 		 * A HEAD request can never have a body in the reply,
+@@ -246,9 +249,18 @@
+ 		return (BS_ERROR);
+ 	}
+ 
+-	if (http_GetHdr(hp, H_Content_Length, &bo->h_content_length)) {
+-		stats->fetch_length++;
+-		return (BS_LENGTH);
++	cl = http_GetContentLength(hp);
++	if (cl == -2)
++		return (BS_ERROR);
++	if (cl >= 0) {
++		bo->content_length = cl;
++		if (cl == 0) {
++			stats->fetch_zero++;
++			return (BS_NONE);
++		} else {
++			stats->fetch_length++;
++			return (BS_LENGTH);
++		}
+ 	}
+ 
+ 	if (http_HdrIs(hp, H_Connection, "keep-alive")) {
+diff -Nur varnish-4.0.3.nofix/bin/varnishtest/tests/r01691.vtc varnish-4.0.3/bin/varnishtest/tests/r01691.vtc
+--- varnish-4.0.3.nofix/bin/varnishtest/tests/r01691.vtc	1970-01-01 01:00:00.000000000 +0100
++++ varnish-4.0.3/bin/varnishtest/tests/r01691.vtc	2015-03-13 16:22:42.945549731 +0100
+@@ -0,0 +1,21 @@
++varnishtest "Test bogus Content-Length header"
++
++server s1 {
++	rxreq
++	txresp -nolen -hdr "Content-Length: bogus"
++} -start
++
++varnish v1 -vcl+backend {
++
++} -start
++
++logexpect l1 -v v1 {
++	  expect * 1002	VCL_Error "Body cannot be fetched"
++} -start
++
++client c1 {
++	txreq
++	rxresp
++} -run
++
++logexpect l1 -wait
diff --git a/varnish.spec b/varnish.spec
index 1da58ea..921f649 100644
--- a/varnish.spec
+++ b/varnish.spec
@@ -6,7 +6,7 @@
 Summary: High-performance HTTP accelerator
 Name: varnish
 Version: 4.0.3
-Release: 2%{?v_rc}%{?dist}
+Release: 3%{?v_rc}%{?dist}
 License: BSD
 Group: System Environment/Daemons
 URL: http://www.varnish-cache.org/
@@ -18,6 +18,7 @@ Patch1:  varnish-4.0.2.fix_ld_library_path_in_sphinx_build.patch
 Patch2:  varnish-4.0.3_fix_Werror_el6.patch
 Patch3:  varnish-4.0.3_fix_python24.el5.patch
 Patch4:  varnish-4.0.3_fix_varnish4_selinux.el6.patch
+Patch5:  varnish-4.0.3_fix_content_length_bug.patch
 BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 # To build from git, start with a make dist, see redhat/README.redhat 
 # You will need at least automake autoconf libtool python-docutils
@@ -122,6 +123,7 @@ Minimal selinux policy for running varnish4
 %if 0%{?rhel} == 6
 %patch4 -p0
 %endif
+%patch5 -p1
 
 %build
 #export CFLAGS="$CFLAGS -Wp,-D_FORTIFY_SOURCE=0"
@@ -363,6 +365,10 @@ fi
 %endif
 
 %changelog
+* Fri Mar 13 2015 Ingvar Hagelund <ingvar at redpill-linpro.com> 4.0.3-3
+- Added a patch fixing a crash on bogus content-length header,
+  closing #1200034
+
 * Fri Mar 06 2015 Ingvar Hagelund <ingvar at redpill-linpro.com> 4.0.3-2
 - Added selinux module for varnish4 on el6
 


More information about the scm-commits mailing list