Implement better tryCatch 97/275097/9
authorTomasz Swierczek <t.swierczek@samsung.com>
Mon, 16 May 2022 12:31:44 +0000 (14:31 +0200)
committerTomasz Swierczek <t.swierczek@samsung.com>
Wed, 22 Jun 2022 12:17:13 +0000 (14:17 +0200)
     "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

src/common/CMakeLists.txt
src/common/exceptions/TryCatch.cpp [deleted file]
src/common/exceptions/TryCatch.h
src/helpers/creds-commons/CMakeLists.txt
test/CMakeLists.txt

index 2408612ef494e442e72d4ee0b02f0e19b64ad3bb..10e4790eafededcb63f04310ef6c8b34148f58fe 100644 (file)
@@ -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 (file)
index 947d439..0000000
+++ /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 <m.niesluchow@samsung.com>
- * @author      Oskar Świtalski <o.switalski@samsung.com>
- * @version     1.0
- * @brief       This file contains an implementation of a function for catching exceptions
- */
-
-#include <cxxabi.h>
-#include <exception>
-#include <new>
-
-#include <exceptions/AccessDeniedException.h>
-#include <exceptions/InvalidProtocolException.h>
-#include <exceptions/NoMemoryException.h>
-#include <log/log.h>
-
-#include <cynara-error.h>
-#include "TryCatch.h"
-
-namespace Cynara {
-
-int tryCatch(const std::function<int(void)> &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
-
index e6cee4bb3486893b8d7adb348b36837cf4712689..8eac4e1f351f93778b68b374f2b80c4b63bd79e5 100644 (file)
@@ -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.
  */
 /**
  * @file        src/common/exceptions/TryCatch.h
- * @author      Marcin Niesluchowski <m.niesluchow@samsung.com>
- * @author      Oskar Świtalski <o.switalski@samsung.com>
- * @version     1.0
+ * @author      Tomasz Swierczek <t.swierczek@samsung.com>
+ * @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 <cxxabi.h>
+#include <exception>
 #include <functional>
+#include <new>
+
+#include <exceptions/AccessDeniedException.h>
+#include <exceptions/InvalidProtocolException.h>
+#include <exceptions/NoMemoryException.h>
+#include <log/log.h>
+
+#include <cynara-error.h>
 
 namespace Cynara {
 
-int tryCatch(const std::function<int(void)> &func);
+template <class F>
+int tryCatch(F &&f) {
+    try {
+        return std::forward<F>(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
 
index f4c89ace6f19b80b7dc68b7e88bd120d5a52ed2e..2869250934bc9cd7b143afd40e9050676c9affe1 100644 (file)
@@ -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
     )
index 38ac4c72253b3eb051a160adb98d98af32ac6737..06f4ddbe8eda58964d32a76e78f6337bcc259d56 100644 (file)
@@ -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