Fix allowed_local_media_path psirt

src/capi_frontend/capi.cpp

@@ -62,6 +62,7 @@
 #include "servablemetadata.hpp"
 #include "server_settings.hpp"
 #include "serialization.hpp"
+#include "../filesystem/filesystem.hpp"
 
 using ovms::Buffer;
 using ovms::ExecutionContext;
@@ -585,7 +586,7 @@ DLL_PUBLIC OVMS_Status* OVMS_ServerSettingsSetAllowedLocalMediaPath(OVMS_ServerS
         return reinterpret_cast<OVMS_Status*>(new Status(StatusCode::NONEXISTENT_PTR, "log path"));
     }
     ovms::ServerSettingsImpl* serverSettings = reinterpret_cast<ovms::ServerSettingsImpl*>(settings);
-    serverSettings->allowedLocalMediaPath = allowed_local_media_path;
+    serverSettings->allowedLocalMediaPath = ovms::FileSystem::normalizeConfiguredPath(allowed_local_media_path);
     return nullptr;
 }
 

src/cli_parser.cpp

@@ -530,7 +530,7 @@ void CLIParser::prepareServer(ServerSettingsImpl& serverSettings) {
         serverSettings.allowedMediaDomains = result->operator[]("allowed_media_domains").as<std::vector<std::string>>();
     }
     if (result->count("allowed_local_media_path")) {
-        serverSettings.allowedLocalMediaPath = result->operator[]("allowed_local_media_path").as<std::string>();
+        serverSettings.allowedLocalMediaPath = FileSystem::normalizeConfiguredPath(result->operator[]("allowed_local_media_path").as<std::string>());
     }
 
     if (result->count("grpc_bind_address"))

src/filesystem/filesystem.cpp

@@ -15,6 +15,7 @@
 //*****************************************************************************
 #include "filesystem.hpp"
 
+#include <algorithm>
 #include <memory>
 #ifdef _WIN32
 #include <windows.h>
@@ -200,4 +201,20 @@ Status FileSystem::createFileOverwrite(const std::string& filePath, const std::s
     return StatusCode::OK;
 }
 
+std::string FileSystem::normalizeConfiguredPath(const std::string& pathString) {
+    std::string normalized = pathString;
+    std::replace(normalized.begin(), normalized.end(), '\\', '/');
+    std::filesystem::path path(normalized);
+    if (path.is_relative()) {
+        path = std::filesystem::current_path() / path;
+    }
+    path = path.lexically_normal();
+    std::error_code ec;
+    auto weakCanonicalPath = std::filesystem::weakly_canonical(path, ec);
+    if (!ec) {
+        return weakCanonicalPath.lexically_normal().string();
+    }
+    return path.string();
+}
+
 }  // namespace ovms

src/filesystem/filesystem.hpp

@@ -231,6 +231,8 @@ class FileSystem {
         return !path.empty() && (path[0] == '/');
     }
 
