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.

diff --git a/src/log.c b/src/log.c
index 47a4317e2e6126c100127c05b7ca5315309ed89c..dffef54150f2b468f5f37146d5ceeefdd028d850 100644 (file)
--- 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 : "<BADREQ>";
+                               uri = txn && txn->uri ? txn->uri : "<BADREQ>";
                                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 : "<BADREQ>";
+                               uri = txn && txn->uri ? txn->uri : "<BADREQ>";
 
                                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 = "<BADREQ>";
                                        chunk.len = strlen("<BADREQ>");
                                } 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 = "<BADREQ>";
                                        chunk.len = strlen("<BADREQ>");
                                } 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 : "<BADREQ>";
+                               uri = txn && txn->uri ? txn->uri : "<BADREQ>";
 
                                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 = "<BADREQ>";
                                        chunk.len = strlen("<BADREQ>");
                                } 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 : "<BADREQ>";
+                               uri = txn && txn->uri ? txn->uri : "<BADREQ>";
                                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 : "<BADREQ>";
+                               uri = txn && txn->uri ? txn->uri : "<BADREQ>";
                                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 = "<BADREQ>";
                                        chunk.len = strlen("<BADREQ>");
                                } else if (uri == end) {