Validate group identifiers before using them in filesystem paths

Group names received via the set-group command were only checked for
being non-empty before being concatenated into temporary and shared
configuration file paths, and into the group download URL. A malicious
or compromised manager could send a group name such as
'../../etc/wazuh-agent' to resolve the destination outside the shared
configuration directory and have the agent write there.

Add IsValidGroupId() to restrict group names to alphanumeric characters,
'_' and '-', rejecting path separators, '..' and leading dots before any
path construction. As defense in depth, normalize the resolved
destination path and verify it stays under the shared configuration
directory before creating directories or moving the file, and compare
normalized paths in the temporary-file cleanup guard.

Closes #858

Author: allamiro

src/agent/centralized_configuration/src/centralized_configuration.cpp

@@ -6,6 +6,8 @@
 #include <filesystem_wrapper.hpp>
 #include <logger.hpp>
 
+#include <algorithm>
+#include <cctype>
 #include <chrono>
 #include <filesystem>
 #include <random>
@@ -26,6 +28,19 @@ namespace
 
         return std::to_string(timestamp) + "_" + std::to_string(random);
     }
+
+    bool IsValidGroupId(const std::string& groupId)
+    {
+        if (groupId.empty() || groupId == "." || groupId == ".." || groupId.front() == '.')
+        {
+            return false;
+        }
+
+        return std::all_of(groupId.begin(),
+                           groupId.end(),
+                           [](const unsigned char character)
+                           { return std::isalnum(character) != 0 || character == '_' || character == '-'; });
+    }
 } // namespace
 
 namespace centralized_configuration
@@ -64,12 +79,12 @@ namespace centralized_configuration
             if (command == module_command::SET_GROUP_COMMAND)
             {
                 groupIds = parameters.at(module_command::GROUPS_ARG).get<std::vector<std::string>>();
-                if (!std::all_of(groupIds.begin(), groupIds.end(), [](const std::string& id) { return !id.empty(); }))
+                if (!std::all_of(groupIds.begin(), groupIds.end(), IsValidGroupId))
                 {
-                    LogWarn("Group name can not be an empty string.");
+                    LogWarn("Invalid group name. Group names may only contain alphanumeric characters, '_' or '-'.");
                     co_return module_command::CommandExecutionResult {
                         module_command::Status::FAILURE,
-                        "CentralizedConfiguration group set failed, a group name can not be an empty string."};
+                        "CentralizedConfiguration group set failed, invalid group name received."};
                 }
 
                 if (!m_setGroupIdFunction(groupIds))
@@ -136,7 +151,8 @@ namespace centralized_configuration
                     try
                     {
                         if (m_fileSystemWrapper->exists(tmpGroupFile) &&
-                            tmpGroupFile.parent_path() == m_fileSystemWrapper->temp_directory_path())
+                            tmpGroupFile.parent_path().lexically_normal() ==
+                                m_fileSystemWrapper->temp_directory_path().lexically_normal())
                         {
                             if (!m_fileSystemWrapper->remove(tmpGroupFile))
                             {
@@ -156,8 +172,19 @@ namespace centralized_configuration
                         "CentralizedConfiguration validate file failed, invalid file received."};
                 }
 
-                const std::filesystem::path destGroupFile = std::filesystem::path(config::DEFAULT_SHARED_CONFIG_PATH) /
-                                                            (groupId + config::DEFAULT_SHARED_FILE_EXTENSION);
+                const std::filesystem::path sharedConfigPath =
+                    std::filesystem::path(config::DEFAULT_SHARED_CONFIG_PATH);
+                const std::filesystem::path destGroupFile =
+                    (sharedConfigPath / (groupId + config::DEFAULT_SHARED_FILE_EXTENSION)).lexically_normal();
+
+                if (destGroupFile.parent_path().lexically_normal() != sharedConfigPath.lexically_normal())
+                {
+                    LogWarn("Resolved group file path is outside the shared configuration directory: {}",
+                            destGroupFile.string());
+                    co_return module_command::CommandExecutionResult {
+                        module_command::Status::FAILURE,
+                        "CentralizedConfiguration group set failed, invalid group destination path."};
+                }
 
                 try
                 {

src/agent/centralized_configuration/tests/centralized_configuration_tests.cpp

@@ -163,12 +163,49 @@ TEST(CentralizedConfiguration, ExecuteCommandReturnsFailureOnParseParameters)
                                         "CentralizedConfiguration error while parsing parameters");
 
             const nlohmann::json parameterListCase3 = nlohmann::json::parse(R"({"groups":["", "group2"]})");
-            co_await TestExecuteCommand(
-                centralizedConfiguration,
-                "set-group",
-                parameterListCase3,
-                module_command::Status::FAILURE,
-                "CentralizedConfiguration group set failed, a group name can not be an empty string.");
+            co_await TestExecuteCommand(centralizedConfiguration,
+                                        "set-group",
+                                        parameterListCase3,
+                                        module_command::Status::FAILURE,
+                                        "CentralizedConfiguration group set failed, invalid group name received.");
+        }(),
+        boost::asio::detached);
+
+    io_context.run();
+}
+
+TEST(CentralizedConfiguration, ExecuteCommandRejectsInvalidGroupNames)
+{
+    boost::asio::io_context io_context;
+
+    boost::asio::co_spawn(
+        io_context,
+        []() -> boost::asio::awaitable<void>
+        {
+            CentralizedConfiguration centralizedConfiguration(
+                [](const std::vector<std::string>&) { return true; },
+                []() { return std::vector<std::string> {}; },
+                [](std::string, std::string) -> boost::asio::awaitable<bool> { co_return true; },
+                [](const std::filesystem::path&) { return true; },
+                []() {});
+
+            const std::vector<nlohmann::json> invalidGroupCases = {
+                nlohmann::json::parse(R"({"groups":["../../etc/wazuh-agent"]})"),
+                nlohmann::json::parse(R"({"groups":["/etc/wazuh-agent"]})"),
+                nlohmann::json::parse(R"({"groups":["..\\windows"]})"),
+                nlohmann::json::parse(R"({"groups":[".."]})"),
+                nlohmann::json::parse(R"({"groups":[".hidden"]})"),
+                nlohmann::json::parse(R"({"groups":["group/../escape"]})"),
+                nlohmann::json::parse(R"({"groups":["good", "bad/name"]})")};
+
+            for (const auto& invalidCase : invalidGroupCases)
+            {
+                co_await TestExecuteCommand(centralizedConfiguration,
+                                            "set-group",
+                                            invalidCase,
+                                            module_command::Status::FAILURE,
+                                            "CentralizedConfiguration group set failed, invalid group name received.");
+            }
         }(),
         boost::asio::detached);