[4.1] c1fa324 Fix buffer overflow in HTC_RxInit on workspace exhaustion

Martin Blix Grydeland martin at varnish-software.com
Mon Dec 5 16:00:06 CET 2016


commit c1fa32485906131013ccf252304487fd613708e1
Author: Martin Blix Grydeland <martin at varnish-software.com>
Date:   Mon Nov 28 16:41:21 2016 +0100

    Fix buffer overflow in HTC_RxInit on workspace exhaustion
    
    HTC_RxInit and HTC_RxReInit could write a single '\0' NUL character
    outside of the workspace when its called and there is zero bytes left
    in the workspace. This would trigger the workspace canary causing
    subsequent assertion.
    
    Fix by releaving HTC_RxInit and HTC_RxReInit of adding the '\0'
    character.
    
    HTC_RxStuff and V1F_FetchRespHdr returns HTC_S_OVERFLOW if the
    available buffer space is zero. Both make sure to insert the '\0'
    character just before calling the completion check function.
    
    Note that this fix does not change the fact that we have exchausted
    the workspace and are unable to continue. Varnishd will panic
    nonetheless, but at least we have not stepped out of our boundries.
    
    Ref: #1834

diff --git a/bin/varnishd/cache/cache_session.c b/bin/varnishd/cache/cache_session.c
index 6c5930b..8c9c3b6 100644
--- a/bin/varnishd/cache/cache_session.c
+++ b/bin/varnishd/cache/cache_session.c
@@ -154,7 +154,6 @@ SES_RxInit(struct http_conn *htc, struct ws *ws, unsigned maxbytes,
 	(void)WS_Reserve(htc->ws, htc->maxbytes);
 	htc->rxbuf_b = ws->f;
 	htc->rxbuf_e = ws->f;
-	*htc->rxbuf_e = '\0';
 	htc->pipeline_b = NULL;
 	htc->pipeline_e = NULL;
 }
@@ -183,7 +182,6 @@ SES_RxReInit(struct http_conn *htc)
 		htc->pipeline_b = NULL;
 		htc->pipeline_e = NULL;
 	}
-	*htc->rxbuf_e = '\0';
 }
 
 /*----------------------------------------------------------------------
@@ -206,13 +204,24 @@ SES_RxStuff(struct http_conn *htc, htc_complete_f *func,
 	int i;
 
 	CHECK_OBJ_NOTNULL(htc, HTTP_CONN_MAGIC);
+	AN(htc->ws->r);
+	AN(htc->rxbuf_b);
+	assert(htc->rxbuf_b <= htc->rxbuf_e);
 
 	AZ(isnan(tn));
 	if (t1 != NULL)
 		assert(isnan(*t1));
 
+	if (htc->rxbuf_e == htc->ws->r) {
+		/* Can't work with a zero size buffer */
+		WS_ReleaseP(htc->ws, htc->rxbuf_b);
+		return (HTC_S_OVERFLOW);
+	}
+
 	while (1) {
 		now = VTIM_real();
+		assert(htc->rxbuf_e < htc->ws->r);
+		*htc->rxbuf_e = '\0';
 		hs = func(htc);
 		if (hs == HTC_S_OVERFLOW || hs == HTC_S_JUNK) {
 			WS_ReleaseP(htc->ws, htc->rxbuf_b);
@@ -248,10 +257,9 @@ SES_RxStuff(struct http_conn *htc, htc_complete_f *func,
 		if (i == 0 || i == -1) {
 			WS_ReleaseP(htc->ws, htc->rxbuf_b);
 			return (HTC_S_EOF);
-		} else if (i > 0) {
+		} else if (i > 0)
 			htc->rxbuf_e += i;
-			*htc->rxbuf_e = '\0';
-		} else if (i == -2) {
+		else if (i == -2) {
 			if (hs == HTC_S_EMPTY && ti <= now) {
 				WS_ReleaseP(htc->ws, htc->rxbuf_b);
 				return (HTC_S_IDLE);
diff --git a/bin/varnishd/http1/cache_http1_proto.c b/bin/varnishd/http1/cache_http1_proto.c
index e4c9053..3f3e45f 100644
--- a/bin/varnishd/http1/cache_http1_proto.c
+++ b/bin/varnishd/http1/cache_http1_proto.c
@@ -80,7 +80,6 @@ HTTP1_Complete(struct http_conn *htc)
 	if (p == htc->rxbuf_e) {
 		/* All white space */
 		htc->rxbuf_e = htc->rxbuf_b;
-		*htc->rxbuf_e = '\0';
 		return (HTC_S_EMPTY);
 	}
 	/*



More information about the varnish-commit mailing list