From: Zofia Abramowska Date: Tue, 8 Apr 2025 17:02:11 +0000 (+0200) Subject: security-manager: Fix segfaults in PolicyRequest X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=2a7de179b8ab2259be373cdee984ae9c18c0ee9a;p=platform%2Fcore%2Ftest%2Fsecurity-tests.git security-manager: Fix segfaults in PolicyRequest Change-Id: I1273c17991d4c32d5585740b2386d5e04b4e086f --- diff --git a/src/common/sm_api.cpp b/src/common/sm_api.cpp index 478e0c6a..d9ed32c2 100644 --- a/src/common/sm_api.cpp +++ b/src/common/sm_api.cpp @@ -170,10 +170,21 @@ void getConfiguredPolicy(const PolicyEntry &filter, std::vector &po size_t policy_size = 0; int result; + policy_entry *entry_p; + result = security_manager_policy_entry_new(&entry_p); + RUNNER_ASSERT_MSG((lib_retcode)result == SECURITY_MANAGER_SUCCESS, + "creation of new policy entry failed. Result: " << result); + RUNNER_ASSERT_MSG(entry_p != nullptr, "creation of new policy entry did not allocate memory"); + + auto entryPtr = std::unique_ptr + {entry_p, security_manager_policy_entry_free}; + + filter.fill(entry_p); + if (forAdmin) { - result = security_manager_get_configured_policy_for_admin(filter.get(), &pp_privs_policy, &policy_size); + result = security_manager_get_configured_policy_for_admin(entry_p, &pp_privs_policy, &policy_size); } else { - result = security_manager_get_configured_policy_for_self(filter.get(), &pp_privs_policy, &policy_size); + result = security_manager_get_configured_policy_for_self(entry_p, &pp_privs_policy, &policy_size); }; RUNNER_ASSERT_MSG((lib_retcode)result == expectedResult, @@ -181,7 +192,8 @@ void getConfiguredPolicy(const PolicyEntry &filter, std::vector &po << " Result: " << result << ";"); for (unsigned int i = 0; i < policy_size; ++i) { - PolicyEntry pe(*pp_privs_policy[i]); + + PolicyEntry pe(pp_privs_policy[i]); policyEntries.push_back(pe); }; } @@ -192,12 +204,23 @@ void getPolicy(const PolicyEntry &filter, std::vector &policyEntrie size_t policy_size = 0; int result; - result = security_manager_get_policy(filter.get(), &pp_privs_policy, &policy_size); + policy_entry *entry_p; + result = security_manager_policy_entry_new(&entry_p); + RUNNER_ASSERT_MSG((lib_retcode)result == SECURITY_MANAGER_SUCCESS, + "creation of new policy entry failed. Result: " << result); + RUNNER_ASSERT_MSG(entry_p != nullptr, "creation of new policy entry did not allocate memory"); + + auto entryPtr = std::unique_ptr + {entry_p, security_manager_policy_entry_free}; + + filter.fill(entry_p); + + result = security_manager_get_policy(entry_p, &pp_privs_policy, &policy_size); RUNNER_ASSERT_MSG((lib_retcode)result == expectedResult, "Unexpected result" << std::endl << " Result: " << result << ";"); for (unsigned int i = 0; i < policy_size; ++i) { - PolicyEntry pe(*pp_privs_policy[i]); + PolicyEntry pe(pp_privs_policy[i]); policyEntries.push_back(pe); }; } diff --git a/src/common/sm_policy_request.cpp b/src/common/sm_policy_request.cpp index c129a76a..fac911c2 100644 --- a/src/common/sm_policy_request.cpp +++ b/src/common/sm_policy_request.cpp @@ -14,6 +14,8 @@ * limitations under the License. */ +#include +#include #include #include @@ -25,128 +27,102 @@ constexpr char PolicyEntry::LEVEL_DENY[]; constexpr char PolicyEntry::LEVEL_ASK_USER[]; PolicyEntry::PolicyEntry() - : m_appId(true, std::string(SECURITY_MANAGER_ANY)) - , m_user(true, std::string(SECURITY_MANAGER_ANY)) - , m_privilege(true, std::string(SECURITY_MANAGER_ANY)) - , m_currentLevel(false, std::string("")) - , m_maxLevel(false, std::string("")) -{ - int result = security_manager_policy_entry_new(&m_entry); - RUNNER_ASSERT_MSG((lib_retcode)result == SECURITY_MANAGER_SUCCESS, - "creation of new policy entry failed. Result: " << result); - RUNNER_ASSERT_MSG(m_entry != nullptr, "creation of new policy entry did not allocate memory"); - - security_manager_policy_entry_set_application(m_entry, m_appId.second.c_str()); - security_manager_policy_entry_set_user(m_entry, m_user.second.c_str()); - security_manager_policy_entry_set_privilege(m_entry, m_privilege.second.c_str()); -} + : m_appId(std::string(SECURITY_MANAGER_ANY)) + , m_user(std::string(SECURITY_MANAGER_ANY)) + , m_privilege(std::string(SECURITY_MANAGER_ANY)) +{} PolicyEntry::PolicyEntry(const std::string &appId, const std::string &user, const std::string &privilege) - : m_appId(true, std::string(appId)) - , m_user(true, std::string(user)) - , m_privilege(true, std::string(privilege)) - , m_currentLevel(false, std::string("")) - , m_maxLevel(false, std::string("")) + : m_appId(appId) + , m_user(user) + , m_privilege(privilege) +{} + +PolicyEntry::PolicyEntry(policy_entry *entry) { - int result = security_manager_policy_entry_new(&m_entry); - RUNNER_ASSERT_MSG((lib_retcode)result == SECURITY_MANAGER_SUCCESS, - "creation of new policy entry failed. Result: " << result); - RUNNER_ASSERT_MSG(m_entry != nullptr, "creation of new policy entry did not allocate memory"); + constexpr static auto cstr_to_opt = [](const char* cstr) { + return cstr ? std::optional{cstr} : std::nullopt; + }; - security_manager_policy_entry_set_user(m_entry, m_user.second.c_str()); - security_manager_policy_entry_set_application(m_entry, m_appId.second.c_str()); - security_manager_policy_entry_set_privilege(m_entry, m_privilege.second.c_str()); -} -PolicyEntry::PolicyEntry(policy_entry &entry): m_entry(&entry) -{ - m_appId.first = true; - m_appId.second = std::string(security_manager_policy_entry_get_application(m_entry)); + m_appId = cstr_to_opt(security_manager_policy_entry_get_application(entry)); - m_user.first = true; - m_user.second = std::string(security_manager_policy_entry_get_user(m_entry)); + m_user = cstr_to_opt(security_manager_policy_entry_get_user(entry)); - m_privilege.first = true; - m_privilege.second = std::string(security_manager_policy_entry_get_privilege(m_entry)); + m_privilege = cstr_to_opt(security_manager_policy_entry_get_privilege(entry)); - m_currentLevel.first = true; - m_currentLevel.second = std::string(security_manager_policy_entry_get_level(m_entry)); + m_currentLevel = cstr_to_opt(security_manager_policy_entry_get_level(entry)); - m_maxLevel.first = true; - m_maxLevel.second = std::string(security_manager_policy_entry_get_max_level(m_entry)); + m_maxLevel = cstr_to_opt(security_manager_policy_entry_get_max_level(entry)); }; void PolicyEntry::setLevel(const std::string &level) { - m_currentLevel.first = true; - m_currentLevel.second = level; - security_manager_policy_entry_set_level(m_entry, level.c_str()); - m_maxLevel.first = true; - m_maxLevel.second = std::string(security_manager_policy_entry_get_max_level(m_entry)); + m_currentLevel = level; }; void PolicyEntry::setMaxLevel(const std::string &level) { - m_maxLevel.first = true; - m_maxLevel.second = level; - security_manager_policy_entry_admin_set_level(m_entry, level.c_str()); - m_currentLevel.first = true; - m_currentLevel.second = std::string(security_manager_policy_entry_get_level(m_entry)); + m_maxLevel = level; }; +void PolicyEntry::fill(policy_entry *entry_p) const +{ + if (m_appId) { + security_manager_policy_entry_set_application(entry_p, m_appId->c_str()); + } + if (m_user) { + security_manager_policy_entry_set_user(entry_p, m_user->c_str()); + } + if (m_privilege) { + security_manager_policy_entry_set_privilege(entry_p, m_privilege->c_str()); + } + if (m_currentLevel) { + security_manager_policy_entry_set_level(entry_p, m_currentLevel->c_str()); + } + if (m_maxLevel) { + security_manager_policy_entry_admin_set_level(entry_p, m_maxLevel->c_str()); + } +} std::ostream& operator<<(std::ostream &os, const PolicyEntry &request) { - if (request.m_appId.first) - os << "appId: " << request.m_appId.second << "; "; + if (request.m_appId) + os << "appId: " << *request.m_appId << "; "; - if (request.m_user.first) - os << "user: " << request.m_user.second << "; "; + if (request.m_user) + os << "user: " << *request.m_user << "; "; - if (request.m_privilege.first) - os << "privilege: " << request.m_privilege.second << "; "; + if (request.m_privilege) + os << "privilege: " << *request.m_privilege << "; "; - if (request.m_currentLevel.first) - os << "current: " << request.m_currentLevel.second << "; "; + if (request.m_currentLevel) + os << "current: " << *request.m_currentLevel << "; "; - if (request.m_maxLevel.first) - os << "max: " << request.m_maxLevel.second << "; "; + if (request.m_maxLevel) + os << "max: " << *request.m_maxLevel << "; "; return os; } -PolicyEntry::~PolicyEntry() -{ -} - -void PolicyEntry::free(void) -{ - security_manager_policy_entry_free(m_entry); -} - bool PolicyEntry::operator==(const PolicyEntry &other) const { - auto cmp = [](const std::pair &a, const std::pair &b)->bool - { - return (a.first) ? (b.first && a.second == b.second) : !b.first; - }; - - return ( - cmp(m_appId, other.m_appId) && - cmp(m_user, other.m_user) && - cmp(m_privilege, other.m_privilege) && - cmp(m_currentLevel, other.m_currentLevel) && - cmp(m_maxLevel, other.m_maxLevel)); + return + m_appId == other.m_appId && + m_user == other.m_user && + m_privilege == other.m_privilege && + m_currentLevel == other.m_currentLevel && + m_maxLevel == other.m_maxLevel; } -std::string PolicyEntry::toString() const +std::string PolicyEntry::toHashKey() const { std::stringstream ss; - auto append = [&](const std::pair &x) + auto append = [&](const std::optional &x) { - if (x.first) - ss << x.second; + if (x) + ss << *x; ss << '\0'; }; @@ -171,18 +147,27 @@ PolicyRequest::PolicyRequest() PolicyRequest::~PolicyRequest() { - for(std::vector::iterator it = m_entries.begin(); it != m_entries.end(); ++it) { - it->free(); + for (auto entry_p: m_entries) { + security_manager_policy_entry_free(entry_p); } security_manager_policy_update_req_free(m_req); } void PolicyRequest::addEntry(PolicyEntry &entry, - lib_retcode expectedResult) + lib_retcode expectedResult) { - int result = 0; + policy_entry *entry_p; + int result = security_manager_policy_entry_new(&entry_p); + RUNNER_ASSERT_MSG((lib_retcode)result == SECURITY_MANAGER_SUCCESS, + "creation of new policy entry failed. Result: " << result); + RUNNER_ASSERT_MSG(entry_p != nullptr, "creation of new policy entry did not allocate memory"); + + auto entryPtr = std::unique_ptr + {entry_p, security_manager_policy_entry_free}; + + entry.fill(entry_p); - result = security_manager_policy_update_req_add_entry(m_req, entry.get()); + result = security_manager_policy_update_req_add_entry(m_req, entry_p); RUNNER_ASSERT_MSG((lib_retcode)result == expectedResult, "adding policy entry to request returned wrong value." @@ -190,7 +175,8 @@ void PolicyRequest::addEntry(PolicyEntry &entry, << " Result: " << result << ";" << " Expected result: " << expectedResult); - m_entries.push_back(entry); + m_entries.push_back(entry_p); + entryPtr.release(); } std::ostream& operator<<(std::ostream &os, const PolicyRequest &request) @@ -200,7 +186,7 @@ std::ostream& operator<<(std::ostream &os, const PolicyRequest &request) os << "PolicyRequest m_entries size: " << request.m_entries.size() << "; "; for(unsigned int i = 0; i != request.m_entries.size(); i++) { - os << "entry " << i << ": " << request.m_entries[i] << "; "; + os << "entry " << i << ": " << PolicyEntry(request.m_entries[i]) << "; "; } } diff --git a/src/common/sm_policy_request.h b/src/common/sm_policy_request.h index d1791a7d..77d23236 100644 --- a/src/common/sm_policy_request.h +++ b/src/common/sm_policy_request.h @@ -18,6 +18,7 @@ #define SECURITY_MANAGER_TEST_POLICYREQUEST #include +#include #include #include #include @@ -39,31 +40,34 @@ public: const std::string &user, const std::string &privilege ); - ~PolicyEntry(); - - PolicyEntry(policy_entry &entry); - - policy_entry *get() const { return m_entry; } - std::string getUser() const { return m_user.second; } - std::string getAppId() const { return m_appId.second; } - std::string getPrivilege() const { return m_privilege.second; } - std::string getCurrentLevel() const { return m_currentLevel.second; } - std::string getMaxLevel() const { return m_maxLevel.second; } + ~PolicyEntry() = default; + + PolicyEntry(policy_entry *entry); + + bool hasUser() const { return m_user.has_value(); } + bool hasAppId() const { return m_appId.has_value(); } + bool hasPrivilege() const { return m_privilege.has_value(); } + bool hasCurrentLevel() const { return m_currentLevel.has_value(); } + bool hasMaxLevel() const { return m_maxLevel.has_value(); } + std::string getUser() const { return m_user.value_or(""); } + std::string getAppId() const { return m_appId.value_or(""); } + std::string getPrivilege() const { return m_privilege.value_or(""); } + std::string getCurrentLevel() const { return m_currentLevel.value_or(""); } + std::string getMaxLevel() const { return m_maxLevel.value_or(""); } void setLevel(const std::string &level); void setMaxLevel(const std::string &level); - void free(void); + void fill(policy_entry *entry) const; friend std::ostream& operator<<(std::ostream &, const PolicyEntry&); bool operator==(const PolicyEntry &) const; - std::string toString() const; + std::string toHashKey() const; private: - policy_entry *m_entry; - std::pair m_appId; - std::pair m_user; - std::pair m_privilege; - std::pair m_currentLevel; - std::pair m_maxLevel; + std::optional m_appId; + std::optional m_user; + std::optional m_privilege; + std::optional m_currentLevel; + std::optional m_maxLevel; }; std::ostream& operator<<(std::ostream &os, const SecurityManagerTest::PolicyEntry &request); @@ -83,7 +87,7 @@ public: private: policy_update_req *m_req; - std::vector m_entries; + std::vector m_entries; }; std::ostream& operator<<(std::ostream &os, const SecurityManagerTest::PolicyRequest &request); @@ -94,7 +98,9 @@ namespace std { template<> struct hash { - size_t operator()(const SecurityManagerTest::PolicyEntry &x) const { return hash()(x.toString()); } + size_t operator()(const SecurityManagerTest::PolicyEntry &x) const { + return hash()(x.toHashKey()); + } }; } // namespace std