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.
diff --git a/src/fcgi-app.c b/src/fcgi-app.c
index 4a7e0b3..1a25406 100644
--- a/src/fcgi-app.c
+++ b/src/fcgi-app.c
@@ -477,7 +477,7 @@
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 73e43ff..b4aced1 100644
--- a/src/http_act.c
+++ b/src/http_act.c
@@ -124,7 +124,7 @@
_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 @@
_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 @@
_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 @@
_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 @@
_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 c124544..c53f0a6 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -295,14 +295,14 @@
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 */
@@ -508,7 +508,7 @@
_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;
@@ -524,7 +524,7 @@
_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;
@@ -535,14 +535,14 @@
_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 */
@@ -780,7 +780,7 @@
* in case we previously disabled it, otherwise we might cause
* the client to delay further data.
*/
- if ((sess->listener->options & LI_O_NOQUICKACK) && !(htx->flags & HTX_FL_EOM))
+ if ((sess->listener && (sess->listener->options & LI_O_NOQUICKACK)) && !(htx->flags & HTX_FL_EOM))
conn_set_quickack(cli_conn, 1);
/*************************************************************
@@ -802,14 +802,14 @@
_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 */
@@ -928,7 +928,7 @@
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;
}
@@ -964,14 +964,14 @@
_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 */
@@ -1227,7 +1227,7 @@
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);
@@ -1239,7 +1239,7 @@
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);
@@ -1253,7 +1253,7 @@
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);
@@ -1262,7 +1262,7 @@
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 */
@@ -1480,7 +1480,7 @@
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);
@@ -1780,7 +1780,7 @@
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);
@@ -2081,7 +2081,7 @@
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);
@@ -2348,7 +2348,7 @@
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);
@@ -2360,7 +2360,7 @@
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);
@@ -2371,7 +2371,7 @@
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);
@@ -4405,7 +4405,7 @@
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/mux_h1.c b/src/mux_h1.c
index 53cbb44..1f02daa 100644
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -2411,7 +2411,7 @@
proxy_inc_fe_req_ctr(sess->listener, sess->fe);
_HA_ATOMIC_ADD(&sess->fe->fe_counters.p.http.rsp[5], 1);
_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);
h1c->errcode = 500;
@@ -2436,7 +2436,7 @@
proxy_inc_fe_req_ctr(sess->listener, sess->fe);
_HA_ATOMIC_ADD(&sess->fe->fe_counters.p.http.rsp[4], 1);
_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);
h1c->errcode = 400;
@@ -2463,7 +2463,7 @@
proxy_inc_fe_req_ctr(sess->listener, sess->fe);
_HA_ATOMIC_ADD(&sess->fe->fe_counters.p.http.rsp[4], 1);
_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);
h1c->errcode = 501;
@@ -2489,7 +2489,7 @@
proxy_inc_fe_req_ctr(sess->listener, sess->fe);
_HA_ATOMIC_ADD(&sess->fe->fe_counters.p.http.rsp[4], 1);
_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);
h1c->errcode = 408;
diff --git a/src/tcp_act.c b/src/tcp_act.c
index 38da91f..b3993f7 100644
--- a/src/tcp_act.c
+++ b/src/tcp_act.c
@@ -225,7 +225,7 @@
}
_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 da4ce30..24858cc 100644
--- a/src/tcp_rules.c
+++ b/src/tcp_rules.c
@@ -373,7 +373,7 @@
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 @@
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);