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 4a7e0b358fda9b93cf702f3b59fb4de8e690b319..1a25406378a05eea1f8c8b8da0c66378f519ec2e 100644 (file)
--- 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 73e43ff6343325f4c47d103772c50dcdecc67446..b4aced17c1be7dce096b2fd6719323edfdb93a03 100644 (file)
--- 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 c1245442f14d5ee9b6ca5f0cafd06591af1e9f1d..c53f0a617f9b7df78141f09b30a9faa039232ca5 100644 (file)
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -295,14 +295,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 */
 
@@ -508,7 +508,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;
 
@@ -524,7 +524,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;
 
@@ -535,14 +535,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 */
 
@@ -780,7 +780,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) && !(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 @@ 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 */
 
@@ -928,7 +928,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;
        }
@@ -964,14 +964,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 */
 
@@ -1227,7 +1227,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);
@@ -1239,7 +1239,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);
@@ -1253,7 +1253,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);
@@ -1262,7 +1262,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 */
@@ -1480,7 +1480,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);
@@ -1780,7 +1780,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);
@@ -2081,7 +2081,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);
@@ -2348,7 +2348,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);
@@ -2360,7 +2360,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);
@@ -2371,7 +2371,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);
@@ -4405,7 +4405,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/mux_h1.c b/src/mux_h1.c
index 53cbb44febd33fe575e69c3c23ccb44e246d7cf5..1f02daaa97787cfb1080989aae1c5c6477d7ff1d 100644 (file)
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -2411,7 +2411,7 @@ static int h1_handle_internal_err(struct h1c *h1c)
        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 @@ static int h1_handle_bad_req(struct h1c *h1c)
        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 @@ static int h1_handle_not_impl_err(struct h1c *h1c)
        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 @@ static int h1_handle_req_tout(struct h1c *h1c)
        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 38da91f89db516f34691d0941268f2b62ccc9e42..b3993f7de5c41839ebd4e1604aff74b411dc9dae 100644 (file)
--- 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 da4ce302a0978424036040a6720ecf388891c0c9..24858ccfb38ba248f7b2803f08e44ad4f69a916e 100644 (file)
--- 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);