Fixed curlholder Double Free

cmake/sanitizer.cmake

@@ -1,67 +1,69 @@
-include(CheckCXXSourceRuns)
+include(CheckCXXSourceCompiles)
 
-set(ALL_SAN_FLAGS "")
+set(ALL_ACTIVE_SAN_FLAGS "")
 
  # No sanitizers when cross compiling to prevent stuff like this: https://github.com/whoshuu/cpr/issues/582
 if(NOT CMAKE_CROSSCOMPILING)
-    # Thread sanitizer
-    if(CPR_DEBUG_SANITIZER_FLAG_THREAD)
-        set(THREAD_SAN_FLAGS "-fsanitize=thread")
+    function(cpr_check_sanitizer_compile flags result_var)
         set(PREV_FLAG ${CMAKE_REQUIRED_FLAGS})
-        set(CMAKE_REQUIRED_FLAGS "${THREAD_SAN_FLAGS}")
-        check_cxx_source_runs("int main() { return 0; }" THREAD_SANITIZER_AVAILABLE_AND_ENABLED)
+        set(CMAKE_REQUIRED_FLAGS "${flags}")
+        check_cxx_source_compiles("int main() { return 0; }" ${result_var})
         set(CMAKE_REQUIRED_FLAGS ${PREV_FLAG})
-        # Do not add the ThreadSanitizer for builds with all sanitizers enabled because it is incompatible with other sanitizers.
+    endfunction()
+
+    # Thread sanitizer
+    set(THREAD_SAN_FLAGS "-fsanitize=thread")
+    if(CPR_DEBUG_SANITIZER_FLAG_THREAD)
+        cpr_check_sanitizer_compile("${THREAD_SAN_FLAGS}" THREAD_SANITIZER_AVAILABLE_AND_ENABLED)
+        if(NOT THREAD_SANITIZER_AVAILABLE_AND_ENABLED)
+            message(FATAL_ERROR "ThreadSanitizer requested but the test program failed to compile with ${THREAD_SAN_FLAGS}.")
+        endif()
     endif()
 
     # Address sanitizer
+    set(ADDR_SAN_FLAGS "-fsanitize=address")
     if(CPR_DEBUG_SANITIZER_FLAG_ADDR)
-        set(ADDR_SAN_FLAGS "-fsanitize=address")
-        set(PREV_FLAG ${CMAKE_REQUIRED_FLAGS})
-        set(CMAKE_REQUIRED_FLAGS "${ADDR_SAN_FLAGS}")
-        check_cxx_source_runs("int main() { return 0; }" ADDRESS_SANITIZER_AVAILABLE_AND_ENABLED)
-        set(CMAKE_REQUIRED_FLAGS ${PREV_FLAG})
-        if(ADDRESS_SANITIZER_AVAILABLE_AND_ENABLED)
-            set(ALL_SAN_FLAGS "${ALL_SAN_FLAGS} ${ADDR_SAN_FLAGS}")
+        cpr_check_sanitizer_compile("${ADDR_SAN_FLAGS}" ADDRESS_SANITIZER_AVAILABLE_AND_ENABLED)
+        if(NOT ADDRESS_SANITIZER_AVAILABLE_AND_ENABLED)
+            message(FATAL_ERROR "AddressSanitizer requested but the test program failed to compile with ${ADDR_SAN_FLAGS}.")
         endif()
     endif()
 
     # Leak sanitizer
+    set(LEAK_SAN_FLAGS "-fsanitize=leak")
     if(CPR_DEBUG_SANITIZER_FLAG_LEAK)
-        set(LEAK_SAN_FLAGS "-fsanitize=leak")
-        set(PREV_FLAG ${CMAKE_REQUIRED_FLAGS})
-        set(CMAKE_REQUIRED_FLAGS "${LEAK_SAN_FLAGS}")
-        check_cxx_source_runs("int main() { return 0; }" LEAK_SANITIZER_AVAILABLE_AND_ENABLED)
-        set(CMAKE_REQUIRED_FLAGS ${PREV_FLAG})
-        if(LEAK_SANITIZER_AVAILABLE_AND_ENABLED)
-            set(ALL_SAN_FLAGS "${ALL_SAN_FLAGS} ${LEAK_SAN_FLAGS}")
+        cpr_check_sanitizer_compile("${LEAK_SAN_FLAGS}" LEAK_SANITIZER_AVAILABLE_AND_ENABLED)
+        if(NOT LEAK_SANITIZER_AVAILABLE_AND_ENABLED)
+            message(FATAL_ERROR "LeakSanitizer requested but the test program failed to compile with ${LEAK_SAN_FLAGS}.")
         endif()
     endif()
 
     # Undefined behavior sanitizer
+    set(UDEF_SAN_FLAGS "-fsanitize=undefined")
     if(CPR_DEBUG_SANITIZER_FLAG_UB)
