From: Tomasz Swierczek <t.swierczek@samsung.com>
Date: Tue, 30 May 2023 11:46:12 +0000 (+0200)
Subject: Abort app candidate process in case of wrong setup
X-Git-Tag: accepted/tizen/7.0/unified/20230602.171936~1
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=627b6768bcdf953bde0c06375dbae314af402720;p=platform%2Fcore%2Fsecurity%2Fsecurity-manager.git

Abort app candidate process in case of wrong setup

When offending thread with higher privileges is detected,
new error log is added and security-manager-client library
forces entire app candidate process to abort.

This will effectively block possibility of privilege escalation
if a new thread was spawned ie. by Chromium during prepare_app call.

Abort will also generate coredump, making it easier to debug
the source of offending thread.

Change-Id: I16772d0e51aa112548acb64f7b82ccf87948ded9
---

diff --git a/src/client/check-proper-drop.cpp b/src/client/check-proper-drop.cpp
index 12529ef0..2d7343f7 100644
--- a/src/client/check-proper-drop.cpp
+++ b/src/client/check-proper-drop.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016-2020 Samsung Electronics Co., Ltd. All rights reserved.
+ * Copyright (c) 2016-2023 Samsung Electronics Co., Ltd. All rights reserved.
  *
  * This file is licensed under the terms of MIT License or the Apache License
  * Version 2.0 of your choice. See the LICENSE.MIT file for MIT license details.
@@ -118,9 +118,10 @@ class TaskData final {
     }
 
 public:
-    TaskData(const ::std::string &taskId, BitFlags checkProperDropFlags)
-    : m_label(SmackLabels::getSmackLabelFromPid(::std::stoul(taskId)))
-    {
+    TaskData() {}
+
+    void initialize(const ::std::string &taskId, BitFlags checkProperDropFlags) {
+        m_label = SmackLabels::getSmackLabelFromPid(::std::stoul(taskId));
         ::std::string taskRoot = "/proc/self/task/" + taskId + "/";
         fillStatus(taskRoot);
         fillNs(taskRoot, checkProperDropFlags);
@@ -130,6 +131,13 @@ public:
         checkCaps(m_capEff);
     }
 
+
+    TaskData(const ::std::string &taskId, BitFlags checkProperDropFlags)
+    {
+        initialize(taskId, checkProperDropFlags);
+    }
+
+
     void checkSameAs(const TaskData &other) const {
         checkSame(m_label, other.m_label, "label");
         checkSame(m_uid, other.m_uid, "uid");
@@ -150,11 +158,23 @@ void checkThreads(BitFlags checkProperDropFlags)
     if (taskIds.empty())
         ThrowMsg(DropError, "no tasks found");
 
-    TaskData lastTaskData(taskIds.back(), checkProperDropFlags);
+    TaskData lastTaskData;
+    try {
+        lastTaskData.initialize(taskIds.back(), checkProperDropFlags);
+    } catch (...) {
+        LogError("Offending taskId is: " << taskIds.back());
+        throw;
+    }
+
     taskIds.pop_back();
 
     for (const auto &taskId : taskIds)
-        TaskData(taskId, checkProperDropFlags).checkSameAs(lastTaskData);
+        try {
+            TaskData(taskId, checkProperDropFlags).checkSameAs(lastTaskData);
+        } catch (...) {
+            LogError("Offending taskId is: " << taskId);
+            throw;
+        }
 }
 
 }  // namespace CheckProperDrop
diff --git a/src/client/client-security-manager.cpp b/src/client/client-security-manager.cpp
index c622a123..697c1e91 100644
--- a/src/client/client-security-manager.cpp
+++ b/src/client/client-security-manager.cpp
@@ -1021,7 +1021,17 @@ int security_manager_prepare_app2(const char *app_name, const char *subsession_i
             CheckProperDrop::checkThreads(prepareAppFlags >> PREPARE_APP_CPD_FLAG_SHIFT);
         } catch (...) {
             LogError("Privileges haven't been properly dropped for the whole process of application " << app_name);
-            throw;
+            /*
+             * Not all app candidate processes behave properly and some may want to spawn
+             * new threads during this API call. This abort() below makes sure the process will:
+             *
+             * 1) not allow actual applicaton run in environment where there's a thread that still has high privileges
+             * 2) make sure a coredump will be created so that it can be analyzed later which thread caused troubles
+             *
+             * This will effectively block possible privilege escalation in application due to improper setup of
+             * the candidate process.
+             */
+            abort();
         }
 
         LogWarning("security_manager_prepare_app2() finished with return code " << ret);