From a6d6ad761fe77b0412771df85458e4e9cfcff31e Mon Sep 17 00:00:00 2001 From: Ingvar Hagelund Date: Mon, 16 Mar 2015 13:50:34 +0100 Subject: [PATCH] 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(-) create mode 100644 varnish-4.0.3_fix_content_length_bug.patch 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 +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 4.0.3-3 +- Added a patch fixing a crash on bogus content-length header, + closing #1200034 + * Fri Mar 06 2015 Ingvar Hagelund 4.0.3-2 - Added selinux module for varnish4 on el6