-        set(UDEF_SAN_FLAGS "-fsanitize=undefined")
-        set(PREV_FLAG ${CMAKE_REQUIRED_FLAGS})
-        set(CMAKE_REQUIRED_FLAGS "${UDEF_SAN_FLAGS}")
-        check_cxx_source_runs("int main() { return 0; }" UNDEFINED_BEHAVIOR_SANITIZER_AVAILABLE_AND_ENABLED)
-        set(CMAKE_REQUIRED_FLAGS ${PREV_FLAG})
-        if(UNDEFINED_BEHAVIOR_SANITIZER_AVAILABLE_AND_ENABLED)
-            set(ALL_SAN_FLAGS "${ALL_SAN_FLAGS} ${UDEF_SAN_FLAGS}")
+        cpr_check_sanitizer_compile("${UDEF_SAN_FLAGS}" UNDEFINED_BEHAVIOR_SANITIZER_AVAILABLE_AND_ENABLED)
+        if(NOT UNDEFINED_BEHAVIOR_SANITIZER_AVAILABLE_AND_ENABLED)
+            message(FATAL_ERROR "UndefinedBehaviorSanitizer requested but the test program failed to compile with ${UDEF_SAN_FLAGS}.")
         endif()
     endif()
 
     # All sanitizer (without thread sanitizer)
-    if(CPR_DEBUG_SANITIZER_FLAG_ALL AND NOT ALL_SAN_FLAGS STREQUAL "")
-        set(PREV_FLAG ${CMAKE_REQUIRED_FLAGS})
-        set(CMAKE_REQUIRED_FLAGS "${ALL_SAN_FLAGS}")
-        check_cxx_source_runs("int main() { return 0; }" ALL_SANITIZERS_AVAILABLE_AND_ENABLED)
-        set(CMAKE_REQUIRED_FLAGS ${PREV_FLAG})
+    if(CPR_DEBUG_SANITIZER_FLAG_ALL)
+        cpr_check_sanitizer_compile("${ADDR_SAN_FLAGS} ${UDEF_SAN_FLAGS} ${LEAK_SAN_FLAGS}" ALL_SANITIZERS_AVAILABLE_AND_ENABLED)
+        if(NOT ALL_SANITIZERS_AVAILABLE_AND_ENABLED)
+            message(FATAL_ERROR "All sanitizers requested but the test program failed to compile with ${ADDR_SAN_FLAGS} ${UDEF_SAN_FLAGS} ${LEAK_SAN_FLAGS}.")
+        endif()
+        set(ALL_ACTIVE_SAN_FLAGS "${ADDR_SAN_FLAGS} ${UDEF_SAN_FLAGS} ${LEAK_SAN_FLAGS}")
     endif()
 
     if(THREAD_SANITIZER_AVAILABLE_AND_ENABLED)
         set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} ${THREAD_SAN_FLAGS}" CACHE INTERNAL "Flags used by the C compiler during thread sanitizer builds." FORCE)
         set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} ${THREAD_SAN_FLAGS}" CACHE INTERNAL "Flags used by the C++ compiler during thread sanitizer builds." FORCE)
         set(CMAKE_SHARED_LINKER_FLAGS_DEBUG "${CMAKE_SHARED_LINKER_FLAGS_DEBUG}" CACHE INTERNAL "Flags used for the linker during thread sanitizer builds" FORCE)
+    elseif(ALL_SANITIZERS_AVAILABLE_AND_ENABLED)
+        set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} ${ALL_ACTIVE_SAN_FLAGS} -fno-omit-frame-pointer -fno-optimize-sibling-calls" CACHE INTERNAL "Flags used by the C compiler during most possible sanitizer builds." FORCE)
+        set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} ${ALL_ACTIVE_SAN_FLAGS} -fno-omit-frame-pointer -fno-optimize-sibling-calls" CACHE INTERNAL "Flags used by the C++ compiler during most possible sanitizer builds." FORCE)
+        set(CMAKE_SHARED_LINKER_FLAGS_DEBUG "${CMAKE_SHARED_LINKER_FLAGS_DEBUG}" CACHE INTERNAL "Flags used for the linker during most possible sanitizer builds" FORCE)
     elseif(ADDRESS_SANITIZER_AVAILABLE_AND_ENABLED)
         set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} ${ADDR_SAN_FLAGS} -fno-omit-frame-pointer -fno-optimize-sibling-calls" CACHE INTERNAL "Flags used by the C compiler during address sanitizer builds." FORCE)
         set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} ${ADDR_SAN_FLAGS} -fno-omit-frame-pointer -fno-optimize-sibling-calls" CACHE INTERNAL "Flags used by the C++ compiler during address sanitizer builds." FORCE)
