From 656730d92f1c3afaed0baf67911d4ee055528e2e 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 --- src/fcgi-app.c | 2 +- src/http_act.c | 10 +++++----- src/http_ana.c | 46 +++++++++++++++++++++++----------------------- src/tcp_act.c | 2 +- src/tcp_rules.c | 4 ++-- 5 files changed, 32 insertions(+), 32 deletions(-) diff --git a/src/fcgi-app.c b/src/fcgi-app.c index 412584c..4174a07 100644 --- a/src/fcgi-app.c +++ b/src/fcgi-app.c @@ -477,7 +477,7 @@ static int fcgi_flt_http_headers(struct stream *s, struct filter *filter, struct rewrite_err: _HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_rewrites, 1); _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/http_act.c b/src/http_act.c index ed4fce1..ed403a7 100644 --- a/src/http_act.c +++ b/src/http_act.c @@ -124,7 +124,7 @@ static enum act_return http_action_set_req_line(struct act_rule *rule, struct pr _HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_rewrites, 1); if (s->flags & SF_BE_ASSIGNED) _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); @@ -253,7 +253,7 @@ static enum act_return http_action_replace_uri(struct act_rule *rule, struct pro _HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_rewrites, 1); if (s->flags & SF_BE_ASSIGNED) _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); @@ -329,7 +329,7 @@ static enum act_return action_http_set_status(struct act_rule *rule, struct prox _HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_rewrites, 1); if (s->flags & SF_BE_ASSIGNED) _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); @@ -1256,7 +1256,7 @@ static enum act_return http_action_set_header(struct act_rule *rule, struct prox _HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_rewrites, 1); if (s->flags & SF_BE_ASSIGNED) _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); @@ -1369,7 +1369,7 @@ static enum act_return http_action_replace_header(struct act_rule *rule, struct _HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_rewrites, 1); if (s->flags & SF_BE_ASSIGNED) _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/http_ana.c b/src/http_ana.c index f9dbfb1..a1dd807 100644 --- a/src/http_ana.c +++ b/src/http_ana.c @@ -437,14 +437,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.internal_errors, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->internal_errors, 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 */ @@ -655,7 +655,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 (s->flags & SF_BE_ASSIGNED) _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; @@ -671,7 +671,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 (s->flags & SF_BE_ASSIGNED) _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_err; @@ -682,14 +682,14 @@ int http_process_req_common(struct stream *s, struct channel *req, int an_bit, s _HA_ATOMIC_ADD(&sess->fe->fe_counters.internal_errors, 1); if (s->flags & SF_BE_ASSIGNED) _HA_ATOMIC_ADD(&s->be->be_counters.internal_errors, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->internal_errors, 1); goto return_prx_err; 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 */ @@ -923,7 +923,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); @@ -946,14 +946,14 @@ int http_process_request(struct stream *s, struct channel *req, int an_bit) _HA_ATOMIC_ADD(&sess->fe->fe_counters.internal_errors, 1); if (s->flags & SF_BE_ASSIGNED) _HA_ATOMIC_ADD(&s->be->be_counters.internal_errors, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->internal_errors, 1); goto return_prx_cond; return_bad_req: /* let's centralize all bad requests */ 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 */ @@ -1076,7 +1076,7 @@ int http_wait_for_request_body(struct stream *s, struct channel *req, int an_bit if (!(s->flags & SF_ERR_MASK)) s->flags |= SF_ERR_CLITO; _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; } @@ -1112,14 +1112,14 @@ int http_wait_for_request_body(struct stream *s, struct channel *req, int an_bit _HA_ATOMIC_ADD(&sess->fe->fe_counters.internal_errors, 1); if (s->flags & SF_BE_ASSIGNED) _HA_ATOMIC_ADD(&s->be->be_counters.internal_errors, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->internal_errors, 1); goto return_prx_cond; return_bad_req: /* let's centralize all bad requests */ 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 */ @@ -1351,7 +1351,7 @@ int http_request_forward_body(struct stream *s, struct channel *req, int an_bit) return_cli_abort: _HA_ATOMIC_ADD(&sess->fe->fe_counters.cli_aborts, 1); _HA_ATOMIC_ADD(&s->be->be_counters.cli_aborts, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->cli_aborts, 1); if (objt_server(s->target)) _HA_ATOMIC_ADD(&__objt_server(s->target)->counters.cli_aborts, 1); @@ -1363,7 +1363,7 @@ int http_request_forward_body(struct stream *s, struct channel *req, int an_bit) return_srv_abort: _HA_ATOMIC_ADD(&sess->fe->fe_counters.srv_aborts, 1); _HA_ATOMIC_ADD(&s->be->be_counters.srv_aborts, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->srv_aborts, 1); if (objt_server(s->target)) _HA_ATOMIC_ADD(&__objt_server(s->target)->counters.srv_aborts, 1); @@ -1377,7 +1377,7 @@ int http_request_forward_body(struct stream *s, struct channel *req, int an_bit) s->flags |= SF_ERR_INTERNAL; _HA_ATOMIC_ADD(&sess->fe->fe_counters.internal_errors, 1); _HA_ATOMIC_ADD(&s->be->be_counters.internal_errors, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->internal_errors, 1); if (objt_server(s->target)) _HA_ATOMIC_ADD(&__objt_server(s->target)->counters.internal_errors, 1); @@ -1386,7 +1386,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); status = 400; /* fall through */ @@ -1594,7 +1594,7 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit) else if ((rep->flags & CF_SHUTR) && ((s->req.flags & (CF_SHUTR|CF_SHUTW)) == (CF_SHUTR|CF_SHUTW))) { _HA_ATOMIC_ADD(&sess->fe->fe_counters.cli_aborts, 1); _HA_ATOMIC_ADD(&s->be->be_counters.cli_aborts, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->cli_aborts, 1); if (objt_server(s->target)) _HA_ATOMIC_ADD(&__objt_server(s->target)->counters.cli_aborts, 1); @@ -1867,7 +1867,7 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit) return_int_err: _HA_ATOMIC_ADD(&sess->fe->fe_counters.internal_errors, 1); _HA_ATOMIC_ADD(&s->be->be_counters.internal_errors, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->internal_errors, 1); if (objt_server(s->target)) _HA_ATOMIC_ADD(&__objt_server(s->target)->counters.internal_errors, 1); @@ -2167,7 +2167,7 @@ int http_process_res_common(struct stream *s, struct channel *rep, int an_bit, s deny: _HA_ATOMIC_ADD(&sess->fe->fe_counters.denied_resp, 1); _HA_ATOMIC_ADD(&s->be->be_counters.denied_resp, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->denied_resp, 1); if (objt_server(s->target)) _HA_ATOMIC_ADD(&__objt_server(s->target)->counters.denied_resp, 1); @@ -2433,7 +2433,7 @@ int http_response_forward_body(struct stream *s, struct channel *res, int an_bit return_srv_abort: _HA_ATOMIC_ADD(&sess->fe->fe_counters.srv_aborts, 1); _HA_ATOMIC_ADD(&s->be->be_counters.srv_aborts, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->srv_aborts, 1); if (objt_server(s->target)) _HA_ATOMIC_ADD(&__objt_server(s->target)->counters.srv_aborts, 1); @@ -2444,7 +2444,7 @@ int http_response_forward_body(struct stream *s, struct channel *res, int an_bit return_cli_abort: _HA_ATOMIC_ADD(&sess->fe->fe_counters.cli_aborts, 1); _HA_ATOMIC_ADD(&s->be->be_counters.cli_aborts, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->cli_aborts, 1); if (objt_server(s->target)) _HA_ATOMIC_ADD(&__objt_server(s->target)->counters.cli_aborts, 1); @@ -2455,7 +2455,7 @@ int http_response_forward_body(struct stream *s, struct channel *res, int an_bit return_int_err: _HA_ATOMIC_ADD(&sess->fe->fe_counters.internal_errors, 1); _HA_ATOMIC_ADD(&s->be->be_counters.internal_errors, 1); - if (sess->listener->counters) + if (sess->listener && sess->listener->counters) _HA_ATOMIC_ADD(&sess->listener->counters->internal_errors, 1); if (objt_server(s->target)) _HA_ATOMIC_ADD(&__objt_server(s->target)->counters.internal_errors, 1); @@ -4486,7 +4486,7 @@ static void http_end_response(struct stream *s) txn->rsp.msg_state = HTTP_MSG_ERROR; _HA_ATOMIC_ADD(&strm_sess(s)->fe->fe_counters.cli_aborts, 1); _HA_ATOMIC_ADD(&s->be->be_counters.cli_aborts, 1); - if (strm_sess(s)->listener->counters) + if (strm_sess(s)->listener && strm_sess(s)->listener->counters) _HA_ATOMIC_ADD(&strm_sess(s)->listener->counters->cli_aborts, 1); if (objt_server(s->target)) _HA_ATOMIC_ADD(&__objt_server(s->target)->counters.cli_aborts, 1); diff --git a/src/tcp_act.c b/src/tcp_act.c index 601bb53..a92c9bb 100644 --- a/src/tcp_act.c +++ b/src/tcp_act.c @@ -225,7 +225,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_ABRT; diff --git a/src/tcp_rules.c b/src/tcp_rules.c index d5fce26..d83f10e 100644 --- a/src/tcp_rules.c +++ b/src/tcp_rules.c @@ -373,7 +373,7 @@ resume_execution: deny: _HA_ATOMIC_ADD(&s->sess->fe->fe_counters.denied_resp, 1); _HA_ATOMIC_ADD(&s->be->be_counters.denied_resp, 1); - if (s->sess->listener->counters) + if (s->sess->listener && s->sess->listener->counters) _HA_ATOMIC_ADD(&s->sess->listener->counters->denied_resp, 1); if (objt_server(s->target)) _HA_ATOMIC_ADD(&__objt_server(s->target)->counters.denied_resp, 1); @@ -382,7 +382,7 @@ resume_execution: internal: _HA_ATOMIC_ADD(&s->sess->fe->fe_counters.internal_errors, 1); _HA_ATOMIC_ADD(&s->be->be_counters.internal_errors, 1); - if (s->sess->listener->counters) + if (s->sess->listener && s->sess->listener->counters) _HA_ATOMIC_ADD(&s->sess->listener->counters->internal_errors, 1); if (objt_server(s->target)) _HA_ATOMIC_ADD(&__objt_server(s->target)->counters.internal_errors, 1); -- 1.7.10.4