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;