From 57bc8917c31ec34f0ac3e12a2738a7166e58ee5e Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Mon, 25 Apr 2016 17:09:40 +0200 Subject: [PATCH] BUG/MEDIUM: log: fix risk of segfault when logging HTTP fields in TCP mode David Torgerson faced an issue when using HTTP fields in log-format in TCP sections. The txn is dereferenced while it's null, resulting in a crash of the process. Such configurations are invalid and a warning is emitted, but nevertheless the process must not crash. As found by Lukas Tribus, this is a side effect of the split between the stream and the HTTP transaction that happened in 1.6, making it possible to have txn==NULL there. The fix consists in checking that txn is valid before using it. Fortunately it's easy since almost all places already used to check for the existence of a field (eg: txn->uri). This patch should be backported to 1.6. --- src/log.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/log.c b/src/log.c index 47a4317e2e..dffef54150 100644 --- a/src/log.c +++ b/src/log.c @@ -1666,7 +1666,7 @@ int build_logline(struct stream *s, char *dst, size_t maxsize, struct list *list break; case LOG_FMT_STATUS: // %ST - ret = ltoa_o(txn->status, tmplog, dst + maxsize - tmplog); + ret = ltoa_o(txn ? txn->status : 0, tmplog, dst + maxsize - tmplog); if (ret == NULL) goto out; tmplog = ret; @@ -1692,7 +1692,7 @@ int build_logline(struct stream *s, char *dst, size_t maxsize, struct list *list break; case LOG_FMT_CCLIENT: // %CC - src = txn->cli_cookie; + src = txn ? txn->cli_cookie : NULL; ret = lf_text(tmplog, src, dst + maxsize - tmplog, tmp); if (ret == NULL) goto out; @@ -1701,7 +1701,7 @@ int build_logline(struct stream *s, char *dst, size_t maxsize, struct list *list break; case LOG_FMT_CSERVER: // %CS - src = txn->srv_cookie; + src = txn ? txn->srv_cookie : NULL; ret = lf_text(tmplog, src, dst + maxsize - tmplog, tmp); if (ret == NULL) goto out; @@ -1719,8 +1719,8 @@ int build_logline(struct stream *s, char *dst, size_t maxsize, struct list *list case LOG_FMT_TERMSTATE_CK: // %tsc, same as TS with cookie state (for mode HTTP) LOGCHAR(sess_term_cond[(s->flags & SF_ERR_MASK) >> SF_ERR_SHIFT]); LOGCHAR(sess_fin_state[(s->flags & SF_FINST_MASK) >> SF_FINST_SHIFT]); - LOGCHAR((be->ck_opts & PR_CK_ANY) ? sess_cookie[(txn->flags & TX_CK_MASK) >> TX_CK_SHIFT] : '-'); - LOGCHAR((be->ck_opts & PR_CK_ANY) ? sess_set_cookie[(txn->flags & TX_SCK_MASK) >> TX_SCK_SHIFT] : '-'); + LOGCHAR((txn && (be->ck_opts & PR_CK_ANY)) ? sess_cookie[(txn->flags & TX_CK_MASK) >> TX_CK_SHIFT] : '-'); + LOGCHAR((txn && (be->ck_opts & PR_CK_ANY)) ? sess_set_cookie[(txn->flags & TX_SCK_MASK) >> TX_SCK_SHIFT] : '-'); last_isspace = 0; break; @@ -1885,7 +1885,7 @@ int build_logline(struct stream *s, char *dst, size_t maxsize, struct list *list /* Request */ if (tmp->options & LOG_OPT_QUOTE) LOGCHAR('"'); - uri = txn->uri ? txn->uri : ""; + uri = txn && txn->uri ? txn->uri : ""; ret = lf_encode_string(tmplog, dst + maxsize, '#', url_encode_map, uri, tmp); if (ret == NULL || *ret != '\0') @@ -1897,7 +1897,7 @@ int build_logline(struct stream *s, char *dst, size_t maxsize, struct list *list break; case LOG_FMT_HTTP_PATH: // %HP - uri = txn->uri ? txn->uri : ""; + uri = txn && txn->uri ? txn->uri : ""; if (tmp->options & LOG_OPT_QUOTE) LOGCHAR('"'); @@ -1917,7 +1917,7 @@ int build_logline(struct stream *s, char *dst, size_t maxsize, struct list *list while (spc < end && *spc != '?' && !HTTP_IS_SPHT(*spc)) spc++; - if (!txn->uri || nspaces == 0) { + if (!txn || txn->uri || nspaces == 0) { chunk.str = ""; chunk.len = strlen(""); } else { @@ -1940,7 +1940,7 @@ int build_logline(struct stream *s, char *dst, size_t maxsize, struct list *list if (tmp->options & LOG_OPT_QUOTE) LOGCHAR('"'); - if (!txn->uri) { + if (!txn || !txn->uri) { chunk.str = ""; chunk.len = strlen(""); } else { @@ -1971,7 +1971,7 @@ int build_logline(struct stream *s, char *dst, size_t maxsize, struct list *list break; case LOG_FMT_HTTP_URI: // %HU - uri = txn->uri ? txn->uri : ""; + uri = txn && txn->uri ? txn->uri : ""; if (tmp->options & LOG_OPT_QUOTE) LOGCHAR('"'); @@ -1991,7 +1991,7 @@ int build_logline(struct stream *s, char *dst, size_t maxsize, struct list *list while (spc < end && !HTTP_IS_SPHT(*spc)) spc++; - if (!txn->uri || nspaces == 0) { + if (!txn || !txn->uri || nspaces == 0) { chunk.str = ""; chunk.len = strlen(""); } else { @@ -2011,7 +2011,7 @@ int build_logline(struct stream *s, char *dst, size_t maxsize, struct list *list break; case LOG_FMT_HTTP_METHOD: // %HM - uri = txn->uri ? txn->uri : ""; + uri = txn && txn->uri ? txn->uri : ""; if (tmp->options & LOG_OPT_QUOTE) LOGCHAR('"'); @@ -2041,7 +2041,7 @@ int build_logline(struct stream *s, char *dst, size_t maxsize, struct list *list break; case LOG_FMT_HTTP_VERSION: // %HV - uri = txn->uri ? txn->uri : ""; + uri = txn && txn->uri ? txn->uri : ""; if (tmp->options & LOG_OPT_QUOTE) LOGCHAR('"'); @@ -2063,7 +2063,7 @@ int build_logline(struct stream *s, char *dst, size_t maxsize, struct list *list while (uri < end && HTTP_IS_SPHT(*uri)) uri++; - if (!txn->uri || nspaces == 0) { + if (!txn || !txn->uri || nspaces == 0) { chunk.str = ""; chunk.len = strlen(""); } else if (uri == end) { -- 2.39.5