+    static std::string normalizeConfiguredPath(const std::string& pathString);
+
     static std::string joinPath(std::initializer_list<std::string> segments) {
         std::string joined;
 

src/llm/apis/openai_api_handler.cpp

@@ -18,6 +18,7 @@
 
 #include <algorithm>
 #include <cmath>
+#include <filesystem>
 #include <limits>
 #include <memory>
 #include <sstream>
@@ -46,6 +47,48 @@ namespace ovms {
 
 constexpr size_t DEFAULT_MAX_STOP_WORDS = 16;  // same as deep-seek
 
+namespace {
+
+std::string normalizePathSeparators(const std::string& path) {
+    std::string normalizedPath = path;
+    std::replace(normalizedPath.begin(), normalizedPath.end(), '\\', '/');
+    return normalizedPath;
+}
+
+std::string normalizeAllowedLocalMediaPath(const std::string& allowedLocalMediaPath) {
+    std::string normalizedAllowedLocalMediaPath = normalizePathSeparators(allowedLocalMediaPath);
+    if (normalizedAllowedLocalMediaPath.empty()) {
+        return normalizedAllowedLocalMediaPath;
+    }
+    if (normalizedAllowedLocalMediaPath.back() == '/') {
+        return normalizedAllowedLocalMediaPath;
+    }
+    return normalizedAllowedLocalMediaPath + '/';
+}
+
+std::filesystem::path normalizeAndResolvePath(const std::string& pathString) {
+    std::filesystem::path path = normalizePathSeparators(pathString);
+    if (path.is_relative()) {
+        path = std::filesystem::current_path() / path;
+    }
+    path = path.lexically_normal();
+    std::error_code ec;
+    auto weakCanonicalPath = std::filesystem::weakly_canonical(path, ec);
+    if (!ec) {
+        return weakCanonicalPath.lexically_normal();
+    }
+    return path;
+}
+
+bool isPathInsideDirectory(const std::filesystem::path& testedPath, const std::filesystem::path& allowedDirectory) {
+    const auto mismatch = std::mismatch(
+        allowedDirectory.begin(), allowedDirectory.end(),
+        testedPath.begin(), testedPath.end());
+    return mismatch.first == allowedDirectory.end();
+}
+
+}  // namespace
+
 ov::genai::JsonContainer rapidJsonValueToJsonContainer(const rapidjson::Value& value) {
     if (value.IsNull()) {
         return ov::genai::JsonContainer(nullptr);
@@ -234,8 +277,14 @@ absl::StatusOr<ov::Tensor> loadImage(const std::string& imageSource,
             return absl::InvalidArgumentError(ss.str());
         }
         SPDLOG_LOGGER_TRACE(llm_calculator_logger, "Loading image from local filesystem");
-        const auto firstMissmatch = std::mismatch(imageSource.begin(), imageSource.end(), allowedLocalMediaPath.value().begin(), allowedLocalMediaPath.value().end());
-        if (firstMissmatch.second != allowedLocalMediaPath.value().end()) {
+        const auto normalizedAllowedLocalMediaPath = normalizeAllowedLocalMediaPath(allowedLocalMediaPath.value());
+        const auto imagePath = std::filesystem::path(normalizePathSeparators(imageSource));
+        if (imagePath.is_relative()) {
+            return absl::InvalidArgumentError("Relative paths are not allowed for local filesystem access");
+        }
+        const auto resolvedAllowedPath = normalizeAndResolvePath(normalizedAllowedLocalMediaPath);
+        const auto resolvedImagePath = normalizeAndResolvePath(imageSource);
+        if (!isPathInsideDirectory(resolvedImagePath, resolvedAllowedPath)) {
             return absl::InvalidArgumentError("Given filepath is not subpath of allowed_local_media_path");
         }
         try {

src/test/c_api_tests.cpp

@@ -38,6 +38,7 @@
 #include "../capi_frontend/capi_dag_utils.hpp"
 #include "../capi_frontend/servablemetadata.hpp"
 #include "../dags/pipelinedefinitionstatus.hpp"
+#include "../filesystem/filesystem.hpp"
 #include "src/metrics/metric_module.hpp"
 #include "../ovms.h"
 #include "../servablemanagermodule.hpp"
@@ -183,7 +184,7 @@ TEST(CAPIConfigTest, MultiModelConfiguration) {
     EXPECT_EQ(serverSettings->logLevel, "TRACE");
     EXPECT_EQ(serverSettings->logPath, getGenericFullPathForTmp("/tmp/logs"));
     ASSERT_TRUE(serverSettings->allowedLocalMediaPath.has_value());
-    EXPECT_EQ(serverSettings->allowedLocalMediaPath.value(), getGenericFullPathForTmp("/tmp/path"));
+    EXPECT_EQ(serverSettings->allowedLocalMediaPath.value(), ovms::FileSystem::normalizeConfiguredPath(getGenericFullPathForTmp("/tmp/path")));
     ASSERT_TRUE(serverSettings->allowedMediaDomains.has_value());
     EXPECT_EQ(serverSettings->allowedMediaDomains.value().size(), 3);
     EXPECT_EQ(serverSettings->allowedMediaDomains.value()[0], "raw.githubusercontent.com");
@@ -258,6 +259,22 @@ TEST(CAPIConfigTest, SingleModelConfiguration) {
     GTEST_SKIP() << "Use C-API to initialize in next stages, currently not supported";
 }
 
+TEST(CAPIConfigTest, AllowedLocalMediaPathRelativeIsNormalized) {
+    OVMS_ServerSettings* serverSettingsRaw = nullptr;
+    ASSERT_CAPI_STATUS_NULL(OVMS_ServerSettingsNew(&serverSettingsRaw));
+    ASSERT_NE(serverSettingsRaw, nullptr);
+    ServerSettingsImpl* serverSettings = reinterpret_cast<ServerSettingsImpl*>(serverSettingsRaw);
+
+    ASSERT_CAPI_STATUS_NULL(OVMS_ServerSettingsSetAllowedLocalMediaPath(serverSettingsRaw, "src/test"));
+    ASSERT_TRUE(serverSettings->allowedLocalMediaPath.has_value());
+
+    const auto configuredPath = std::filesystem::path(serverSettings->allowedLocalMediaPath.value());
+    const auto expectedPath = std::filesystem::path(ovms::FileSystem::normalizeConfiguredPath("src/test"));
+    EXPECT_EQ(configuredPath.lexically_normal(), expectedPath.lexically_normal());
+
+    OVMS_ServerSettingsDelete(serverSettingsRaw);
+}
+
 TEST(CAPIStartTest, InitializingMultipleServers) {
     OVMS_Server* srv1 = nullptr;
     OVMS_Server* srv2 = nullptr;

src/test/http_openai_handler_test.cpp

@@ -14,6 +14,7 @@
 // limitations under the License.
 //*****************************************************************************
 #include <chrono>
+#include <algorithm>
 #include <fstream>
 #include <functional>
 #include <map>
@@ -2146,7 +2147,77 @@ TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemWithinAl
     EXPECT_EQ(json, std::string("{\"model\":\"llama\",\"messages\":[{\"role\":\"user\",\"content\":\"What is in this image?\"}]}"));
 }
 
+TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemWithinAllowedPathMixedSeparators) {
+    std::string json = R"({
+"model": "llama",
+"messages": [
+  {
+    "role": "user",
+    "content": [
+      {
+        "type": "text",
+        "text": "What is in this image?"
+      },
+      {
+        "type": "image_url",
+        "image_url": {
+          "url":  ")" + getGenericFullPathForSrcTest("/ovms/src/test/binaryutils/rgb.jpg") +
+                       R"("
+        }
+      }
+    ]
+  }
+]
+})";
+    doc.Parse(json.c_str());
+    ASSERT_FALSE(doc.HasParseError());
+
+    std::string mixedSeparatorAllowedPath = getGenericFullPathForSrcTest("/ovms/src/test/binaryutils");
+    std::replace(mixedSeparatorAllowedPath.begin(), mixedSeparatorAllowedPath.end(), '/', '\\');
+
+    std::shared_ptr<ovms::OpenAIChatCompletionsHandler> apiHandler = std::make_shared<ovms::OpenAIChatCompletionsHandler>(doc, ovms::Endpoint::CHAT_COMPLETIONS, std::chrono::system_clock::now(), *tokenizer);
+    ASSERT_EQ(apiHandler->parseMessages(mixedSeparatorAllowedPath), absl::OkStatus());
+    const ovms::ImageHistory& imageHistory = apiHandler->getImageHistory();
+    ASSERT_EQ(imageHistory.size(), 1);
+    auto [index, image] = imageHistory[0];
+    EXPECT_EQ(index, 0);
+    EXPECT_EQ(image.get_element_type(), ov::element::u8);
+    EXPECT_EQ(image.get_size(), 3);
+}
+
 TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemNotWithinAllowedPath) {
+    const std::string imageUrl = getGenericFullPathForSrcTest("/ovms/src/test/binaryutils/rgb.jpg");
+    const std::string allowedPath = getGenericFullPathForSrcTest("/ovms/src/test/llm");
+    std::string json = R"({
+"model": "llama",
+"messages": [
+  {
+    "role": "user",
+    "content": [
+      {
+        "type": "text",
+        "text": "What is in this image?"
+      },
+      {
+        "type": "image_url",
+        "image_url": {
+          "url":  ")" + imageUrl +
+                       R"("
+        }
+      }
+    ]
+  }
+]
+})";
+    doc.Parse(json.c_str());
+    ASSERT_FALSE(doc.HasParseError());
+    std::shared_ptr<ovms::OpenAIChatCompletionsHandler> apiHandler = std::make_shared<ovms::OpenAIChatCompletionsHandler>(doc, ovms::Endpoint::CHAT_COMPLETIONS, std::chrono::system_clock::now(), *tokenizer);
+    ASSERT_EQ(apiHandler->parseMessages(allowedPath), absl::InvalidArgumentError("Given filepath is not subpath of allowed_local_media_path"));
+}
+
+TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemPrefixPathBypassPrevented) {
+    const std::string allowedLocalMediaPath = getGenericFullPathForSrcTest("/ovms/src/test/binaryutils");
+    const std::string siblingPrefixPath = allowedLocalMediaPath + "_private/rgb.jpg";
     std::string json = R"({
 "model": "llama",
 "messages": [
@@ -2160,7 +2231,8 @@ TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemNotWithi
       {
         "type": "image_url",
         "image_url": {
-          "url":  "/ovms/src/test/binaryutils/rgb.jpg"
+          "url":  ")" + siblingPrefixPath +
+                       R"("
         }
       }
     ]
@@ -2170,10 +2242,12 @@ TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemNotWithi
     doc.Parse(json.c_str());
     ASSERT_FALSE(doc.HasParseError());
     std::shared_ptr<ovms::OpenAIChatCompletionsHandler> apiHandler = std::make_shared<ovms::OpenAIChatCompletionsHandler>(doc, ovms::Endpoint::CHAT_COMPLETIONS, std::chrono::system_clock::now(), *tokenizer);
-    ASSERT_EQ(apiHandler->parseMessages("src/test"), absl::InvalidArgumentError("Given filepath is not subpath of allowed_local_media_path"));
+    ASSERT_EQ(apiHandler->parseMessages(allowedLocalMediaPath), absl::InvalidArgumentError("Given filepath is not subpath of allowed_local_media_path"));
 }
 
 TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemInvalidPath) {
+    const std::string allowedPath = getGenericFullPathForSrcTest("/ovms/src/test/");
+    const std::string imageUrl = getGenericFullPathForSrcTest("/ovms/src/test/not_exisiting.jpeg");
     std::string json = R"({
   "model": "llama",
   "messages": [
@@ -2187,7 +2261,8 @@ TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemInvalidP
         {
           "type": "image_url",
           "image_url": {
-            "url":  "/ovms/not_exisiting.jpeg"
+            "url":  ")" +
+                       imageUrl + R"("
           }
         }
       ]
@@ -2197,7 +2272,7 @@ TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemInvalidP
     doc.Parse(json.c_str());
     ASSERT_FALSE(doc.HasParseError());
     std::shared_ptr<ovms::OpenAIChatCompletionsHandler> apiHandler = std::make_shared<ovms::OpenAIChatCompletionsHandler>(doc, ovms::Endpoint::CHAT_COMPLETIONS, std::chrono::system_clock::now(), *tokenizer);
-    EXPECT_EQ(apiHandler->parseMessages("/ovms/"), absl::InvalidArgumentError("Image file /ovms/not_exisiting.jpeg parsing failed: can't fopen"));
+    EXPECT_EQ(apiHandler->parseMessages(allowedPath), absl::InvalidArgumentError("Image file " + imageUrl + " parsing failed: can't fopen"));
 }
 
 TEST_F(HttpOpenAIHandlerParsingTest, ParsingMessagesImageLocalFilesystemInvalidEscaped) {

src/test/ovmsconfig_test.cpp

@@ -2364,7 +2364,7 @@ TEST(OvmsConfigTest, positiveMulti) {
 #endif
     EXPECT_EQ(config.cacheDir(), "/tmp/model_cache");
     ASSERT_TRUE(config.getServerSettings().allowedLocalMediaPath.has_value());
-    EXPECT_EQ(config.getServerSettings().allowedLocalMediaPath.value(), "/tmp/path");
+    EXPECT_EQ(config.getServerSettings().allowedLocalMediaPath.value(), ovms::FileSystem::normalizeConfiguredPath("/tmp/path"));
     ASSERT_TRUE(config.getServerSettings().allowedMediaDomains.has_value());
     EXPECT_EQ(config.getServerSettings().allowedMediaDomains.value().size(), 3);
     EXPECT_EQ(config.getServerSettings().allowedMediaDomains.value()[0], "raw.githubusercontent.com");
@@ -2385,6 +2385,25 @@ TEST(OvmsConfigTest, positiveMulti) {
 #endif
 }
 
+TEST(OvmsConfigTest, allowedLocalMediaPathRelativeIsNormalized) {
+    char* n_argv[] = {
+        "ovms",
+        "--rest_port", "45",
+        "--allowed_local_media_path",
+        "src/test",
+        "--config_path",
+        "/config.json"};
+
+    int arg_count = 7;
+    ConstructorEnabledConfig config;
+    config.parse(arg_count, n_argv);
+
+    ASSERT_TRUE(config.getServerSettings().allowedLocalMediaPath.has_value());
+    const auto configuredPath = std::filesystem::path(config.getServerSettings().allowedLocalMediaPath.value());
+    const auto expectedPath = std::filesystem::path(ovms::FileSystem::normalizeConfiguredPath("src/test"));
+    EXPECT_EQ(configuredPath.lexically_normal(), expectedPath.lexically_normal());
+}
+
 TEST(OvmsConfigTest, positiveSingle) {
 #ifdef _WIN32
     const std::string cpu_extension_lib_path = "tmp_cpu_extension_library_dir";