From c2f7f2964c9394643b69076ab8f0df490b000f45 Mon Sep 17 00:00:00 2001
From: Andrew Clayton <a.clayton@nginx.com>
Date: Fri, 23 Feb 2024 01:05:04 +0000
Subject: Avoid potential NULL pointer dereference in nxt_router_temp_conf()

In nxt_router_temp_conf() we have

  rtcf = nxt_mp_zget(mp, sizeof(nxt_router_conf_t));
  if (nxt_slow_path(rtcf == NULL)) {
      goto fail;
  }

If rtcf is NULL then we do

  fail:

  if (rtcf->tstr_state != NULL) {
      nxt_tstr_state_release(rtcf->tstr_state);
  }

In which case we will dereference the NULL pointer rtcf.

This patch re-works the goto labels to make them more specific to their
intended purpose and ensures we are freeing things which have been
allocated.

This was found by the clang static analyser.

Reviewed-by: Zhidao Hong <z.hong@f5.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
---
 src/nxt_router.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/nxt_router.c b/src/nxt_router.c
index 1a1aca2b..e395929e 100644
--- a/src/nxt_router.c
+++ b/src/nxt_router.c
@@ -1077,14 +1077,14 @@ nxt_router_temp_conf(nxt_task_t *task)
 
     rtcf = nxt_mp_zget(mp, sizeof(nxt_router_conf_t));
     if (nxt_slow_path(rtcf == NULL)) {
-        goto fail;
+        goto out_free_mp;
     }
 
     rtcf->mem_pool = mp;
 
     rtcf->tstr_state = nxt_tstr_state_new(mp, 0);
     if (nxt_slow_path(rtcf->tstr_state == NULL)) {
-        goto fail;
+        goto out_free_mp;
     }
 
 #if (NXT_HAVE_NJS)
@@ -1093,12 +1093,12 @@ nxt_router_temp_conf(nxt_task_t *task)
 
     tmp = nxt_mp_create(1024, 128, 256, 32);
     if (nxt_slow_path(tmp == NULL)) {
-        goto fail;
+        goto out_free_tstr_state;
     }
 
     tmcf = nxt_mp_zget(tmp, sizeof(nxt_router_temp_conf_t));
     if (nxt_slow_path(tmcf == NULL)) {
-        goto temp_fail;
+        goto out_free;
     }
 
     tmcf->mem_pool = tmp;
@@ -1109,7 +1109,7 @@ nxt_router_temp_conf(nxt_task_t *task)
     tmcf->engines = nxt_array_create(tmcf->mem_pool, 4,
                                      sizeof(nxt_router_engine_conf_t));
     if (nxt_slow_path(tmcf->engines == NULL)) {
-        goto temp_fail;
+        goto out_free;
     }
 
     nxt_queue_init(&creating_sockets);
@@ -1131,16 +1131,18 @@ nxt_router_temp_conf(nxt_task_t *task)
 
     return tmcf;
 
-temp_fail:
+out_free:
 
     nxt_mp_destroy(tmp);
 
-fail:
+out_free_tstr_state:
 
     if (rtcf->tstr_state != NULL) {
         nxt_tstr_state_release(rtcf->tstr_state);
     }
 
+out_free_mp:
+
     nxt_mp_destroy(mp);
 
     return NULL;
-- 
cgit