From b75f3eb486af54c3c1e27f22024709c2f09f0f9c Mon Sep 17 00:00:00 2001 From: Ivan Chelyubeev <ijon@ydb.tech> Date: Wed, 25 Dec 2024 19:03:45 +0300 Subject: [PATCH] ydbd cli: fix vulnerability 101576 Some `ydbd schema` subcommands improperly parsed input path. KIKIMR-22386, CLOUD-215116 --- ydb/core/driver_lib/cli_base/cli_cmds_db.cpp | 126 +++++-------------- 1 file changed, 33 insertions(+), 93 deletions(-) diff --git a/ydb/core/driver_lib/cli_base/cli_cmds_db.cpp b/ydb/core/driver_lib/cli_base/cli_cmds_db.cpp index 204714ab8a2a..0a620256f483 100644 --- a/ydb/core/driver_lib/cli_base/cli_cmds_db.cpp +++ b/ydb/core/driver_lib/cli_base/cli_cmds_db.cpp @@ -27,6 +27,31 @@ void WarnProfilePathSet() { Cout << "FYI: profile path is set. You can use short pathnames. Try --help for more info." << Endl; } +// Get dirname and basename from the pathname using profile config +std::pair<TString, TString> SplitPath(const TClientCommand::TConfig& config, const TString& pathname) { + const bool pathIsAbsolute = pathname.StartsWith('/'); + if (config.Path) { + // Profile path is set + if (!pathIsAbsolute) { + return std::make_pair(config.Path, pathname); + } else { + WarnProfilePathSet(); + } + } + // path should be absolute here + if (!pathIsAbsolute) { + ythrow yexception() << "Relative path cannot be used without profile"; + } + + size_t pos = pathname.rfind('/'); + if (pos == TString::npos) { + // should be unreachable + ythrow yexception() << "No single '/' in the absolute path"; + } + + return std::make_pair(pathname.substr(0, pos), pathname.substr(pos + 1)); +} + class TClientCommandSchemaMkdir : public TClientCommand { public: TClientCommandSchemaMkdir() @@ -48,19 +73,7 @@ class TClientCommandSchemaMkdir : public TClientCommand { virtual void Parse(TConfig& config) override { TClientCommand::Parse(config); TString pathname = config.ParseResult->GetFreeArgs()[0]; - size_t pos = pathname.rfind('/'); - if (config.Path) { - // Profile path is set - if (!pathname.StartsWith('/')) { - Base = config.Path; - Name = pathname; - return; - } else { - WarnProfilePathSet(); - } - } - Base = pathname.substr(0, pos); - Name = pathname.substr(pos + 1); + std::tie(Base, Name) = SplitPath(config, pathname); } virtual int Run(TConfig& config) override { @@ -93,19 +106,7 @@ class TClientCommandSchemaDrop : public TClientCommand { virtual void Parse(TConfig& config) override { TClientCommand::Parse(config); TString pathname = config.ParseResult->GetFreeArgs()[0]; - size_t pos = pathname.rfind('/'); - if (config.Path) { - // Profile path is set - if (!pathname.StartsWith('/')) { - Base = config.Path; - Name = pathname; - return; - } else { - WarnProfilePathSet(); - } - } - Base = pathname.substr(0, pos); - Name = pathname.substr(pos + 1); + std::tie(Base, Name) = SplitPath(config, pathname); } virtual int Run(TConfig& config) override { @@ -601,6 +602,9 @@ class TClientCommandSchemaChown : public TClientCommand { } } Path = pathname; + if (Path.rfind('/') == TString::npos) { + ythrow yexception() << "No single '/' in the absolute path"; + } } int Chown(TConfig& config, const TString& path) { @@ -686,21 +690,7 @@ class TClientCommandSchemaAccessAdd : public TClientCommand { virtual void Parse(TConfig& config) override { TClientCommand::Parse(config); TString pathname = config.ParseResult->GetFreeArgs()[0]; - size_t pos = pathname.rfind('/'); - if (config.Path) { - // Profile path is set - if (!pathname.StartsWith('/')) { - Base = config.Path; - Name = pathname; - } else { - WarnProfilePathSet(); - Base = pathname.substr(0, pos); - Name = pathname.substr(pos + 1); - } - } else { - Base = pathname.substr(0, pos); - Name = pathname.substr(pos + 1); - } + std::tie(Base, Name) = SplitPath(config, pathname); Access = config.ParseResult->GetFreeArgs()[1]; } @@ -757,21 +747,7 @@ class TClientCommandSchemaAccessRemove : public TClientCommand { virtual void Parse(TConfig& config) override { TClientCommand::Parse(config); TString pathname = config.ParseResult->GetFreeArgs()[0]; - size_t pos = pathname.rfind('/'); - if (config.Path) { - // Profile path is set - if (!pathname.StartsWith('/')) { - Base = config.Path; - Name = pathname; - } else { - WarnProfilePathSet(); - Base = pathname.substr(0, pos); - Name = pathname.substr(pos + 1); - } - } else { - Base = pathname.substr(0, pos); - Name = pathname.substr(pos + 1); - } + std::tie(Base, Name) = SplitPath(config, pathname); Access = config.ParseResult->GetFreeArgs()[1]; } @@ -830,21 +806,7 @@ class TClientCommandSchemaAccessInheritanceBase : public TClientCommand { virtual void Parse(TConfig& config) override { TClientCommand::Parse(config); TString pathname = config.ParseResult->GetFreeArgs()[0]; - size_t pos = pathname.rfind('/'); - if (config.Path) { - // Profile path is set - if (!pathname.StartsWith('/')) { - Base = config.Path; - Name = pathname; - } else { - WarnProfilePathSet(); - Base = pathname.substr(0, pos); - Name = pathname.substr(pos + 1); - } - } else { - Base = pathname.substr(0, pos); - Name = pathname.substr(pos + 1); - } + std::tie(Base, Name) = SplitPath(config, pathname); } virtual int Run(TConfig& config) override { @@ -1154,28 +1116,6 @@ class TClientCommandSchemaUserAttributeGet: public TClientCommand { } }; -std::pair<TString, TString> SplitPath(const TClientCommand::TConfig& config, const TString& pathname) { - std::pair<TString, TString> result; - - size_t pos = pathname.rfind('/'); - if (config.Path) { - // Profile path is set - if (!pathname.StartsWith('/')) { - result.first = config.Path; - result.second = pathname; - } else { - WarnProfilePathSet(); - result.first = pathname.substr(0, pos); - result.second = pathname.substr(pos + 1); - } - } else { - result.first = pathname.substr(0, pos); - result.second = pathname.substr(pos + 1); - } - - return result; -} - class TClientCommandSchemaUserAttributeSet: public TClientCommand { using TUserAttributesLimits = NSchemeShard::TUserAttributesLimits;