From: Heiko Hund <heiko@ist.eigentlich.net>
Date: Tue, 20 May 2025 07:33:48 +0000 (+0200)
Subject: dns: fix potential NULL pointer dereference
X-Git-Tag: v2.7_alpha1~5
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d1045d19bf4ea1f717edac7c67f20e9d8eb3a261;p=thirdparty%2Fopenvpn.git

dns: fix potential NULL pointer dereference

Fix issue reported by Coverity (CID 1646952): Dereferencing a pointer
that might be NULL dvf when calling env_set_write_file.

In addition to the fix, inline the write_dns_vars_file() helper function.
Also output a log line in case this error happens, because when it
happens it will hinder communication with the updown runner process, i.e.
setting up / tearing down DNS things will not work as expected.

Change-Id: I275bf939f43577427e14890e7093d63c5213ae5d
Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20250520073354.17091-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg31720.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
---

diff --git a/src/openvpn/dns.c b/src/openvpn/dns.c
index 221e9a9b..283ce670 100644
--- a/src/openvpn/dns.c
+++ b/src/openvpn/dns.c
@@ -688,18 +688,6 @@ run_updown_runner(bool up, struct options *o, const struct tuntap *tt, struct dn
     return true;
 }
 
-static const char *
-write_dns_vars_file(bool up, const struct options *o, const struct tuntap *tt, struct gc_arena *gc)
-{
-    struct env_set *es = env_set_create(gc);
-    const char *dvf = platform_create_temp_file(o->tmp_dir, "dvf", gc);
-
-    updown_env_set(up, &o->dns_options, tt, es);
-    env_set_write_file(dvf, es);
-
-    return dvf;
-}
-
 static void
 run_up_down_command(bool up, struct options *o, const struct tuntap *tt, struct dns_updown_runner_info *updown_runner)
 {
@@ -708,7 +696,7 @@ run_up_down_command(bool up, struct options *o, const struct tuntap *tt, struct
         return;
     }
 
-    int status;
+    int status = -1;
 
     if (!updown_runner->required)
     {
@@ -727,11 +715,19 @@ run_up_down_command(bool up, struct options *o, const struct tuntap *tt, struct
         }
 
         struct gc_arena gc = gc_new();
-        int rfd = updown_runner->fds[0];
+        const char *dvf = platform_create_temp_file(o->tmp_dir, "dvf", &gc);
+        if (!dvf)
+        {
+            msg(M_ERR, "could not create dns vars file");
+            goto out_free;
+        }
+
+        struct env_set *es = env_set_create(&gc);
+        updown_env_set(up, &o->dns_options, tt, es);
+        env_set_write_file(dvf, es);
+
         int wfd = updown_runner->fds[1];
-        const char *dvf = write_dns_vars_file(up, o, tt, &gc);
         size_t dvf_size = strlen(dvf) + 1;
-
         while (1)
         {
             ssize_t len = write(wfd, dvf, dvf_size);
@@ -746,6 +742,7 @@ run_up_down_command(bool up, struct options *o, const struct tuntap *tt, struct
             break;
         }
 
+        int rfd = updown_runner->fds[0];
         while (1)
         {
             ssize_t len = read(rfd, &status, sizeof(status));
@@ -760,6 +757,7 @@ run_up_down_command(bool up, struct options *o, const struct tuntap *tt, struct
             break;
         }
 
+out_free:
         gc_free(&gc);
     }