4

I'm trying to create a unique temporary directory in the system temp folder and have been reading about the security and file creation issues of tmpnam().

I've written the below code and was wondering whether it would satisfy these issues, is my use of the tmpnam() function correct and the throwing of the filesystem_error? Should I be adding checks for other things (e.g. temp_directory_path, which also throws an exception)?

    // Unique temporary directory path in the system temporary directory path.
std::filesystem::path tmp_dir_path {std::filesystem::temp_directory_path() /= std::tmpnam(nullptr)};

// Attempt to create the directory.
if (std::filesystem::create_directories(tmp_dir_path)) {

    // Directory successfully created.
    return tmp_dir_path;

} else {

    // Directory could not be created.
    throw std::filesystem_error("directory could not be created.");

}
user1406186
  • 940
  • 3
  • 16
  • 28

2 Answers2

5

From cppreference.com:

Although the names generated by std::tmpnam are difficult to guess, it is possible that a file with that name is created by another process between the moment std::tmpnam returns and the moment this program attempts to use the returned name to create a file.

The problem is not with how you use it, but the fact that you do.

For example, in your code sample, if a malicious user successfully guesses and creates the directory right in between the first and second line, it might deny service (DOS) from your application, which might be critical, or not.

Instead, there is a way to do this without races on POSIX-compliant systems:

Baruch
  • 20,590
  • 28
  • 126
  • 201
Daniel Trugman
  • 8,186
  • 20
  • 41
1

Your code is fine. Because you try to create the directory the OS will arbitrate between your process and another process trying to create the same file so, if you win, you own the file and if you lose you get an error.

I wrote a similar function recently. Whether you throw an exception or not depends on how you want to use this function. You could, for example simply return either an open or closed std::fstream and use std::fstream::is_open as a measure of success or return an empty pathname on failure.

Looking up std::filesystem::create_directories it will throw its own exception if you don't supply a std::error_code parameter so you don't need to throw your own exception:

std::filesystem::path tmp_dir_path {std::filesystem::temp_directory_path() /= std::tmpnam(nullptr)};

// Attempt to create the directory.
std::filesystem::create_directories(tmp_dir_path));

// If that failed an exception will have been thrown
// so no need to check or throw your own

// Directory successfully created.
return tmp_dir_path;
Galik
  • 47,303
  • 4
  • 80
  • 117
  • Thanks for your comments, I've tried compiling the code and when I throw the filesystem_error, i'm meant to include an error_code argument. I'm not entirely sure what I should be putting as the error_code argument for this exception. – user1406186 Oct 21 '18 at 14:18
  • 1. Why does filesystem_error require an error_code? 2. Is there anyway to just throw the error with no error code or is that bad practice? – user1406186 Oct 21 '18 at 14:38
  • @user1406186 The call to attempt to create the directory will throw its own exception if it fails so you don't need to. I edited the answer to show equivalent code to yours in light of that. – Galik Oct 21 '18 at 14:51
  • @user1406186 If you don't want it to throw its own exception you need to supplu another parameter to `create_directories(path, std::error_code)`. That's how you obtain the error code. – Galik Oct 21 '18 at 14:53