From: Tomasz Swierczek Date: Mon, 16 May 2022 12:31:44 +0000 (+0200) Subject: Implement better tryCatch X-Git-Tag: accepted/tizen/7.0/unified/20221129.172448~2 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=ab8fa8b55de924f787bcb8e9172efcedc1cbb804;p=platform%2Fcore%2Fsecurity%2Fcynara.git Implement better tryCatch "Gotta catch them all!" - from Pokémon Recently it was reported that there's an issue if not-handled std::bad_alloc in Cynara. After getting more information (incl. decompiled version of the module), it was found that when compiled using gcc, implementation of std::function made on-the fly as argument to tryCatch() is being allocated BEFORE passing it to the tryCatch() function. When compiled using clang9, there is no new() in that place. This patch changes tryCatch() to a template with function being an R-value reference with std::forward, solving the issue. Decompiled version with additional new (thanks to Konstiantyn Melnik, k.melnik@samsung.com): int __cdecl cynara_check(void *cynara_struct, char *client, char *client_session, char *user, char *privilege) { int v5; // ebx char ***lambda_func_obj; // eax char *__loc_privilege_ptr; // [esp+1Ch] [ebp-4Ch] char *__loc_user_str; // [esp+20h] [ebp-48h] char *__loc_client_session_str; // [esp+24h] [ebp-44h] char *__loc_client_str; // [esp+28h] [ebp-40h] void *__loc_cynara_struct; // [esp+2Ch] [ebp-3Ch] char ***lambda_func_ptr; // [esp+3Ch] [ebp-2Ch] int (__cdecl *v14)(void **, void **, unsigned int); // [esp+44h] [ebp-24h] int (__cdecl *func_to_run)(void *); // [esp+48h] [ebp-20h] unsigned int guard_var; // [esp+4Ch] [ebp-1Ch] __loc_privilege_ptr = privilege; __loc_cynara_struct = cynara_struct; __loc_user_str = user; __loc_client_str = client; __loc_client_session_str = client_session; guard_var = __readgsdword(0x14u); if ( cynara_struct && *(_DWORD *)cynara_struct // if (!p_cynara || !p_cynara->impl) // return CYNARA_API_INVALID_PARAM; && client // if (!isStringValid(client) || !isStringValid(client_session)) // return CYNARA_API_INVALID_PARAM; && strlen(client) <= 0x1000 && client_session && strlen(client_session) <= 0x1000 && user && strlen(user) <= 0x1000 // if (!isStringValid(user) || !isStringValid(privilege)) // return CYNARA_API_INVALID_PARAM; && privilege && strlen(privilege) <= 0x1000 ) { lambda_func_obj = (char ***)operator new(0x14u); lambda_func_ptr = lambda_func_obj; *lambda_func_obj = &__loc_client_str; lambda_func_obj[1] = &__loc_client_session_str; lambda_func_obj[2] = &__loc_user_str; lambda_func_obj[3] = &__loc_privilege_ptr; lambda_func_obj[4] = (char **)&__loc_cynara_struct; func_to_run = (int (__cdecl *)(void *))sub_4B80; v14 = sub_3930; v5 = Cynara::tryCatch(&lambda_func_ptr); if ( v14 ) v14((void **)&lambda_func_ptr, (void **)&lambda_func_ptr, 3u); } else { v5 = -4; } if ( __readgsdword(0x14u) != guard_var ) terminate_proc(); return v5; } Change-Id: I5455d03d411b03ed76a81659efb60c6474ceb99b --- diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index 2408612e..10e4790e 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -35,7 +35,6 @@ SET(COMMON_SOURCES ${COMMON_PATH}/containers/BinaryQueue.cpp ${COMMON_PATH}/error/api.cpp ${COMMON_PATH}/error/SafeStrError.cpp - ${COMMON_PATH}/exceptions/TryCatch.cpp ${COMMON_PATH}/lock/FileLock.cpp ${COMMON_PATH}/log/AuditLog.cpp ${COMMON_PATH}/log/log.cpp diff --git a/src/common/exceptions/TryCatch.cpp b/src/common/exceptions/TryCatch.cpp deleted file mode 100644 index 947d4398..00000000 --- a/src/common/exceptions/TryCatch.cpp +++ /dev/null @@ -1,75 +0,0 @@ -/* - * Copyright (c) 2020 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. - * See the LICENSE file or the notice below for Apache License Version 2.0 - * details. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License - */ -/** - * @file src/common/exceptions/TryCatch.cpp - * @author Marcin Niesluchowski - * @author Oskar Świtalski - * @version 1.0 - * @brief This file contains an implementation of a function for catching exceptions - */ - -#include -#include -#include - -#include -#include -#include -#include - -#include -#include "TryCatch.h" - -namespace Cynara { - -int tryCatch(const std::function &func) { - try { - return func(); - } catch (const std::bad_alloc &e) { - LOGE_NOTHROW("%s", e.what()); - return CYNARA_API_OUT_OF_MEMORY; - } catch (const NoMemoryException &e) { - LOGE_NOTHROW("%s", e.what()); - return CYNARA_API_OUT_OF_MEMORY; - } catch (const InvalidProtocolException &e) { - LOGE_NOTHROW("%s", e.what()); - return CYNARA_API_INVALID_PARAM; - } catch (const AccessDeniedException &e) { - LOGE_NOTHROW("%s", e.what()); - return CYNARA_API_PERMISSION_DENIED; - } catch (const std::exception &e) { - LOGE_NOTHROW("%s", e.what()); - return CYNARA_API_UNKNOWN_ERROR; - } catch (const abi::__forced_unwind &) { - /** - * workaround for pthread_cancel, which cancels thread using this exception - * and expects it to be rethrown in every try-catch - */ - LOGD_NOTHROW("We are 'gracefully' stopped by pthread_cancel"); - throw; - } catch (...) { - LOGE_NOTHROW("Unexpected exception"); - return CYNARA_API_UNKNOWN_ERROR; - } -} - -} // namespace Cynara - diff --git a/src/common/exceptions/TryCatch.h b/src/common/exceptions/TryCatch.h index e6cee4bb..8eac4e1f 100644 --- a/src/common/exceptions/TryCatch.h +++ b/src/common/exceptions/TryCatch.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014-2020 Samsung Electronics Co., Ltd. All rights reserved. + * Copyright (c) 2014-2022 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. @@ -20,20 +20,60 @@ */ /** * @file src/common/exceptions/TryCatch.h - * @author Marcin Niesluchowski - * @author Oskar Świtalski - * @version 1.0 + * @author Tomasz Swierczek + * @version 2.0 * @brief This file contains a declaration of a function for catching exceptions */ #ifndef SRC_COMMON_EXCEPTIONS_TRYCATCH_H_ #define SRC_COMMON_EXCEPTIONS_TRYCATCH_H_ + +#include +#include #include +#include + +#include +#include +#include +#include + +#include namespace Cynara { -int tryCatch(const std::function &func); +template +int tryCatch(F &&f) { + try { + return std::forward(f)(); + } catch (const std::bad_alloc &e) { + LOGE_NOTHROW("%s", e.what()); + return CYNARA_API_OUT_OF_MEMORY; + } catch (const NoMemoryException &e) { + LOGE_NOTHROW("%s", e.what()); + return CYNARA_API_OUT_OF_MEMORY; + } catch (const InvalidProtocolException &e) { + LOGE_NOTHROW("%s", e.what()); + return CYNARA_API_INVALID_PARAM; + } catch (const AccessDeniedException &e) { + LOGE_NOTHROW("%s", e.what()); + return CYNARA_API_PERMISSION_DENIED; + } catch (const std::exception &e) { + LOGE_NOTHROW("%s", e.what()); + return CYNARA_API_UNKNOWN_ERROR; + } catch (const abi::__forced_unwind &) { + /** + * workaround for pthread_cancel, which cancels thread using this exception + * and expects it to be rethrown in every try-catch + */ + LOGD_NOTHROW("We are 'gracefully' stopped by pthread_cancel"); + throw; + } catch (...) { + LOGE_NOTHROW("Unexpected exception"); + return CYNARA_API_UNKNOWN_ERROR; + } +} } // namespace Cynara diff --git a/src/helpers/creds-commons/CMakeLists.txt b/src/helpers/creds-commons/CMakeLists.txt index f4c89ace..28692509 100644 --- a/src/helpers/creds-commons/CMakeLists.txt +++ b/src/helpers/creds-commons/CMakeLists.txt @@ -29,7 +29,6 @@ SET(LIB_CREDS_COMMONS_VERSION ${LIB_CREDS_COMMONS_VERSION_MAJOR}.17.1) SET(LIB_CREDS_COMMONS_PATH ${CYNARA_PATH}/helpers/creds-commons) SET(LIB_CREDS_COMMONS_SOURCES - ${CYNARA_PATH}/common/exceptions/TryCatch.cpp ${LIB_CREDS_COMMONS_PATH}/creds-commons.cpp ${LIB_CREDS_COMMONS_PATH}/CredsCommonsInner.cpp ) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 38ac4c72..06f4ddbe 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -84,7 +84,6 @@ SET(CYNARA_SOURCES_FOR_TESTS ${CYNARA_SRC}/common/config/PathConfig.cpp ${CYNARA_SRC}/common/containers/BinaryQueue.cpp ${CYNARA_SRC}/common/error/SafeStrError.cpp - ${CYNARA_SRC}/common/exceptions/TryCatch.cpp ${CYNARA_SRC}/common/protocol/ProtocolAdmin.cpp ${CYNARA_SRC}/common/protocol/ProtocolAgent.cpp ${CYNARA_SRC}/common/protocol/ProtocolClient.cpp