]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
file: use functions on fd to avoid toctou
authorPhilippe Antoine <contact@catenacyber.fr>
Sat, 16 Apr 2022 14:46:01 +0000 (16:46 +0200)
committerVictor Julien <vjulien@oisf.net>
Wed, 1 Jun 2022 08:54:17 +0000 (10:54 +0200)
Ticket: #5308

src/suricata.c
src/util-debug.c
src/util-logopenfile.c
src/util-pidfile.c

index f4f3b0c52d0045b558d59dc67813b571417a28a5..9dcf8b3aa34d6531b1de6bdb631dc7a3969fa4f0 100644 (file)
@@ -530,23 +530,22 @@ static void SetBpfStringFromFile(char *filename)
                               " Use firewall filtering if possible.");
     }
 
+    fp = fopen(filename, "r");
+    if (fp == NULL) {
+        SCLogError(SC_ERR_FOPEN, "Failed to open file %s", filename);
+        exit(EXIT_FAILURE);
+    }
+
 #ifdef OS_WIN32
-    if(_stat(filename, &st) != 0) {
+    if (_fstat(_fileno(fp), &st) != 0) {
 #else
-    if(stat(filename, &st) != 0) {
+    if (fstat(fileno(fp), &st) != 0) {
 #endif /* OS_WIN32 */
         SCLogError(SC_ERR_FOPEN, "Failed to stat file %s", filename);
         exit(EXIT_FAILURE);
     }
     bpf_len = st.st_size + 1;
 
-    // coverity[toctou : FALSE]
-    fp = fopen(filename,"r");
-    if (fp == NULL) {
-        SCLogError(SC_ERR_FOPEN, "Failed to open file %s", filename);
-        exit(EXIT_FAILURE);
-    }
-
     bpf_filter = SCMalloc(bpf_len * sizeof(char));
     if (unlikely(bpf_filter == NULL)) {
         SCLogError(SC_ERR_MEM_ALLOC, "Failed to allocate buffer for bpf filter in file %s", filename);
index b96d75feef9880222e7f4a4bf5ac91a7826f31a1..3c8a95f35f55154eee07463698a6e515357a7081 100644 (file)
@@ -742,7 +742,7 @@ static inline SCLogOPIfaceCtx *SCLogInitFileOPIface(const char *file, uint32_t u
 
 #ifndef OS_WIN32
     if (userid != 0 || groupid != 0) {
-        if (chown(file, userid, groupid) == -1) {
+        if (fchown(fileno(iface_ctx->file_d), userid, groupid) == -1) {
             SCLogWarning(SC_WARN_CHOWN, "Failed to change ownership of file %s: %s", file,
                     strerror(errno));
         }
index 1150166d93d488519c438e93765eba23aa3c2bdd..8e0f89dd16ac012a007e08b1e6fdea6f6fc3d425 100644 (file)
@@ -401,7 +401,11 @@ SCLogOpenFileFp(const char *path, const char *append_setting, uint32_t mode)
                    filename, strerror(errno));
     } else {
         if (mode != 0) {
-            int r = chmod(filename, (mode_t)mode);
+#ifdef OS_WIN32
+            int r = _chmod(filename, (mode_t)mode);
+#else
+            int r = fchmod(fileno(ret), (mode_t)mode);
+#endif
             if (r < 0) {
                 SCLogWarning(SC_WARN_CHMOD, "Could not chmod %s to %o: %s",
                              filename, mode, strerror(errno));
index acb92b230ef1075dfdc7b95ec93eb5640c149afe..cbc57d6cb40d3fa42129b7021e849689699d8d35 100644 (file)
@@ -104,37 +104,37 @@ void SCPidfileRemove(const char *pid_filename)
  */
 int SCPidfileTestRunning(const char *pid_filename)
 {
-    if (access(pid_filename, F_OK) == 0) {
-        /* Check if the existing process is still alive. */
-        FILE *pf;
+    /* Check if the existing process is still alive. */
+    FILE *pf;
 
-        // coverity[toctou : FALSE]
-        pf = fopen(pid_filename, "r");
-        if (pf == NULL) {
-            SCLogError(SC_ERR_INITIALIZATION,
-                    "pid file '%s' exists and can not be read. Aborting!",
+    pf = fopen(pid_filename, "r");
+    if (pf == NULL) {
+        if (access(pid_filename, F_OK) == 0) {
+            SCLogError(SC_ERR_INITIALIZATION, "pid file '%s' exists and can not be read. Aborting!",
                     pid_filename);
             return -1;
+        } else {
+            return 0;
         }
+    }
 
 #ifndef OS_WIN32
-        pid_t pidv;
-        if (fscanf(pf, "%d", &pidv) == 1 && kill(pidv, 0) == 0) {
-            SCLogError(SC_ERR_INITIALIZATION,
-                    "pid file '%s' exists and Suricata appears to be running. "
-                    "Aborting!", pid_filename);
-        } else
+    pid_t pidv;
+    if (fscanf(pf, "%d", &pidv) == 1 && kill(pidv, 0) == 0) {
+        SCLogError(SC_ERR_INITIALIZATION,
+                "pid file '%s' exists and Suricata appears to be running. "
+                "Aborting!",
+                pid_filename);
+    } else
 #endif
-        {
-            SCLogError(SC_ERR_INITIALIZATION,
-                    "pid file '%s' exists but appears stale. "
-                    "Make sure Suricata is not running and then remove %s. "
-                    "Aborting!",
-                    pid_filename, pid_filename);
-        }
-
-        fclose(pf);
-        return -1;
+    {
+        SCLogError(SC_ERR_INITIALIZATION,
+                "pid file '%s' exists but appears stale. "
+                "Make sure Suricata is not running and then remove %s. "
+                "Aborting!",
+                pid_filename, pid_filename);
     }
-    return 0;
+
+    fclose(pf);
+    return -1;
 }