@@ -74,9 +76,5 @@ if(NOT CMAKE_CROSSCOMPILING)
         set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} ${UDEF_SAN_FLAGS}" CACHE INTERNAL "Flags used by the C compiler during undefined behavior sanitizer builds." FORCE)
         set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} ${UDEF_SAN_FLAGS}" CACHE INTERNAL "Flags used by the C++ compiler during undefined behavior sanitizer builds." FORCE)
         set(CMAKE_SHARED_LINKER_FLAGS_DEBUG "${CMAKE_SHARED_LINKER_FLAGS_DEBUG}" CACHE INTERNAL "Flags used for the linker during undefined behavior sanitizer builds" FORCE)
-    elseif(ALL_SANITIZERS_AVAILABLE_AND_ENABLED)
-        set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} ${ALL_SAN_FLAGS} -fno-omit-frame-pointer -fno-optimize-sibling-calls" CACHE INTERNAL "Flags used by the C compiler during most possible sanitizer builds." FORCE)
-        set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} ${ALL_SAN_FLAGS} -fno-omit-frame-pointer -fno-optimize-sibling-calls" CACHE INTERNAL "Flags used by the C++ compiler during most possible sanitizer builds." FORCE)
-        set(CMAKE_SHARED_LINKER_FLAGS_DEBUG "${CMAKE_SHARED_LINKER_FLAGS_DEBUG}" CACHE INTERNAL "Flags used for the linker during most possible sanitizer builds" FORCE)
     endif()
 endif()

cpr/curlholder.cpp

@@ -20,7 +20,15 @@ CurlHolder::CurlHolder() {
     curl_easy_init_mutex_().unlock();
 
     assert(handle);
-} // namespace cpr
+}
+
+CurlHolder::CurlHolder(CurlHolder&& old) noexcept : handle(old.handle), chunk(old.chunk), resolveCurlList(old.resolveCurlList), multipart(old.multipart), error(std::move(old.error)) {
+    // Avoid double free
+    old.handle = nullptr;
+    old.chunk = nullptr;
+    old.resolveCurlList = nullptr;
+    old.multipart = nullptr;
+}
 
 CurlHolder::~CurlHolder() {
     curl_slist_free_all(chunk);
@@ -29,6 +37,28 @@ CurlHolder::~CurlHolder() {
     curl_easy_cleanup(handle);
 }
 
+CurlHolder& CurlHolder::operator=(CurlHolder&& old) noexcept {
+    // Free the previous stuff
+    curl_slist_free_all(chunk);
+    curl_slist_free_all(resolveCurlList);
+    curl_mime_free(multipart);
+    curl_easy_cleanup(handle);
+
+    // Move
+    handle = old.handle;
+    chunk = old.chunk;
+    resolveCurlList = old.resolveCurlList;
+    multipart = old.multipart;
+    error = std::move(old.error);
+
+    // Avoid double free
+    old.handle = nullptr;
+    old.chunk = nullptr;
+    old.resolveCurlList = nullptr;
+    old.multipart = nullptr;
+    return *this;
+}
+
 util::SecureString CurlHolder::urlEncode(std::string_view s) const {
     assert(handle);
     char* output = curl_easy_escape(handle, s.data(), static_cast<int>(s.length()));

include/cpr/curlholder.h

@@ -33,12 +33,12 @@ struct CurlHolder {
     std::array<char, CURL_ERROR_SIZE> error{};
 
     CurlHolder();
-    CurlHolder(const CurlHolder& other) = default;
-    CurlHolder(CurlHolder&& old) noexcept = default;
+    CurlHolder(const CurlHolder& other) = delete;
+    CurlHolder(CurlHolder&& old) noexcept;
     ~CurlHolder();
 
-    CurlHolder& operator=(CurlHolder&& old) noexcept = default;
-    CurlHolder& operator=(const CurlHolder& other) = default;
+    CurlHolder& operator=(const CurlHolder& other) = delete;
+    CurlHolder& operator=(CurlHolder&& old) noexcept;
 
     /**
      * Uses curl_easy_escape(...) for escaping the given string.

test/CMakeLists.txt

@@ -72,6 +72,7 @@ add_cpr_test(threadpool)
 add_cpr_test(testUtils)
 add_cpr_test(connection_pool)
 add_cpr_test(sse)
+add_cpr_test(curlholder)
 
 if (ENABLE_SSL_TESTS)
     add_cpr_test(ssl)

test/curlholder_tests.cpp

@@ -0,0 +1,20 @@
+#include <gtest/gtest.h>
+
+#include <utility>
+
+#include "cpr/curlholder.h"
+
+// Check if there is a double free in curl holder after move.
+// To reproduce this, run with address sanitizers enabled.
+// https://github.com/libcpr/cpr/issues/1286
+TEST(CurlholderTests, MoveOperator) {
+    cpr::CurlHolder a;
+    cpr::CurlHolder b;
+
+    a = std::move(b);
+}
+
+int main(int argc, char** argv) {
+    ::testing::InitGoogleTest(&argc, argv);
+    return RUN_ALL_TESTS();
+}