From 9be22198e258d0e7a5c41f4291792214a29405cf Mon Sep 17 00:00:00 2001

From: Martin Blix Grydeland <martin@varnish-software.com>

Date: Tue, 22 Jun 2021 11:47:55 +0200

Subject: [PATCH] Take content length into account on H/2 request bodies

When receiving H/2 data frames, make sure to take the advertised content

length into account, and fail appropriately if the combined sum of the

data frames does not match the content length.

---

 bin/varnishd/http2/cache_http2.h       |  2 ++

 bin/varnishd/http2/cache_http2_proto.c | 49 ++++++++++++++++++++------

 2 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/bin/varnishd/http2/cache_http2.h b/bin/varnishd/http2/cache_http2.h

index c377d03aac..205b96ccb7 100644

--- a/bin/varnishd/http2/cache_http2.h

+++ b/bin/varnishd/http2/cache_http2.h

@@ -131,6 +131,8 @@ struct h2_req {

 	/* Where to wake this stream up */

 	struct worker			*wrk;

+	ssize_t				reqbody_bytes;

+

 	VTAILQ_ENTRY(h2_req)		tx_list;

 	h2_error			error;

diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c

index cb35bb4873..98f5dc4f37 100644

--- a/bin/varnishd/http2/cache_http2_proto.c

+++ b/bin/varnishd/http2/cache_http2_proto.c

@@ -546,7 +546,7 @@ h2_end_headers(struct worker *wrk, struct h2_sess *h2,

     struct req *req, struct h2_req *r2)

 {

 	h2_error h2e;

-	const char *b;

+	ssize_t cl;

 	ASSERT_RXTHR(h2);

 	assert(r2->state == H2_S_OPEN);

@@ -572,14 +572,24 @@ h2_end_headers(struct worker *wrk, struct h2_sess *h2,

 	// XXX: Have I mentioned H/2 Is hodge-podge ?

 	http_CollectHdrSep(req->http, H_Cookie, "; ");	// rfc7540,l,3114,3120

+	cl = http_GetContentLength(req->http);

+	assert(cl >= -2);

+	if (cl == -2) {

+		VSLb(h2->vsl, SLT_Debug, "Non-parseable Content-Length");

+		return (H2SE_PROTOCOL_ERROR);

+	}

+

 	if (req->req_body_status == REQ_BODY_INIT) {

-		if (!http_GetHdr(req->http, H_Content_Length, &b))

+		if (cl == -1)

 			req->req_body_status = REQ_BODY_WITHOUT_LEN;

 		else

 			req->req_body_status = REQ_BODY_WITH_LEN;

+		req->htc->content_length = cl;

 	} else {

+		/* A HEADER frame contained END_STREAM */

 		assert (req->req_body_status == REQ_BODY_NONE);

-		if (http_GetContentLength(req->http) > 0)

+		r2->state = H2_S_CLOS_REM;

+		if (cl > 0)

 			return (H2CE_PROTOCOL_ERROR); //rfc7540,l,1838,1840

 	}

@@ -736,6 +746,7 @@ h2_rx_data(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)

 	int w1 = 0, w2 = 0;

 	char buf[4];

 	unsigned wi;

+	ssize_t cl;

 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);

 	ASSERT_RXTHR(h2);

@@ -754,6 +765,23 @@ h2_rx_data(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)

 		Lck_Unlock(&h2->sess->mtx);

 		return (h2->error ? h2->error : r2->error);

 	}

+

+	r2->reqbody_bytes += h2->rxf_len;

+	if (h2->rxf_flags & H2FF_DATA_END_STREAM)

+		r2->state = H2_S_CLOS_REM;

+	cl = r2->req->htc->content_length;

+	if (cl >= 0 && (r2->reqbody_bytes > cl ||

+	      (r2->state >= H2_S_CLOS_REM && r2->reqbody_bytes != cl))) {

+		VSLb(h2->vsl, SLT_Debug,

+		    "H2: stream %u: Received data and Content-Length"

+		    " mismatch", h2->rxf_stream);

+		r2->error = H2SE_PROTOCOL_ERROR; // rfc7540,l,3150,3163

+		if (r2->cond)

+			AZ(pthread_cond_signal(r2->cond));

+		Lck_Unlock(&h2->sess->mtx);

+		return (H2SE_PROTOCOL_ERROR);

+	}

+

 	AZ(h2->mailcall);

 	h2->mailcall = r2;

 	h2->req0->r_window -= h2->rxf_len;

@@ -772,6 +800,8 @@ h2_rx_data(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)

 		r2->r_window += wi;

 		w2 = 1;

 	}

+

+

 	Lck_Unlock(&h2->sess->mtx);

 	if (w1 || w2) {

@@ -794,7 +824,7 @@ h2_vfp_body(struct vfp_ctx *vc, struct vfp_entry *vfe, void *ptr, ssize_t *lp)

 	struct h2_req *r2;

 	struct h2_sess *h2;

 	unsigned l;

-	enum vfp_status retval = VFP_OK;

+	enum vfp_status retval;

 	CHECK_OBJ_NOTNULL(vc, VFP_CTX_MAGIC);

 	CHECK_OBJ_NOTNULL(vfe, VFP_ENTRY_MAGIC);

@@ -807,7 +837,6 @@ h2_vfp_body(struct vfp_ctx *vc, struct vfp_entry *vfe, void *ptr, ssize_t *lp)

 	*lp = 0;

 	Lck_Lock(&h2->sess->mtx);

-	assert (r2->state == H2_S_OPEN);

 	r2->cond = &vc->wrk->cond;

 	while (h2->mailcall != r2 && h2->error == 0 && r2->error == 0)

 		AZ(Lck_CondWait(r2->cond, &h2->sess->mtx, 0));

@@ -830,12 +859,10 @@ h2_vfp_body(struct vfp_ctx *vc, struct vfp_entry *vfe, void *ptr, ssize_t *lp)

 			Lck_Unlock(&h2->sess->mtx);

 			return (VFP_OK);

 		}

-		if (h2->rxf_len == 0) {

-			if (h2->rxf_flags & H2FF_DATA_END_STREAM) {

-				retval = VFP_END;

-				r2->state = H2_S_CLOS_REM;

-			}

-		}

+		if (h2->rxf_len == 0 && r2->state >= H2_S_CLOS_REM)

+			retval = VFP_END;

+		else

+			retval = VFP_OK;

 		h2->mailcall = NULL;

 		AZ(pthread_cond_signal(h2->cond));

 	}