From 31d1ebeff20e5f17a2665dcd758b45bbdadb6a6c Mon Sep 17 00:00:00 2001 From: Fabian Sauter Date: Mon, 19 Jan 2026 19:45:41 +0100 Subject: [PATCH 1/3] Fixed double free inside curl holder --- cpr/curlholder.cpp | 32 +++++++++++++++++++++++++++++++- include/cpr/curlholder.h | 8 ++++---- 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/cpr/curlholder.cpp b/cpr/curlholder.cpp index 3e80ad17e..4c8084634 100644 --- a/cpr/curlholder.cpp +++ b/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(s.length())); diff --git a/include/cpr/curlholder.h b/include/cpr/curlholder.h index 19ca2ba09..75aff10cd 100644 --- a/include/cpr/curlholder.h +++ b/include/cpr/curlholder.h @@ -33,12 +33,12 @@ struct CurlHolder { std::array 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. From d44340b3d6754332cf1975bbba4889c6f1d61c5e Mon Sep 17 00:00:00 2001 From: Fabian Sauter Date: Mon, 19 Jan 2026 19:46:10 +0100 Subject: [PATCH 2/3] Fixed sanitizer activation --- cmake/sanitizer.cmake | 74 +++++++++++++++++++++---------------------- 1 file changed, 36 insertions(+), 38 deletions(-) diff --git a/cmake/sanitizer.cmake b/cmake/sanitizer.cmake index f49db4d71..5801817bb 100644 --- a/cmake/sanitizer.cmake +++ b/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() From f3ecc3cf68dca0b215ced890e95aab0dfd15e01e Mon Sep 17 00:00:00 2001 From: Fabian Sauter Date: Mon, 19 Jan 2026 19:46:33 +0100 Subject: [PATCH 3/3] Added curl holder double free tests --- test/CMakeLists.txt | 1 + test/curlholder_tests.cpp | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+) create mode 100644 test/curlholder_tests.cpp diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 3b624419c..9ba6148e0 100644 --- a/test/CMakeLists.txt +++ b/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) diff --git a/test/curlholder_tests.cpp b/test/curlholder_tests.cpp new file mode 100644 index 000000000..c7cdc454a --- /dev/null +++ b/test/curlholder_tests.cpp @@ -0,0 +1,20 @@ +#include + +#include + +#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(); +}