From eb57aae1f56acff2784887e4f24c74c4141657a6 Mon Sep 17 00:00:00 2001
From: Przemyslaw Czarnowski <przemyslaw.hawrylewicz.czarnowski@intel.com>
Date: Thu, 14 Oct 2021 21:52:30 +0200
Subject: virtual-media: Replace tmpnam with mkstemp

In the mechanism of passing the password to client application a file
with password is used. Until now, the file has been created with use of
unsecure tmpnam function, which can be a subject of TOCTOU vulnerability.

Changing tmpnam to mkstemp required some changes in the
flow (std::fstream can't reuse file descriptor of opened file,
appropriate file permissions are set by mkstemp).

Tested:
Manually. Password is passed to nbdkit, temporary file permissions are the same as
before.

Change-Id: Icdd1719cafa08946d5b06414a0db7fa4714cb7ee
Signed-off-by: Czarnowski, Przemyslaw <przemyslaw.hawrylewicz.czarnowski@intel.com>
---
 src/utils.hpp | 55 +++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 39 insertions(+), 16 deletions(-)

diff --git a/src/utils.hpp b/src/utils.hpp
index fd2e320..8eaebfa 100644
--- a/src/utils.hpp
+++ b/src/utils.hpp
@@ -1,5 +1,7 @@
 #pragma once
 
+#include <unistd.h>
+
 #include <algorithm>
 #include <boost/process/async_pipe.hpp>
 #include <boost/type_traits/has_dereference.hpp>
@@ -163,7 +165,7 @@ class NamedPipe
         return unix_fd{impl.native_sink()};
     }
 
-    const std::string& file()
+    const std::string& file() const
     {
         return name;
     }
@@ -202,12 +204,12 @@ class VolatileFile
     using Buffer = CredentialsProvider::SecureBuffer;
 
   public:
-    VolatileFile(Buffer&& contents) :
-        filePath(fs::temp_directory_path() / std::tmpnam(nullptr)),
-        size(contents->size())
+    VolatileFile(Buffer&& contents) : size(contents->size())
     {
         auto data = std::move(contents);
-        create(filePath, data);
+        filePath = fs::temp_directory_path().c_str();
+        filePath.append("/VM-XXXXXX");
+        create(data);
     }
 
     ~VolatileFile()
@@ -221,19 +223,40 @@ class VolatileFile
         return filePath;
     }
 
-  private:
-    static void create(const std::string& filePath, const Buffer& data)
+    class FileObject
     {
-        std::ofstream file(filePath);
-        limitPermissionsToOwnerOnly(filePath);
-        file.write(data->data(), data->size());
-    }
+      public:
+        explicit FileObject(int fd) : fd(fd){};
+        FileObject() = delete;
+        FileObject(const FileObject&) = delete;
+        FileObject& operator=(const FileObject&) = delete;
+
+        ssize_t write(void* data, ssize_t nw)
+        {
+            return ::write(fd, data, nw);
+        }
 
-    static void limitPermissionsToOwnerOnly(const std::string& filePath)
+        ~FileObject()
+        {
+            close(fd);
+        }
+
+      private:
+        int fd;
+    };
+
+  private:
+    void create(const Buffer& data)
     {
-        fs::permissions(filePath,
-                        fs::perms::owner_read | fs::perms::owner_write,
-                        fs::perm_options::replace);
+        auto fd = mkstemp(filePath.data());
+
+        FileObject file(fd);
+        if (file.write(data->data(), data->size()) !=
+            static_cast<ssize_t>(data->size()))
+        {
+            throw sdbusplus::exception::SdBusError(
+                EIO, "I/O error on temporary file");
+        }
     }
 
     void purgeFileContents()
@@ -254,7 +277,7 @@ class VolatileFile
         }
     }
 
-    const std::string filePath;
+    std::string filePath;
     const std::size_t size;
 };
 } // namespace utils
-- 
cgit v1.2.3