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.
This commit is contained in:
Christopher Faulet 2019-02-27 15:15:23 +01:00 committed by Willy Tarreau
parent 2b9b6784b9
commit 5d45e381b4
2 changed files with 18 additions and 9 deletions

View File

@ -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 */

View File

@ -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 */