From de93866101bc59d39b07a0082657671bc6e9eda6 Mon Sep 17 00:00:00 2001 From: Leonardo Grasso Date: Fri, 10 Apr 2026 14:52:15 +0200 Subject: [PATCH] chore: make plugin library path traversal check cross-platform Signed-off-by: Leonardo Grasso --- CMakeLists.txt | 17 ++++++--- .../falco/test_configuration_schema.cpp | 24 ++++++++++-- userspace/falco/configuration.h | 38 ++++++++++++------- 3 files changed, 56 insertions(+), 23 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 246187eb..a07413c3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,6 +1,6 @@ # SPDX-License-Identifier: Apache-2.0 # -# Copyright (C) 2023 The Falco Authors. +# Copyright (C) 2026 The Falco Authors. # # Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except # in compliance with the License. You may obtain a copy of the License at @@ -134,10 +134,17 @@ if(NOT DEFINED FALCO_COMPONENT_NAME) endif() if(CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT) - set(CMAKE_INSTALL_PREFIX - /usr - CACHE PATH "Default install path" FORCE - ) + if(WIN32) + set(CMAKE_INSTALL_PREFIX + "C:/Program Files/${CMAKE_PROJECT_NAME}" + CACHE PATH "Default install path" FORCE + ) + else() + set(CMAKE_INSTALL_PREFIX + /usr + CACHE PATH "Default install path" FORCE + ) + endif() endif() set(CMD_MAKE make) diff --git a/unit_tests/falco/test_configuration_schema.cpp b/unit_tests/falco/test_configuration_schema.cpp index 0556221f..cc7d762a 100644 --- a/unit_tests/falco/test_configuration_schema.cpp +++ b/unit_tests/falco/test_configuration_schema.cpp @@ -1,6 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 /* -Copyright (C) 2023 The Falco Authors. +Copyright (C) 2026 The Falco Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -158,9 +158,6 @@ plugins: )"; EXPECT_NO_THROW(res = falco_config.init_from_content(config, {})); EXPECT_VALIDATION_STATUS(res, yaml_helper::validation_ok); - // The resolved path must start with the plugins dir. - EXPECT_TRUE(sinsp_utils::startswith(falco_config.m_plugins[0].m_library_path, - FALCO_ENGINE_PLUGINS_DIR)); // A relative path with ".." that escapes the plugins dir must be rejected. config = R"( @@ -170,7 +167,25 @@ plugins: )"; EXPECT_THROW(falco_config.init_from_content(config, {}), std::exception); + // Traversal via "./" prefix followed by ".." must also be rejected. + config = R"( +plugins: + - name: evil + library_path: ./../../tmp/evil.so +)"; + EXPECT_THROW(falco_config.init_from_content(config, {}), std::exception); + + // Nested traversal that descends then escapes must be rejected. + config = R"( +plugins: + - name: evil + library_path: subdir/../../../tmp/evil.so +)"; + EXPECT_THROW(falco_config.init_from_content(config, {}), std::exception); + +#ifndef _WIN32 // Absolute paths bypass the prefix logic and are allowed as-is. + // This test uses a Unix absolute path syntax. config = R"( plugins: - name: myplugin @@ -179,6 +194,7 @@ plugins: EXPECT_NO_THROW(res = falco_config.init_from_content(config, {})); EXPECT_VALIDATION_STATUS(res, yaml_helper::validation_ok); EXPECT_EQ(falco_config.m_plugins[0].m_library_path, "/opt/falco/plugins/libmyplugin.so"); +#endif } TEST(Configuration, schema_yaml_helper_validator) { diff --git a/userspace/falco/configuration.h b/userspace/falco/configuration.h index 0ab5b670..bd328397 100644 --- a/userspace/falco/configuration.h +++ b/userspace/falco/configuration.h @@ -1,6 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 /* -Copyright (C) 2025 The Falco Authors. +Copyright (C) 2026 The Falco Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -401,25 +401,35 @@ struct convert { return false; } rhs.m_library_path = node["library_path"].as(); - if(!rhs.m_library_path.empty() && rhs.m_library_path.at(0) != '/') { - // prepend share dir if path is not absolute - rhs.m_library_path = std::string(FALCO_ENGINE_PLUGINS_DIR) + rhs.m_library_path; - // Canonicalize to resolve ".." components and verify - // the resulting path stays within the plugins directory. - auto canonical_str = std::filesystem::weakly_canonical(rhs.m_library_path).string(); - auto plugins_dir_str = - std::filesystem::weakly_canonical(FALCO_ENGINE_PLUGINS_DIR).string(); - if(!plugins_dir_str.empty() && plugins_dir_str.back() != '/') { - plugins_dir_str += '/'; - } - if(canonical_str.compare(0, plugins_dir_str.size(), plugins_dir_str) != 0) { + if(!rhs.m_library_path.empty() && + !std::filesystem::path(rhs.m_library_path).is_absolute() && + rhs.m_library_path.at(0) != '/') { + // Relative path: resolve against the plugins directory + // and verify the result stays within it. + auto full_path = std::filesystem::path(FALCO_ENGINE_PLUGINS_DIR) / rhs.m_library_path; + // lexically_normal resolves . and .. purely lexically, + // without filesystem access (unlike weakly_canonical which + // leaves .. unresolved for non-existent path components). + auto normalized = full_path.lexically_normal(); + auto plugins_dir = std::filesystem::path(FALCO_ENGINE_PLUGINS_DIR).lexically_normal(); + auto rel = normalized.lexically_relative(plugins_dir); + if(rel.empty()) { throw YAML::Exception(node["library_path"].Mark(), "plugin library_path '" + node["library_path"].as() + "' resolves outside the plugins directory (" + std::string(FALCO_ENGINE_PLUGINS_DIR) + ")"); } - rhs.m_library_path = canonical_str; + for(const auto& component : rel) { + if(component == "..") { + throw YAML::Exception(node["library_path"].Mark(), + "plugin library_path '" + + node["library_path"].as() + + "' resolves outside the plugins directory (" + + std::string(FALCO_ENGINE_PLUGINS_DIR) + ")"); + } + } + rhs.m_library_path = normalized.string(); } if(node["init_config"] && !node["init_config"].IsNull()) {