Skip to content

Commit df19aba

Browse files
gRPC path validation during backups to the file system (#32782)
Co-authored-by: Ilnaz Nizametdinov <i.nizametdinov@gmail.com>
1 parent 6a75b4a commit df19aba

File tree

7 files changed

+425
-70
lines changed

7 files changed

+425
-70
lines changed
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
#include "fs_path_validation.h"
2+
3+
#include <util/string/builder.h>
4+
#include <util/folder/pathsplit.h>
5+
6+
namespace NKikimr::NGRpcService {
7+
8+
namespace {
9+
10+
bool ValidatePathComponents(const TString& path, const TString& pathDescription, TString& error) {
11+
for (const auto& component : TPathSplit(path)) {
12+
if (component == "..") {
13+
error = TStringBuilder() << pathDescription << " contains path traversal sequence (..)";
14+
return false;
15+
}
16+
17+
if (component == ".") {
18+
error = TStringBuilder() << pathDescription << " contains current directory reference (.)";
19+
return false;
20+
}
21+
}
22+
return true;
23+
}
24+
25+
} // anonymous namespace
26+
27+
bool ValidateFsPath(const TString& path, const TString& pathDescription, TString& error) {
28+
if (path.empty()) {
29+
return true;
30+
}
31+
32+
// Check for null bytes
33+
if (path.find('\0') != TString::npos) {
34+
error = TStringBuilder() << pathDescription << " contains null byte";
35+
return false;
36+
}
37+
38+
#ifndef _win_
39+
if (path.Contains('\\')) {
40+
error = TStringBuilder() << pathDescription
41+
<< " contains invalid path separator backslash (\\)";
42+
return false;
43+
}
44+
#endif
45+
46+
return ValidatePathComponents(path, pathDescription, error);
47+
}
48+
49+
} // namespace NKikimr::NGRpcService
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
#pragma once
2+
3+
#include <util/generic/string.h>
4+
5+
namespace NKikimr::NGRpcService {
6+
7+
/**
8+
* Validates filesystem path for security vulnerabilities using current platform conventions.
9+
*
10+
* Platform-aware validation
11+
* Checks for:
12+
* - Null bytes
13+
* - Path traversal sequences (..)
14+
* - Current directory references (.)
15+
*
16+
* @param path The path to validate
17+
* @param pathDescription Human-readable description of the path (for error messages)
18+
* @param error Output parameter for error message if validation fails
19+
* @return true if path is valid, false otherwise
20+
*/
21+
bool ValidateFsPath(const TString& path, const TString& pathDescription, TString& error);
22+
23+
} // namespace NKikimr::NGRpcService

ydb/core/grpc_services/rpc_export.cpp

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include "rpc_export_base.h"
44
#include "rpc_calls.h"
55
#include "rpc_operation_request_base.h"
6+
#include "fs_path_validation.h"
67

78
#include <ydb/public/api/protos/ydb_export.pb.h>
89
#include <ydb/core/backup/common/encryption.h>
@@ -13,9 +14,15 @@
1314

1415
#include <ydb/library/actors/core/hfunc.h>
1516

17+
#include <util/folder/path.h>
1618
#include <util/generic/ptr.h>
1719
#include <util/string/builder.h>
1820

21+
#ifdef _linux_
22+
#include <sys/vfs.h>
23+
#include <linux/magic.h>
24+
#endif
25+
1926
namespace NKikimr {
2027
namespace NGRpcService {
2128

@@ -569,10 +576,34 @@ class TExportRPC: public TRpcOperationRequestActor<TDerived, TEvRequest, true>,
569576
}
570577

571578
if constexpr (IsFsExport) {
572-
if (!settings.base_path().StartsWith("/")) {
579+
if (!TFsPath(settings.base_path()).IsAbsolute()) {
573580
return this->Reply(StatusIds::BAD_REQUEST, TIssuesIds::DEFAULT_ERROR,
574581
"base_path must be an absolute path");
575582
}
583+
584+
#ifdef _linux_
585+
struct statfs fsInfo;
586+
if (statfs(settings.base_path().c_str(), &fsInfo) == 0) {
587+
if (fsInfo.f_type != NFS_SUPER_MAGIC) {
588+
return this->Reply(StatusIds::BAD_REQUEST, TIssuesIds::DEFAULT_ERROR,
589+
"base_path must be on NFS filesystem");
590+
}
591+
}
592+
#endif
593+
594+
TString error;
595+
if (!ValidateFsPath(settings.base_path(), "base_path", error)) {
596+
return this->Reply(StatusIds::BAD_REQUEST, TIssuesIds::DEFAULT_ERROR, error);
597+
}
598+
599+
for (const auto& item : TTraits::GetItems(settings)) {
600+
if (!TTraits::GetDestination(item).empty()) {
601+
const auto pathDesc = TStringBuilder() << "destination_path for item \"" << item.source_path() << "\"";
602+
if (!ValidateFsPath(TTraits::GetDestination(item), pathDesc, error)) {
603+
return this->Reply(StatusIds::BAD_REQUEST, TIssuesIds::DEFAULT_ERROR, error);
604+
}
605+
}
606+
}
576607
}
577608

578609
if constexpr (!TTraits::HasSourcePath) {

ydb/core/grpc_services/rpc_import.cpp

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include "rpc_import_base.h"
44
#include "rpc_calls.h"
55
#include "rpc_operation_request_base.h"
6+
#include "fs_path_validation.h"
67

78
#include <ydb/public/api/protos/ydb_import.pb.h>
89

@@ -13,9 +14,15 @@
1314

1415
#include <library/cpp/regex/pcre/regexp.h>
1516

17+
#include <util/folder/path.h>
1618
#include <util/generic/ptr.h>
1719
#include <util/string/builder.h>
1820

21+
#ifdef _linux_
22+
#include <sys/vfs.h>
23+
#include <linux/magic.h>
24+
#endif
25+
1926
namespace NKikimr {
2027
namespace NGRpcService {
2128

@@ -137,14 +144,37 @@ class TImportRPC: public TRpcOperationRequestActor<TDerived, TEvRequest, true>,
137144
}
138145

139146
if constexpr (IsFsImport) {
140-
if (!settings.base_path().StartsWith("/")) {
147+
if (!TFsPath(settings.base_path()).IsAbsolute()) {
141148
return this->Reply(StatusIds::BAD_REQUEST, TIssuesIds::DEFAULT_ERROR,
142149
"base_path must be an absolute path");
143150
}
151+
152+
#ifdef _linux_
153+
struct statfs fsInfo;
154+
if (statfs(settings.base_path().c_str(), &fsInfo) == 0) {
155+
if (fsInfo.f_type != NFS_SUPER_MAGIC) {
156+
return this->Reply(StatusIds::BAD_REQUEST, TIssuesIds::DEFAULT_ERROR,
157+
"base_path must be on NFS filesystem");
158+
}
159+
}
160+
#endif
161+
162+
TString error;
163+
if (!ValidateFsPath(settings.base_path(), "base_path", error)) {
164+
return this->Reply(StatusIds::BAD_REQUEST, TIssuesIds::DEFAULT_ERROR, error);
165+
}
166+
144167
for (const auto& item : settings.items()) {
145168
if (item.destination_path().empty() && item.source_path().empty()) {
146169
return this->Reply(StatusIds::BAD_REQUEST, TIssuesIds::DEFAULT_ERROR, "Empty item is not allowed");
147170
}
171+
172+
if (!item.source_path().empty()) {
173+
const auto pathDesc = TStringBuilder() << "source_path for item to \"" << item.destination_path() << "\"";
174+
if (!ValidateFsPath(item.source_path(), pathDesc, error)) {
175+
return this->Reply(StatusIds::BAD_REQUEST, TIssuesIds::DEFAULT_ERROR, error);
176+
}
177+
}
148178
}
149179
}
150180

ydb/core/grpc_services/ya.make

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ SRCS(
99
audit_log.cpp
1010
audit_logins.cpp
1111
db_metadata_cache.h
12+
fs_path_validation.cpp
1213
grpc_endpoint_publish_actor.cpp
1314
grpc_helper.cpp
1415
grpc_mon.cpp

0 commit comments

Comments
 (0)