diff --git a/changelog_unreleased/3832-CVE-2025-32803-Files-should-not-be-created-as-world--readable b/changelog_unreleased/3832-CVE-2025-32803-Files-should-not-be-created-as-world--readable new file mode 100644 index 0000000000..af617dc2ae --- /dev/null +++ b/changelog_unreleased/3832-CVE-2025-32803-Files-should-not-be-created-as-world--readable @@ -0,0 +1,8 @@ +[sec]* fdupont + Change the umask to no group write and no other access + at the entry of Kea server/agent binaries. + CVE:2025-32803 + (Gitlab #3832) + + + diff --git a/src/bin/agent/main.cc b/src/bin/agent/main.cc index 68687083a6..0d3b3c6b77 100644 --- a/src/bin/agent/main.cc +++ b/src/bin/agent/main.cc @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -14,6 +15,8 @@ using namespace isc::agent; using namespace isc::process; int main(int argc, char* argv[]) { + isc::util::file::setUmask(); + int ret = EXIT_SUCCESS; // Launch the controller passing in command line arguments. diff --git a/src/bin/d2/main.cc b/src/bin/d2/main.cc index 25975a77d3..7a06993e20 100644 --- a/src/bin/d2/main.cc +++ b/src/bin/d2/main.cc @@ -10,6 +10,7 @@ #include #include #include +#include #include @@ -23,6 +24,8 @@ using namespace std; /// The exit value of the program will be EXIT_SUCCESS if there were no /// errors, EXIT_FAILURE otherwise. int main(int argc, char* argv[]) { + isc::util::file::setUmask(); + int ret = EXIT_SUCCESS; // Launch the controller passing in command line arguments. diff --git a/src/bin/dhcp4/main.cc b/src/bin/dhcp4/main.cc index af577ab710..bdc3b7308b 100644 --- a/src/bin/dhcp4/main.cc +++ b/src/bin/dhcp4/main.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include @@ -71,6 +72,8 @@ usage() { int main(int argc, char* argv[]) { + isc::util::file::setUmask(); + int ch; // The default. Any other values are useful for testing only. int server_port_number = DHCP4_SERVER_PORT; diff --git a/src/bin/dhcp6/main.cc b/src/bin/dhcp6/main.cc index 15529c9e60..1ea3aea9f8 100644 --- a/src/bin/dhcp6/main.cc +++ b/src/bin/dhcp6/main.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include @@ -71,6 +72,8 @@ usage() { int main(int argc, char* argv[]) { + isc::util::file::setUmask(); + int ch; // The default. Any other values are useful for testing only. int server_port_number = DHCP6_SERVER_PORT; diff --git a/src/bin/lfc/main.cc b/src/bin/lfc/main.cc index 1b49e9625a..6b9430a67b 100644 --- a/src/bin/lfc/main.cc +++ b/src/bin/lfc/main.cc @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -24,6 +25,8 @@ using namespace isc::lfc; /// The exit value of the program will be EXIT_SUCCESS if there were no /// errors, EXIT_FAILURE otherwise. int main(int argc, char* argv[]) { + isc::util::file::setUmask(); + int ret = EXIT_SUCCESS; try { // Ask scheduling to not give too much resources to LFC. diff --git a/src/bin/netconf/main.cc b/src/bin/netconf/main.cc index a6ac2a14af..bc241ea51e 100644 --- a/src/bin/netconf/main.cc +++ b/src/bin/netconf/main.cc @@ -7,6 +7,7 @@ #include #include +#include #include #include @@ -18,6 +19,8 @@ using namespace isc::process; using namespace std; int main(int argc, char* argv[]) { + isc::util::file::setUmask(); + int ret = EXIT_SUCCESS; // Launch the controller passing in command line arguments. diff --git a/src/bin/perfdhcp/main.cc b/src/bin/perfdhcp/main.cc index 5262ace5fa..1572d8d79f 100644 --- a/src/bin/perfdhcp/main.cc +++ b/src/bin/perfdhcp/main.cc @@ -6,6 +6,7 @@ #include +#include #include #include #include @@ -19,6 +20,8 @@ using namespace isc::perfdhcp; int main(int argc, char* argv[]) { + isc::util::file::setUmask(); + int ret_code = 0; std::string diags; bool parser_error = true; diff --git a/src/lib/util/filesystem.cc b/src/lib/util/filesystem.cc index 4ff057466f..d6ae8e9939 100644 --- a/src/lib/util/filesystem.cc +++ b/src/lib/util/filesystem.cc @@ -78,6 +78,17 @@ isSocket(string const& path) { return ((statbuf.st_mode & S_IFMT) == S_IFSOCK); } +void +setUmask() { + // No group write and no other access. + mode_t mask(S_IWGRP | S_IRWXO); + mode_t orig = umask(mask); + // Handle the case where the original umask was already more restrictive. + if ((orig | mask) != mask) { + static_cast(umask(orig | mask)); + } +} + Path::Path(string const& full_name) { if (!full_name.empty()) { bool dir_present = false; @@ -225,14 +236,15 @@ FileManager::validatePath(const std::string supported_path_str, const std::strin if (filename.empty()) { isc_throw(BadValue, "path: '" << input_path.str() << "' has no filename"); } - + auto parent_path = input_path.parentPath(); if (!parent_path.empty()) { if (!enforce_path) { // Security set to lax, let it fly. return (input_path_str); } - + + // We only allow absolute path equal to default. Catch an invalid path. if (parent_path != supported_path_copy) { isc_throw(BadValue, "invalid path specified: '" @@ -240,7 +252,7 @@ FileManager::validatePath(const std::string supported_path_str, const std::strin << supported_path_copy << "'"); } } - + std::string valid_path(supported_path_copy + "/" + filename); return (valid_path); } diff --git a/src/lib/util/filesystem.h b/src/lib/util/filesystem.h index ce5a276189..0ab788c566 100644 --- a/src/lib/util/filesystem.h +++ b/src/lib/util/filesystem.h @@ -48,9 +48,19 @@ isDir(const std::string& path); bool isFile(const std::string& path); +/// @brief Check if there is a socket at the given path. +/// +/// @param path The path being checked. +/// +/// @return True if the path points to a socket, false otherwise including +/// if the pointed location does not exist. bool isSocket(const std::string& path); +/// @brief Set umask (at least 0027 i.e. no group write and no other access). +void +setUmask(); + /// @brief Paths on a filesystem struct Path { /// @brief Constructor diff --git a/src/lib/util/tests/filesystem_unittests.cc b/src/lib/util/tests/filesystem_unittests.cc index 1b3e88168a..3f8c6c2939 100644 --- a/src/lib/util/tests/filesystem_unittests.cc +++ b/src/lib/util/tests/filesystem_unittests.cc @@ -71,6 +71,39 @@ TEST_F(FileUtilTest, isFile) { EXPECT_FALSE(isFile(TEST_DATA_BUILDDIR)); } +/// @brief Test fixture class for testing operations on umask. +struct UMaskUtilTest : ::testing::Test { + /// @brief Constructor. + /// + /// Cache the original umask value. + UMaskUtilTest() : orig_umask_(umask(S_IWGRP | S_IWOTH)) { } + + /// @brief Destructor. + /// + /// Restore the original umask value. + virtual ~UMaskUtilTest() { + static_cast(umask(orig_umask_)); + } + +private: + /// @brief Original umask. + mode_t orig_umask_; +}; + +/// @brief Check setUmask from 0000. +TEST_F(UMaskUtilTest, umask0) { + static_cast(umask(0)); + ASSERT_NO_THROW(setUmask()); + EXPECT_EQ(S_IWGRP | S_IRWXO, umask(0)); +} + +/// @brief Check setUmask from no group access. +TEST_F(UMaskUtilTest, umask077) { + static_cast(umask(S_IRWXG | S_IRWXO)); + ASSERT_NO_THROW(setUmask()); + EXPECT_EQ(S_IRWXG | S_IRWXO, umask(0)); +} + /// @brief Check that the components are split correctly. TEST(PathTest, components) { // Complete name @@ -126,8 +159,6 @@ TEST(PathTest, replaceParentPath) { EXPECT_EQ("/just/some/dir/a.b", fname.str()); } - - // Verifies FileManager::validatePath() when enforce_path is true. TEST(FileManager, validatePathEnforcePath) { std::string def_path = std::string(TEST_DATA_BUILDDIR) + "/"; @@ -168,7 +199,7 @@ TEST(FileManager, validatePathEnforcePath) { "" }, { - // White space for file name. + // White space for file name. __LINE__, " ", "", @@ -233,7 +264,7 @@ TEST(FileManager, validatePathEnforcePathFalse) { "" }, { - // White space for file name. + // White space for file name. __LINE__, " ", "",