From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Fri, 30 Aug 2019 08:36:59 +0000 (+0200)
Subject: rec: fix two coverity issues
X-Git-Tag: rec-4.3.0-alpha1^2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a73da04b6350a0b15ee974990d34fc783d5446dd;p=pdns

rec: fix two coverity issues

1. Uninitialized field in DNSComboWriter.
2. TOCTOU wrt stat() and then opendir(). opendir() already tells us if the path
is not a dir or not accesible, so no need to do a TOCTOU sensitive pre-check.
Use uniform error messages in log and exceptions while here.
---

diff --git a/pdns/arguments.cc b/pdns/arguments.cc
index 1f2d04456..b41bdbcdd 100644
--- a/pdns/arguments.cc
+++ b/pdns/arguments.cc
@@ -460,43 +460,35 @@ bool ArgvMap::file(const char *fname, bool lax, bool included)
 
 void ArgvMap::gatherIncludes(std::vector<std::string> &extraConfigs) {
   extraConfigs.clear();
-  if (params["include-dir"].empty()) return; // nothing to do
-    struct stat st;
-    DIR *dir;
-    struct dirent *ent;
-
-    // stat
-    if (stat(params["include-dir"].c_str(), &st)) {
-       g_log << Logger::Error << params["include-dir"] << " does not exist!" << std::endl;
-       throw ArgException(params["include-dir"] + " does not exist!");
-    }
-
-    // wonder if it's accessible directory
-    if (!S_ISDIR(st.st_mode)) {
-       g_log << Logger::Error << params["include-dir"] << " is not a directory" << std::endl;
-       throw ArgException(params["include-dir"] + " is not a directory");
-    }
-
-    if (!(dir = opendir(params["include-dir"].c_str()))) {
-       g_log << Logger::Error << params["include-dir"] << " is not accessible" << std::endl;
-       throw ArgException(params["include-dir"] + " is not accessible");
-    }
+  if (params["include-dir"].empty())
+    return; // nothing to do
+
+  DIR *dir;
+  if (!(dir = opendir(params["include-dir"].c_str()))) {
+    int err = errno;
+    string msg = params["include-dir"] + " is not accessible: " + strerror(err);
+    g_log << Logger::Error << msg << std::endl;
+    throw ArgException(msg);
+  }
 
-    while((ent = readdir(dir)) != NULL) {
-      if (ent->d_name[0] == '.') continue; // skip any dots
-      if (boost::ends_with(ent->d_name, ".conf")) {
-        // build name
-        std::ostringstream namebuf;
-        namebuf << params["include-dir"].c_str() << "/" << ent->d_name; // FIXME: Use some path separator
-        // ensure it's readable file
-        if (stat(namebuf.str().c_str(), &st) || !S_ISREG(st.st_mode)) {
-          g_log << Logger::Error << namebuf.str() << " is not a file" << std::endl;
-          closedir(dir);
-          throw ArgException(namebuf.str() + " does not exist!");
-        }
-        extraConfigs.push_back(namebuf.str());
+  struct dirent *ent;
+  while ((ent = readdir(dir)) != NULL) {
+    if (ent->d_name[0] == '.')
+      continue; // skip any dots
+    if (boost::ends_with(ent->d_name, ".conf")) {
+      // build name
+      string name = params["include-dir"] + "/" + ent->d_name; // FIXME: Use some path separator
+      // ensure it's readable file
+      struct stat st;
+      if (stat(name.c_str(), &st) || !S_ISREG(st.st_mode)) {
+        string msg = name + " is not a regular file";
+        g_log << Logger::Error << msg << std::endl;
+        closedir(dir);
+        throw ArgException(msg);
       }
+      extraConfigs.push_back(name);
     }
-    std::sort(extraConfigs.begin(), extraConfigs.end(), CIStringComparePOSIX()); 
-    closedir(dir);
+  }
+  std::sort(extraConfigs.begin(), extraConfigs.end(), CIStringComparePOSIX());
+  closedir(dir);
 }
diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index d4db9e958..0b7a8f098 100644
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -342,7 +342,7 @@ struct DNSComboWriter {
   bool d_ecsParsed{false};
   bool d_followCNAMERecords{false};
   bool d_logResponse{false};
-  bool d_tcp;
+  bool d_tcp{false};
 };
 
 MT_t* getMT()