Skip to content

Commit

Permalink
Add pdns::openFileForWriting() to control permissions when creating…
Browse files Browse the repository at this point in the history
… a file
  • Loading branch information
rgacogne committed Mar 18, 2024
1 parent 114b879 commit b1564d4
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 14 deletions.
3 changes: 1 addition & 2 deletions pdns/dnspcap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -235,8 +235,7 @@ PcapPacketWriter::PcapPacketWriter(const string& fname, const PcapPacketReader&

PcapPacketWriter::PcapPacketWriter(const string& fname) : d_fname(fname)
{
d_fp = pdns::UniqueFilePtr(fopen(fname.c_str(),"w"));

d_fp = pdns::openFileForWriting(fname, 0600, true, false);
if (!d_fp) {
unixDie("Unable to open file");
}
Expand Down
5 changes: 3 additions & 2 deletions pdns/dnspcap2protobuf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,10 @@ try {

PcapPacketReader pr(argv[1]);

auto filePtr = pdns::UniqueFilePtr(fopen(argv[2], "w"));
auto filePtr = pdns::openFileForWriting(argv[2], 0600, true, false);

Check warning on line 66 in pdns/dnspcap2protobuf.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, auth)

do not use pointer arithmetic (cppcoreguidelines-pro-bounds-pointer-arithmetic - Level=Warning)
if (!filePtr) {
cerr<<"Error opening output file "<<argv[2]<<": "<<stringerror()<<endl;
auto error = errno;
cerr<<"Error opening output file "<<argv[2]<<": "<<stringerror(error)<<endl;

Check warning on line 69 in pdns/dnspcap2protobuf.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, auth)

do not use pointer arithmetic (cppcoreguidelines-pro-bounds-pointer-arithmetic - Level=Warning)
exit(EXIT_FAILURE);
}

Expand Down
13 changes: 3 additions & 10 deletions pdns/libssl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1053,20 +1053,13 @@ static void libssl_key_log_file_callback(const SSL* ssl, const char* line)
pdns::UniqueFilePtr libssl_set_key_log_file(std::unique_ptr<SSL_CTX, decltype(&SSL_CTX_free)>& ctx, const std::string& logFile)
{
#ifdef HAVE_SSL_CTX_SET_KEYLOG_CALLBACK
int fd = open(logFile.c_str(), O_WRONLY | O_CREAT | O_APPEND, 0600);
if (fd == -1) {
unixDie("Error opening TLS log file '" + logFile + "'");
}
auto filePtr = pdns::UniqueFilePtr(fdopen(fd, "a"));
auto filePtr = pdns::openFileForWriting(logFile, 0600, false, true);
if (!filePtr) {
int error = errno; // close might clobber errno
close(fd);
throw std::runtime_error("Error opening TLS log file '" + logFile + "': " + stringerror(error));
auto error = errno;
throw std::runtime_error("Error opening file " + logFile + " for writing: " + stringerror(error));
}

SSL_CTX_set_ex_data(ctx.get(), s_keyLogIndex, filePtr.get());
SSL_CTX_set_keylog_callback(ctx.get(), &libssl_key_log_file_callback);

return filePtr;
#else
return pdns::UniqueFilePtr(nullptr);
Expand Down
24 changes: 24 additions & 0 deletions pdns/misc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1777,4 +1777,28 @@ std::optional<std::string> visit_directory(const std::string& directory, const s

return std::nullopt;
}

UniqueFilePtr openFileForWriting(const std::string& filePath, mode_t permissions, bool mustNotExist, bool appendIfExists)
{
int flags = O_WRONLY | O_CREAT;
if (mustNotExist) {
flags |= O_EXCL;
}
else if (appendIfExists) {
flags |= O_APPEND;
}
int fileDesc = open(filePath.c_str(), flags, permissions);

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This argument to a file access function is derived from
user input (a command-line argument)
and then passed to open(__file).
if (fileDesc == -1) {
return UniqueFilePtr(nullptr);

Check warning on line 1792 in pdns/misc.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, auth)

avoid repeating the return type from the declaration; use a braced initializer list instead (modernize-return-braced-init-list - Level=Warning)

Check warning on line 1792 in pdns/misc.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, rec)

avoid repeating the return type from the declaration; use a braced initializer list instead (modernize-return-braced-init-list - Level=Warning)

Check warning on line 1792 in pdns/misc.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, dnsdist)

avoid repeating the return type from the declaration; use a braced initializer list instead (modernize-return-braced-init-list - Level=Warning)
}
auto filePtr = pdns::UniqueFilePtr(fdopen(fileDesc, appendIfExists ? "a" : "w"));
if (!filePtr) {
auto error = errno;
close(fileDesc);
errno = error;
return UniqueFilePtr(nullptr);

Check warning on line 1799 in pdns/misc.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, auth)

avoid repeating the return type from the declaration; use a braced initializer list instead (modernize-return-braced-init-list - Level=Warning)

Check warning on line 1799 in pdns/misc.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, rec)

avoid repeating the return type from the declaration; use a braced initializer list instead (modernize-return-braced-init-list - Level=Warning)

Check warning on line 1799 in pdns/misc.cc

View workflow job for this annotation

GitHub Actions / Analyze (cpp, dnsdist)

avoid repeating the return type from the declaration; use a braced initializer list instead (modernize-return-braced-init-list - Level=Warning)
}
return filePtr;
}

}
2 changes: 2 additions & 0 deletions pdns/misc.hh
Original file line number Diff line number Diff line change
Expand Up @@ -852,4 +852,6 @@ struct FilePtrDeleter
};

using UniqueFilePtr = std::unique_ptr<FILE, FilePtrDeleter>;

UniqueFilePtr openFileForWriting(const std::string& filePath, mode_t permissions, bool mustNotExist = true, bool appendIfExists = false);
}

0 comments on commit b1564d4

Please sign in to comment.