From 946e55d8ebc06f22c774f3439c4c9034c267dde6 Mon Sep 17 00:00:00 2001 From: William Lallemand Date: Mon, 8 Mar 2021 15:26:48 +0100 Subject: [PATCH] BUG/MEDIUM: session: NULL dereference possible when accessing the listener When implementing a client applet, a NULL dereference was encountered on the error path which increment the counters. Indeed, the counters incremented are the one in the listener which does not exist in the case of client applets, so in sess->listener->counters, listener is NULL. This patch fixes the access to the listener structure when accessing from a sesssion, most of the access are the counters in error paths. Must be backported as far as 1.8. (cherry picked from commit 36119de182154b1f87e0cdf4bd1efba9e2e64113) [wt: minor ctx adjustments in http_ana and mux_h1] Signed-off-by: Willy Tarreau (cherry picked from commit 656730d92f1c3afaed0baf67911d4ee055528e2e) Signed-off-by: Christopher Faulet (cherry picked from commit 35054f1d6cc549dbd53832d551f7f9f446a0d18d) Signed-off-by: Christopher Faulet --- src/fcgi-app.c | 2 +- src/http_ana.c | 26 +++++++++++++------------- src/proto_tcp.c | 2 +- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/fcgi-app.c b/src/fcgi-app.c index 2f54129..01f2011 100644 --- a/src/fcgi-app.c +++ b/src/fcgi-app.c @@ -478,7 +478,7 @@ static int fcgi_flt_http_headers(struct stream *s, struct filter *filter, struct _HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_rewrites, 1); if (sess->fe != s->be) _HA_ATOMIC_ADD(&s->be->be_counters.failed_rewrites, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->failed_rewrites, 1); hdr_rule_err: node = ebpt_first(&hdr_rules); diff --git a/src/http_ana.c b/src/http_ana.c index 7ab8470..9122c57 100644 --- a/src/http_ana.c +++ b/src/http_ana.c @@ -435,14 +435,14 @@ int http_wait_for_request(struct stream *s, struct channel *req, int an_bit) if (!(s->flags & SF_ERR_MASK)) s->flags |= SF_ERR_INTERNAL; _HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_req, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->failed_req, 1); goto return_prx_cond; return_bad_req: txn->status = 400; _HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_req, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->failed_req, 1); /* fall through */ @@ -650,7 +650,7 @@ int http_process_req_common(struct stream *s, struct channel *req, int an_bit, s _HA_ATOMIC_ADD(&sess->fe->fe_counters.denied_req, 1); if (sess->fe != s->be) _HA_ATOMIC_ADD(&s->be->be_counters.denied_req, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->denied_req, 1); goto done_without_exp; @@ -669,7 +669,7 @@ int http_process_req_common(struct stream *s, struct channel *req, int an_bit, s _HA_ATOMIC_ADD(&sess->fe->fe_counters.denied_req, 1); if (sess->fe != s->be) _HA_ATOMIC_ADD(&s->be->be_counters.denied_req, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->denied_req, 1); goto return_prx_cond; @@ -678,7 +678,7 @@ int http_process_req_common(struct stream *s, struct channel *req, int an_bit, s http_reply_and_close(s, txn->status, http_error_message(s)); _HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_req, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->failed_req, 1); return_prx_cond: @@ -913,7 +913,7 @@ int http_process_request(struct stream *s, struct channel *req, int an_bit) * in case we previously disabled it, otherwise we might cause * the client to delay further data. */ - if ((sess->listener->options & LI_O_NOQUICKACK) && + if (sess->listener && (sess->listener->options & LI_O_NOQUICKACK) && (htx_get_tail_type(htx) != HTX_BLK_EOM)) conn_set_quickack(cli_conn, 1); @@ -935,7 +935,7 @@ int http_process_request(struct stream *s, struct channel *req, int an_bit) http_reply_and_close(s, txn->status, http_error_message(s)); _HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_req, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->failed_req, 1); if (!(s->flags & SF_ERR_MASK)) @@ -1102,7 +1102,7 @@ int http_wait_for_request_body(struct stream *s, struct channel *req, int an_bit req->analysers &= AN_REQ_FLT_END; _HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_req, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->failed_req, 1); DBG_TRACE_DEVEL("leaving on error", STRM_EV_STRM_ANA|STRM_EV_HTTP_ANA|STRM_EV_HTTP_ERR, s, txn); @@ -1345,7 +1345,7 @@ int http_request_forward_body(struct stream *s, struct channel *req, int an_bit) return_bad_req: _HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_req, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->failed_req, 1); if (!(s->flags & SF_ERR_MASK)) s->flags |= SF_ERR_CLICL; @@ -1961,7 +1961,7 @@ int http_process_res_common(struct stream *s, struct channel *rep, int an_bit, s _HA_ATOMIC_ADD(&s->be->be_counters.denied_resp, 1); _HA_ATOMIC_ADD(&sess->fe->fe_counters.denied_resp, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->denied_resp, 1); goto return_srv_prx_502; } @@ -2098,7 +2098,7 @@ int http_process_res_common(struct stream *s, struct channel *rep, int an_bit, s _HA_ATOMIC_ADD(&s->be->be_counters.denied_resp, 1); _HA_ATOMIC_ADD(&sess->fe->fe_counters.denied_resp, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->denied_resp, 1); ha_alert("Blocking cacheable cookie in response from instance %s, server %s.\n", @@ -3002,7 +3002,7 @@ static enum rule_result http_req_get_intercept_rule(struct proxy *px, struct lis _HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_rewrites, 1); if (sess->fe != s->be) _HA_ATOMIC_ADD(&s->be->be_counters.failed_rewrites, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->failed_rewrites, 1); } free_trash_chunk(replace); @@ -3341,7 +3341,7 @@ resume_execution: _HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_rewrites, 1); if (sess->fe != s->be) _HA_ATOMIC_ADD(&s->be->be_counters.failed_rewrites, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->failed_rewrites, 1); if (objt_server(s->target)) _HA_ATOMIC_ADD(&objt_server(s->target)->counters.failed_rewrites, 1); diff --git a/src/proto_tcp.c b/src/proto_tcp.c index 0b7ba6d..33139b5 100644 --- a/src/proto_tcp.c +++ b/src/proto_tcp.c @@ -1367,7 +1367,7 @@ static enum act_return tcp_exec_action_silent_drop(struct act_rule *rule, struct } _HA_ATOMIC_ADD(&sess->fe->fe_counters.denied_req, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->denied_req, 1); return ACT_RET_STOP; -- 1.7.10.4