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 12529ef..2d7343f 100644 (file)
--- 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 c622a12..697c1e9 100644 (file)
--- 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);