From: Junghyun Yeon <jungh.yeon@samsung.com>
Date: Tue, 24 Aug 2021 01:41:02 +0000 (+0900)
Subject: Add checking routine to prevent path traverse attack
X-Git-Tag: submit/tizen/20210826.063234~4
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=1b3abafca8350eda8f2f293589527217cc2c2546;p=platform%2Fcore%2Fappfw%2Fpkgmgr-tool.git

Add checking routine to prevent path traverse attack

Change-Id: Ie90df0c9d1075725870cdce7c86f7c1a6c5b703a
Signed-off-by: Junghyun Yeon <jungh.yeon@samsung.com>
---

diff --git a/src/res-copy/include/param_checker.hh b/src/res-copy/include/param_checker.hh
index 437a0e2..7434863 100644
--- a/src/res-copy/include/param_checker.hh
+++ b/src/res-copy/include/param_checker.hh
@@ -43,6 +43,7 @@ class ParamChecker {
 
   void SetRequestType(std::string key);
   bool ValidatePkgID();
+  bool ValidatePathList();
 };
 
 }  // res_handler
diff --git a/src/res-copy/src/param_checker.cc b/src/res-copy/src/param_checker.cc
index d44ca85..72083a0 100644
--- a/src/res-copy/src/param_checker.cc
+++ b/src/res-copy/src/param_checker.cc
@@ -35,6 +35,22 @@
 namespace bs = boost::system;
 namespace bpo = boost::program_options;
 
+namespace {
+
+bool IsPathTraversal(const std::string& path) {
+  if (path.size() == 0)
+    return true;
+
+  if (path.find("..", 0) != std::string::npos) {
+    LOG(ERROR) << "Invalid path : " << path;
+    return false;
+  }
+
+  return true;
+}
+
+}  // namespace
+
 namespace res_handler {
 
 ParamChecker::ParamChecker(int argc, char* argv[]) {
@@ -110,12 +126,8 @@ bool ParamChecker::Validate() {
   if (!ValidatePkgID())
     return false;
 
-  if (req_type_ != ReqType::REQ_TYPE_UNINSTALL) {
-    if (path_info_list_.size() == 0) {
-      LOG(ERROR) << "Path is not given";
-      return false;
-    }
-  }
+  if (!ValidatePathList())
+    return false;
 
   return true;
 }
@@ -140,4 +152,21 @@ bool ParamChecker::ValidatePkgID() {
   return true;
 }
 
+bool ParamChecker::ValidatePathList() {
+  if (req_type_ != ReqType::REQ_TYPE_UNINSTALL) {
+    if (path_info_list_.size() == 0) {
+      LOG(ERROR) << "Path is not given";
+      return false;
+    }
+  }
+
+  for (auto& path_info : path_info_list_) {
+    if (!IsPathTraversal(path_info.GetSrcPath()) ||
+        !IsPathTraversal(path_info.GetDstPath()))
+      return false;
+  }
+
+  return true;
+}
+
 }  // namespace res_handler
diff --git a/tests/unit_tests/res-copy/src/test_param_checker.cc b/tests/unit_tests/res-copy/src/test_param_checker.cc
index 69163e9..b76b45d 100644
--- a/tests/unit_tests/res-copy/src/test_param_checker.cc
+++ b/tests/unit_tests/res-copy/src/test_param_checker.cc
@@ -45,10 +45,12 @@ class ParamCheckerTest : public TestFixture {
   virtual ~ParamCheckerTest() {}
 
   virtual void SetUp() {
-    EXPECT_CALL(GetMock<PkgMgrInfoMock>(), pkgmgrinfo_pkginfo_get_usr_pkginfo(_, _, _))
+    EXPECT_CALL(GetMock<PkgMgrInfoMock>(),
+            pkgmgrinfo_pkginfo_get_usr_pkginfo(_, _, _))
             .WillRepeatedly(Return(0));
-    EXPECT_CALL(GetMock<PkgMgrInfoMock>(), pkgmgrinfo_pkginfo_destroy_pkginfo(_))
-                .WillRepeatedly(Return(0)) ;
+    EXPECT_CALL(GetMock<PkgMgrInfoMock>(),
+            pkgmgrinfo_pkginfo_destroy_pkginfo(_))
+            .WillRepeatedly(Return(0)) ;
   }
   virtual void TearDown() {}
 };
@@ -70,8 +72,8 @@ TEST_F(ParamCheckerTest, PkgIDNotGiven) {
 }
 
 TEST_F(ParamCheckerTest, CopyRes) {
-  const char *argv[] = { "/bin/res-copy", "--uid", "5001", "-p", "srcpath", "dstpath",
-      "--copy", "org.test.targetpkgid", nullptr};
+  const char *argv[] = { "/bin/res-copy", "--uid", "5001",
+      "-p", "srcpath", "dstpath", "--copy", "org.test.targetpkgid", nullptr};
 
   ParamChecker checker(8, (char**)argv);
 
@@ -102,7 +104,8 @@ TEST_F(ParamCheckerTest, DeleteRes) {
   ParamChecker checker(5, (char**)argv);
 
   EXPECT_EQ(checker.Validate(), true);
-  EXPECT_EQ(checker.GetRequestType(), res_handler::ReqType::REQ_TYPE_UNINSTALL);
+  EXPECT_EQ(checker.GetRequestType(),
+      res_handler::ReqType::REQ_TYPE_UNINSTALL);
   EXPECT_EQ(checker.GetPkgID(), "org.test.targetpkgid");
   EXPECT_EQ(checker.GetPathList().size(), 0);
   EXPECT_EQ(checker.GetUID(), 5001);
@@ -131,3 +134,33 @@ TEST_F(ParamCheckerTest, PathNotGiven) {
 
   EXPECT_EQ(checker.Validate(), false);
 }
+
+TEST_F(ParamCheckerTest, CopyRes_SrcPathTraverseAttack) {
+  const char *argv[] = { "/bin/res-copy", "--uid",
+      "5001", "-p", "data/../../attackpath", "dstpath",
+      "--copy", "org.test.targetpkgid", nullptr};
+
+  ParamChecker checker(8, (char**)argv);
+
+  EXPECT_FALSE(checker.Validate());
+}
+
+TEST_F(ParamCheckerTest, CopyRes_DstPathTraverseAttack) {
+  const char *argv[] = { "/bin/res-copy", "--uid",
+      "5001", "-p", "data/normal_path", "../../attackpath",
+      "--copy", "org.test.targetpkgid", nullptr};
+
+  ParamChecker checker(8, (char**)argv);
+
+  EXPECT_FALSE(checker.Validate());
+}
+
+TEST_F(ParamCheckerTest, RemoveRes_PathTraverseAttack) {
+  const char *argv[] = { "/bin/res-copy", "--uid",
+      "5001", "-p", "../../../attackpath", "",
+      "--remove", "org.test.targetpkgid", nullptr};
+
+  ParamChecker checker(8, (char**)argv);
+
+  EXPECT_FALSE(checker.Validate());
+}