[master] 76ae363 Verify padding and priority flag numbers in h2_rx_headers

[Dag Haavi Finstad]

commit 76ae363534d4e81e35700b99213b4612b56b534c
Author: Dag Haavi Finstad <daghf at varnish-software.com>
Date:   Thu Jul 6 17:26:55 2017 +0200

    Verify padding and priority flag numbers in h2_rx_headers
    
    Unchecked decoding of the padding length and also the priority flag
    turns into an integer underrun when the input length is smaller than
    what we end up subtracting.
    
    rfc7540 doesn't seem to adress what should be the error condition when
    the priority flag is enabled but the payload is smaller than what could
    contain the priority bits. Seeing as a HEADERS frame changes connection
    state I assume closing down via PROTOCOL_ERROR is the proper handling.
    
    Fixes: #2349

diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
index 87dca6e..f820d05 100644
--- a/bin/varnishd/http2/cache_http2_proto.c
+++ b/bin/varnishd/http2/cache_http2_proto.c
@@ -577,14 +577,17 @@ h2_rx_headers(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
 	AN(r2->decode);
 	h2h_decode_init(h2, r2->decode);
 
-	/* XXX: Error handling */
 	p = h2->rxf_data;
 	l = h2->rxf_len;
 	if (h2->rxf_flags & H2FF_HEADERS_PADDED) {
+		if (*p > l)
+			return (H2CE_PROTOCOL_ERROR);	// rfc7540,l,1884,1887
 		l -= 1 + *p;
 		p += 1;
 	}
 	if (h2->rxf_flags & H2FF_HEADERS_PRIORITY) {
+		if (l < 5)
+			return (H2CE_PROTOCOL_ERROR);
 		l -= 5;
 		p += 5;
 	}
diff --git a/bin/varnishtest/tests/t02003.vtc b/bin/varnishtest/tests/t02003.vtc
index 2f32a51..86417a0 100644
--- a/bin/varnishtest/tests/t02003.vtc
+++ b/bin/varnishtest/tests/t02003.vtc
@@ -369,6 +369,41 @@ client c1 {
 	} -run
 } -run
 
+
+#2349: Padding exceeds frame size
+client c1 {
+	stream 1 {
+		sendhex 000001
+		sendhex 01
+		sendhex 09
+		sendhex 00000001
+		sendhex { ff }
+	} -run
+	stream 0 {
+		rxgoaway
+		expect goaway.err == PROTOCOL_ERROR
+		expect goaway.laststream == 1
+	} -run
+	expect_close
+} -run
+
+#2349: Integer underrun may also occur when the padding flag is set
+client c1 {
+	stream 1 {
+		sendhex 000004
+		sendhex 01
+		sendhex 21
+		sendhex 00000001
+		sendhex { aabb ccdd }
+	} -run
+	stream 0 {
+		rxgoaway
+		expect goaway.err == PROTOCOL_ERROR
+		expect goaway.laststream == 1
+	} -run
+	expect_close
+} -run
+
 varnish v1 -vsl_catchup
 
 varnish v1 -expect MEMPOOL.req0.live == 0