diff options
author | Przemyslaw Czarnowski <przemyslaw.hawrylewicz.czarnowski@intel.com> | 2021-10-14 22:52:30 +0300 |
---|---|---|
committer | Czarnowski, Przemyslaw <przemyslaw.hawrylewicz.czarnowski@intel.com> | 2021-11-16 17:56:21 +0300 |
commit | eb57aae1f56acff2784887e4f24c74c4141657a6 (patch) | |
tree | db876b86ed32f1f3a14f88b5b415ad309f223ea9 | |
parent | 40916ed7ba8283509fd3c78612558391806c37f9 (diff) | |
download | virtual-media-eb57aae1f56acff2784887e4f24c74c4141657a6.tar.xz |
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>
-rw-r--r-- | src/utils.hpp | 55 |
1 files 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 |