From 6a7b5ba87b536c510b0ce849f26caf6f1b4d54bf Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Wed, 12 Aug 2020 14:04:52 +0200 Subject: [PATCH] BUG/MEDIUM: htx: smp_prefetch_htx() must always validate the direction It is possible to process a channel based on desynchronized info if a request fetch is called from a response and conversely. However, the code in smp_prefetch_htx() already makes sure the analysis has already started before trying to fetch from a buffer, so the problem effectively lies in response rules making use of request expressions only. Usually it's not a problem as extracted data are checked against the current HTTP state, except when it comes to the start line, which is usually accessed directly from sample fetch functions such as status, path, url, url32, query and so on. In this case, trying to access the request buffer from the response path will lead to unpredictable results. When building with DEBUG_STRICT, a process violating these rules will simply die after emitting: FATAL: bug condition "htx->first == -1" matched at src/http_htx.c:67 But when this is not enabled, it may or may not crash depending on what the pending request buffer data look like when trying to spot a start line there. This is typically what happens in issue #806. This patch adds a test in smp_prefetch_htx() so that it does not try to parse an HTX buffer in a channel belonging to the wrong direction. There's one special case on the "method" sample fetch since it can retrieve info even without a buffer, from the other direction, as long as the method is one of the well known ones. Three, we call smp_prefetch_htx() only if needed. This was reported in 2.0 and must be backported there (oldest stable version with HTX). (cherry picked from commit a6d9879e69d65e8821347860394829c4dc72f937) Signed-off-by: Willy Tarreau (cherry picked from commit 05f3910f58a8d5e0710a4a8eea82f8bc24861e32) [wt: adjusted context; chn already checked; 3 args to smp_prefetch_htx()] Signed-off-by: Willy Tarreau --- src/http_fetch.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/http_fetch.c b/src/http_fetch.c index 5f94369..738cc30 100644 --- a/src/http_fetch.c +++ b/src/http_fetch.c @@ -167,6 +167,8 @@ static int get_http_auth(struct sample *smp, struct htx *htx) * decide whether or not an HTTP message is present ; * NULL if the requested data cannot be fetched or if it is certain that * we'll never have any HTTP message there ; + * NULL if the sample's direction does not match the channel's (i.e. the + * function was asked to work on the wrong channel) * The HTX message if ready */ struct htx *smp_prefetch_htx(struct sample *smp, struct channel *chn, int vol) @@ -184,6 +186,10 @@ struct htx *smp_prefetch_htx(struct sample *smp, struct channel *chn, int vol) if (!s || !chn) return NULL; + if (((smp->opt & SMP_OPT_DIR) == SMP_OPT_DIR_REQ && (chn->flags & CF_ISRESP)) || + ((smp->opt & SMP_OPT_DIR) == SMP_OPT_DIR_RES && !(chn->flags & CF_ISRESP))) + return 0; + if (!s->txn) { if (unlikely(!http_alloc_txn(s))) return NULL; /* not enough memory */ @@ -304,24 +310,29 @@ struct htx *smp_prefetch_htx(struct sample *smp, struct channel *chn, int vol) static int smp_fetch_meth(const struct arg *args, struct sample *smp, const char *kw, void *private) { struct channel *chn = SMP_REQ_CHN(smp); - struct htx *htx = smp_prefetch_htx(smp, chn, 0); struct http_txn *txn; int meth; - if (!htx) + txn = smp->strm->txn; + if (!txn) return 0; - txn = smp->strm->txn; meth = txn->meth; smp->data.type = SMP_T_METH; smp->data.u.meth.meth = meth; if (meth == HTTP_METH_OTHER) { + struct htx *htx; struct htx_sl *sl; if ((smp->opt & SMP_OPT_DIR) == SMP_OPT_DIR_RES) { /* ensure the indexes are not affected */ return 0; } + + htx = smp_prefetch_htx(smp, chn, 0); + if (!htx) + return 0; + sl = http_get_stline(htx); smp->flags |= SMP_F_CONST; smp->data.u.meth.str.area = HTX_SL_REQ_MPTR(sl); -- 1.7.10.4