Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/capi_frontend/capi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
#include "servablemetadata.hpp"
#include "server_settings.hpp"
#include "serialization.hpp"
#include "../filesystem/filesystem.hpp"

using ovms::Buffer;
using ovms::ExecutionContext;
Expand Down Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion src/cli_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,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"))
Expand Down
23 changes: 23 additions & 0 deletions src/filesystem/filesystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
//*****************************************************************************
#include "filesystem.hpp"

#include <algorithm>
#include <memory>
#ifdef _WIN32
#include <windows.h>
Expand Down Expand Up @@ -200,4 +201,26 @@ 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;
#ifdef _WIN32
// Backslash is a path separator only on Windows. On POSIX it is a valid
// filename character; rewriting it would let a path like
// "/allowed\secret.jpg" be authorized as "/allowed/secret.jpg" while
// actually opening a sibling file outside the allowlist.
std::replace(normalized.begin(), normalized.end(), '\\', '/');
#endif
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
2 changes: 2 additions & 0 deletions src/filesystem/filesystem.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
22 changes: 18 additions & 4 deletions src/llm/apis/openai_api_handler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#include <algorithm>
#include <cmath>
#include <filesystem>
#include <limits>
#include <memory>
#include <sstream>
Expand Down Expand Up @@ -46,6 +47,17 @@ namespace ovms {

constexpr size_t DEFAULT_MAX_STOP_WORDS = 16; // same as deep-seek

namespace {

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);
Expand Down Expand Up @@ -234,15 +246,17 @@ 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 std::filesystem::path resolvedAllowedPath = FileSystem::normalizeConfiguredPath(allowedLocalMediaPath.value());
const std::string resolvedImagePathStr = FileSystem::normalizeConfiguredPath(imageSource);
const std::filesystem::path resolvedImagePath = resolvedImagePathStr;
if (!isPathInsideDirectory(resolvedImagePath, resolvedAllowedPath)) {
Comment on lines +249 to +252
return absl::InvalidArgumentError("Given filepath is not subpath of allowed_local_media_path");
}
try {
tensor = loadImageStbiFromFile(imageSource.c_str());
tensor = loadImageStbiFromFile(resolvedImagePathStr.c_str());
} catch (std::runtime_error& e) {
std::stringstream ss;
ss << "Image file " << imageSource.c_str() << " parsing failed: " << e.what();
ss << "Image file " << resolvedImagePathStr << " parsing failed: " << e.what();
SPDLOG_LOGGER_DEBUG(llm_calculator_logger, ss.str());
return absl::InvalidArgumentError(ss.str());
}
Expand Down
19 changes: 18 additions & 1 deletion src/test/c_api_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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;
Expand Down
Loading