From: Christopher Faulet <cfaulet@haproxy.com>
Date: Wed, 27 Feb 2019 14:15:23 +0000 (+0100)
Subject: BUG/MINOR: stats: Be more strict on what is a valid request to the stats applet
X-Git-Tag: v2.0-dev2~52
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5d45e381b44fe0faf1a8dda92b471ee92a289f91;p=thirdparty%2Fhaproxy.git

BUG/MINOR: stats: Be more strict on what is a valid request to the stats applet

First of all, only GET, HEAD and POST methods are now allowed. Others will be
rejected with the status code STAT_STATUS_IVAL (invalid request). Then, for the
legacy HTTP, only POST requests with a content-length are allowed. Now, chunked
encoded requests are also considered as invalid because the chunk formatting
will interfere with the parsing of POST parameters. In HTX, It is not a problem
because data are unchunked.

This patch must be backported to 1.9. For prior versions too, but HTX part must
be removed. The patch introducing the status code STAT_STATUS_IVAL must also be
backported.
---

diff --git a/src/proto_http.c b/src/proto_http.c
index 7bf0e20f28..4190313d59 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -1311,22 +1311,27 @@ int http_handle_stats(struct stream *s, struct channel *req)
 		}
 	}
 
-	/* Was the status page requested with a POST ? */
-	if (unlikely(txn->meth == HTTP_METH_POST && txn->req.body_len > 0)) {
+	if (txn->meth == HTTP_METH_GET || txn->meth == HTTP_METH_HEAD)
+		appctx->st0 = STAT_HTTP_HEAD;
+	else if (txn->meth == HTTP_METH_POST && (msg->flags & HTTP_MSGF_CNT_LEN)) {
 		if (appctx->ctx.stats.flags & STAT_ADMIN) {
 			/* we'll need the request body, possibly after sending 100-continue */
-			if (msg->msg_state < HTTP_MSG_CHUNK_SIZE)
+			if (msg->msg_state < HTTP_MSG_DATA)
 				req->analysers |= AN_REQ_HTTP_BODY;
 			appctx->st0 = STAT_HTTP_POST;
 		}
 		else {
+			/* POST without admin level */
+			appctx->ctx.stats.flags &= ~STAT_CHUNKED;
 			appctx->ctx.stats.st_code = STAT_STATUS_DENY;
 			appctx->st0 = STAT_HTTP_LAST;
 		}
 	}
 	else {
-		/* So it was another method (GET/HEAD) */
-		appctx->st0 = STAT_HTTP_HEAD;
+		/* Unsupported method or chunked POST */
+		appctx->ctx.stats.flags &= ~STAT_CHUNKED;
+		appctx->ctx.stats.st_code = STAT_STATUS_IVAL;
+		appctx->st0 = STAT_HTTP_LAST;
 	}
 
 	s->task->nice = -32; /* small boost for HTTP statistics */
diff --git a/src/proto_htx.c b/src/proto_htx.c
index 255199edf5..a65297dc08 100644
--- a/src/proto_htx.c
+++ b/src/proto_htx.c
@@ -4952,8 +4952,9 @@ static int htx_handle_stats(struct stream *s, struct channel *req)
 		}
 	}
 
-	/* Was the status page requested with a POST ? */
-	if (unlikely(txn->meth == HTTP_METH_POST)) {
+	if (txn->meth == HTTP_METH_GET || txn->meth == HTTP_METH_HEAD)
+		appctx->st0 = STAT_HTTP_HEAD;
+	else if (txn->meth == HTTP_METH_POST) {
 		if (appctx->ctx.stats.flags & STAT_ADMIN) {
 			/* we'll need the request body, possibly after sending 100-continue */
 			if (msg->msg_state < HTTP_MSG_DATA)
@@ -4961,14 +4962,17 @@ static int htx_handle_stats(struct stream *s, struct channel *req)
 			appctx->st0 = STAT_HTTP_POST;
 		}
 		else {
+			/* POST without admin level */
 			appctx->ctx.stats.flags &= ~STAT_CHUNKED;
 			appctx->ctx.stats.st_code = STAT_STATUS_DENY;
 			appctx->st0 = STAT_HTTP_LAST;
 		}
 	}
 	else {
-		/* So it was another method (GET/HEAD) */
-		appctx->st0 = STAT_HTTP_HEAD;
+		/* Unsupported method */
+		appctx->ctx.stats.flags &= ~STAT_CHUNKED;
+		appctx->ctx.stats.st_code = STAT_STATUS_IVAL;
+		appctx->st0 = STAT_HTTP_LAST;
 	}
 
 	s->task->nice = -32; /* small boost for HTTP statistics */