Merge pull request #1170 from mikael-s-persson/fix/secure_string

COM8 · web-flow · commit 4c416069bbe6 · 2025-02-08T10:32:17.000+01:00
Replaced the secureStringClear mechanism with a SecureString class
diff --git a/cpr/CMakeLists.txt b/cpr/CMakeLists.txt
@@ -4,7 +4,6 @@ add_library(cpr
         accept_encoding.cpp
         async.cpp
         auth.cpp
-        bearer.cpp
         callback.cpp
         cert_info.cpp
         cookies.cpp
diff --git a/cpr/auth.cpp b/cpr/auth.cpp
@@ -1,5 +1,4 @@
 #include "cpr/auth.h"
-#include "cpr/util.h"
 
 #include <string_view>
 
@@ -12,10 +11,6 @@ Authentication::Authentication(std::string_view username, std::string_view passw
     auth_string_ += password;
 }
 
-Authentication::~Authentication() noexcept {
-    util::secureStringClear(auth_string_);
-}
-
 const char* Authentication::GetAuthString() const noexcept {
     return auth_string_.c_str();
 }
diff --git a/cpr/bearer.cpp b/cpr/bearer.cpp
diff --git a/cpr/cookies.cpp b/cpr/cookies.cpp
@@ -5,6 +5,7 @@
 #include <iomanip>
 #include <sstream>
 #include <string>
+#include <string_view>
 
 namespace cpr {
 const std::string Cookie::GetDomain() const {
@@ -53,15 +54,15 @@ const std::string Cookies::GetEncoded(const CurlHolder& holder) const {
     std::stringstream stream;
     for (const cpr::Cookie& item : cookies_) {
         // Depending on if encoding is set to "true", we will URL-encode cookies
-        stream << (encode ? holder.urlEncode(item.GetName()) : item.GetName()) << "=";
+        stream << (encode ? std::string_view{holder.urlEncode(item.GetName())} : std::string_view{item.GetName()}) << "=";
 
         // special case version 1 cookies, which can be distinguished by
         // beginning and trailing quotes
         if (!item.GetValue().empty() && item.GetValue().front() == '"' && item.GetValue().back() == '"') {
             stream << item.GetValue();
         } else {
             // Depending on if encoding is set to "true", we will URL-encode cookies
-            stream << (encode ? holder.urlEncode(item.GetValue()) : item.GetValue());
+            stream << (encode ? std::string_view{holder.urlEncode(item.GetValue())} : std::string_view{item.GetValue()});
         }
         stream << "; ";
     }
diff --git a/cpr/curl_container.cpp b/cpr/curl_container.cpp
@@ -27,11 +27,11 @@ const std::string CurlContainer<Parameter>::GetContent(const CurlHolder& holder)
             content += "&";
         }
 
-        const std::string escapedKey = encode ? holder.urlEncode(parameter.key) : parameter.key;
+        const std::string escapedKey = encode ? std::string{holder.urlEncode(parameter.key)} : parameter.key;
         if (parameter.value.empty()) {
             content += escapedKey;
         } else {
-            const std::string escapedValue = encode ? holder.urlEncode(parameter.value) : parameter.value;
+            const std::string escapedValue = encode ? std::string{holder.urlEncode(parameter.value)} : parameter.value;
             content += escapedKey + "=";
             content += escapedValue;
         }
@@ -47,7 +47,7 @@ const std::string CurlContainer<Pair>::GetContent(const CurlHolder& holder) cons
         if (!content.empty()) {
             content += "&";
         }
-        const std::string escaped = encode ? holder.urlEncode(element.value) : element.value;
+        const std::string escaped = encode ? std::string{holder.urlEncode(element.value)} : element.value;
         content += element.key + "=" + escaped;
     }
 
diff --git a/cpr/curlholder.cpp b/cpr/curlholder.cpp
@@ -2,7 +2,8 @@
 #include <cassert>
 #include <curl/curl.h>
 #include <curl/easy.h>
-#include <string>
+#include <string_view>
+#include "cpr/secure_string.h"
 
 namespace cpr {
 CurlHolder::CurlHolder() {
@@ -28,22 +29,22 @@ CurlHolder::~CurlHolder() {
     curl_easy_cleanup(handle);
 }
 
-std::string CurlHolder::urlEncode(const std::string& s) const {
+util::SecureString CurlHolder::urlEncode(std::string_view s) const {
     assert(handle);
-    char* output = curl_easy_escape(handle, s.c_str(), static_cast<int>(s.length()));
+    char* output = curl_easy_escape(handle, s.data(), static_cast<int>(s.length()));
     if (output) {
-        std::string result = output;
+        util::SecureString result = output;
         curl_free(output);
         return result;
     }
     return "";
 }
 
-std::string CurlHolder::urlDecode(const std::string& s) const {
+util::SecureString CurlHolder::urlDecode(std::string_view s) const {
     assert(handle);
-    char* output = curl_easy_unescape(handle, s.c_str(), static_cast<int>(s.length()), nullptr);
+    char* output = curl_easy_unescape(handle, s.data(), static_cast<int>(s.length()), nullptr);
     if (output) {
-        std::string result = output;
+        util::SecureString result = output;
         curl_free(output);
         return result;
     }
diff --git a/cpr/file.cpp b/cpr/file.cpp
@@ -5,11 +5,7 @@
 
 namespace cpr {
 
-Files::Files(const std::initializer_list<std::string>& p_filepaths) {
-    for (const std::string& filepath : p_filepaths) {
-        files.emplace_back(filepath);
-    }
-}
+Files::Files(const std::initializer_list<std::string>& p_filepaths) : files(p_filepaths.begin(), p_filepaths.end()) {}
 
 Files::iterator Files::begin() {
     return files.begin();
diff --git a/cpr/proxyauth.cpp b/cpr/proxyauth.cpp
@@ -3,29 +3,25 @@
 #include <string>
 
 namespace cpr {
-EncodedAuthentication::~EncodedAuthentication() noexcept {
-    util::secureStringClear(username);
-    util::secureStringClear(password);
-}
 
-const std::string& EncodedAuthentication::GetUsername() const {
+std::string_view EncodedAuthentication::GetUsername() const {
     return username;
 }
 
-const std::string& EncodedAuthentication::GetPassword() const {
+std::string_view EncodedAuthentication::GetPassword() const {
     return password;
 }
 
 bool ProxyAuthentication::has(const std::string& protocol) const {
     return proxyAuth_.count(protocol) > 0;
 }
 
-const char* ProxyAuthentication::GetUsername(const std::string& protocol) {
-    return proxyAuth_[protocol].username.c_str();
+std::string_view ProxyAuthentication::GetUsername(const std::string& protocol) {
+    return proxyAuth_[protocol].GetUsername();
 }
 
-const char* ProxyAuthentication::GetPassword(const std::string& protocol) {
-    return proxyAuth_[protocol].password.c_str();
+std::string_view ProxyAuthentication::GetPassword(const std::string& protocol) {
+    return proxyAuth_[protocol].GetPassword();
 }
 
 } // namespace cpr
diff --git a/cpr/session.cpp b/cpr/session.cpp
@@ -903,7 +903,7 @@ const std::optional<Response> Session::intercept() {
     if (current_interceptor_ == interceptors_.end()) {
         current_interceptor_ = first_interceptor_;
     } else {
-        current_interceptor_++;
+        ++current_interceptor_;
     }
 
     if (current_interceptor_ != interceptors_.end()) {
diff --git a/cpr/util.cpp b/cpr/util.cpp
@@ -11,6 +11,7 @@
 #include <curl/curl.h>
 #include <fstream>
 #include <ios>
+#include <iterator>
 #include <sstream>
 #include <string>
 #include <type_traits>
@@ -166,7 +167,7 @@ int debugUserFunction(CURL* /*handle*/, curl_infotype type, char* data, size_t s
  * std::string input = "Hello World!";
  * std::string result = holder.urlEncode(input);
  **/
-std::string urlEncode(const std::string& s) {
+util::SecureString urlEncode(std::string_view s) {
     const CurlHolder holder; // Create a temporary new holder for URL encoding
     return holder.urlEncode(s);
 }
@@ -181,55 +182,16 @@ std::string urlEncode(const std::string& s) {
  * std::string input = "Hello%20World%21";
  * std::string result = holder.urlDecode(input);
  **/
-std::string urlDecode(const std::string& s) {
+util::SecureString urlDecode(std::string_view s) {
     const CurlHolder holder; // Create a temporary new holder for URL decoding
     return holder.urlDecode(s);
 }
 
-#if defined(__STDC_LIB_EXT1__)
-void secureStringClear(std::string& s) {
-    if (s.empty()) {
-        return;
-    }
-    memset_s(&s.front(), s.length(), 0, s.length());
-    s.clear();
-}
-#elif defined(_WIN32)
-void secureStringClear(std::string& s) {
-    if (s.empty()) {
-        return;
-    }
-    SecureZeroMemory(&s.front(), s.length());
-    s.clear();
-}
-#else
-#if defined(__clang__)
-#pragma clang optimize off // clang
-#elif defined(__GNUC__) || defined(__MINGW32__) || defined(__MINGW32__) || defined(__MINGW64__)
-#pragma GCC push_options   // g++
-#pragma GCC optimize("O0") // g++
-#endif
-void secureStringClear(std::string& s) {
-    if (s.empty()) {
-        return;
-    }
-    // NOLINTNEXTLINE (readability-container-data-pointer)
-    char* ptr = &(s[0]);
-    memset(ptr, '\0', s.length());
-    s.clear();
-}
-
-#if defined(__clang__)
-#pragma clang optimize on // clang
-#elif defined(__GNUC__) || defined(__MINGW32__) || defined(__MINGW32__) || defined(__MINGW64__)
-#pragma GCC pop_options // g++
-#endif
-#endif
-
 bool isTrue(const std::string& s) {
-    std::string temp_string{s};
-    std::transform(temp_string.begin(), temp_string.end(), temp_string.begin(), [](unsigned char c) { return static_cast<unsigned char>(std::tolower(c)); });
-    return temp_string == "true";
+    constexpr std::string_view tmp = "true";
+    auto [s_it, tmp_it] = std::mismatch(s.begin(), s.end(), tmp.begin(), tmp.end(),
+        [](auto s_c, auto t_c) { return std::tolower(s_c) == t_c; });
+    return s_it == s.end() && tmp_it == tmp.end();
 }
 
 time_t sTimestampToT(const std::string& st) {
diff --git a/include/CMakeLists.txt b/include/CMakeLists.txt
@@ -32,6 +32,7 @@ target_sources(cpr PRIVATE
     cpr/proxies.h
     cpr/proxyauth.h
     cpr/response.h
+    cpr/secure_string.h
     cpr/session.h
     cpr/singleton.h
     cpr/ssl_ctx.h
diff --git a/include/cpr/auth.h b/include/cpr/auth.h
@@ -4,23 +4,21 @@
 #include <string>
 #include <string_view>
 
+#include "cpr/util.h"
+
 namespace cpr {
 
 enum class AuthMode { BASIC, DIGEST, NTLM, NEGOTIATE };
 
 class Authentication {
   public:
     Authentication(std::string_view username, std::string_view password, AuthMode auth_mode);
-    Authentication(const Authentication& other) = default;
-    ~Authentication() noexcept;
-
-    Authentication& operator=(const Authentication& other) = default;
 
     const char* GetAuthString() const noexcept;
     AuthMode GetAuthMode() const noexcept;
 
   private:
-    std::string auth_string_;
+    util::SecureString auth_string_;
     AuthMode auth_mode_;
 };
 
diff --git a/include/cpr/bearer.h b/include/cpr/bearer.h
@@ -6,6 +6,8 @@
 
 #include <utility>
 
+#include "cpr/util.h"
+
 namespace cpr {
 
 // Only supported with libcurl >= 7.61.0.
@@ -14,18 +16,20 @@ namespace cpr {
 class Bearer {
   public:
     // NOLINTNEXTLINE(google-explicit-constructor, hicpp-explicit-conversions)
-    Bearer(std::string token) : token_string_{std::move(token)} {}
+    Bearer(std::string_view token) : token_string_{token} {}
     Bearer(const Bearer& other) = default;
     Bearer(Bearer&& old) noexcept = default;
-    virtual ~Bearer() noexcept;
+    virtual ~Bearer() noexcept = default;
 
     Bearer& operator=(Bearer&& old) noexcept = default;
     Bearer& operator=(const Bearer& other) = default;
 
-    virtual const char* GetToken() const noexcept;
+    virtual const char* GetToken() const noexcept {
+        return token_string_.c_str();
+    }
 
   protected:
-    std::string token_string_;
+    util::SecureString token_string_;
 };
 #endif
 
diff --git a/include/cpr/curlholder.h b/include/cpr/curlholder.h
@@ -6,7 +6,10 @@
 #include <mutex>
 #include <string>
 
+#include "cpr/secure_string.h"
+
 namespace cpr {
+
 struct CurlHolder {
   private:
     /**
@@ -41,12 +44,12 @@ struct CurlHolder {
     /**
      * Uses curl_easy_escape(...) for escaping the given string.
      **/
-    [[nodiscard]] std::string urlEncode(const std::string& s) const;
+    [[nodiscard]] util::SecureString urlEncode(std::string_view s) const;
 
     /**
      * Uses curl_easy_unescape(...) for unescaping the given string.
      **/
-    [[nodiscard]] std::string urlDecode(const std::string& s) const;
+    [[nodiscard]] util::SecureString urlDecode(std::string_view s) const;
 };
 } // namespace cpr
 
diff --git a/include/cpr/proxyauth.h b/include/cpr/proxyauth.h
diff --git a/include/cpr/secure_string.h b/include/cpr/secure_string.h
diff --git a/include/cpr/ssl_options.h b/include/cpr/ssl_options.h
diff --git a/include/cpr/util.h b/include/cpr/util.h
diff --git a/test/util_tests.cpp b/test/util_tests.cpp

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,4 @@`
`1`	`1`	`#include "cpr/auth.h"`
`2`		`-#include "cpr/util.h"`
`3`	`2`
`4`	`3`	`#include <string_view>`
`5`	`4`
`@@ -12,10 +11,6 @@ Authentication::Authentication(std::string_view username, std::string_view passw`
`12`	`11`	`auth_string_ += password;`
`13`	`12`	`}`
`14`	`13`
`15`		`-Authentication::~Authentication() noexcept {`
`16`		`- util::secureStringClear(auth_string_);`
`17`		`-}`
`18`		`-`
`19`	`14`	`const char* Authentication::GetAuthString() const noexcept {`
`20`	`15`	`return auth_string_.c_str();`
`21`	`16`	`}`
Original file line number	Diff line number	Diff line change
`@@ -27,11 +27,11 @@ const std::string CurlContainer<Parameter>::GetContent(const CurlHolder& holder)`
`27`	`27`	`content += "&";`
`28`	`28`	`}`
`29`	`29`
`30`		`- const std::string escapedKey = encode ? holder.urlEncode(parameter.key) : parameter.key;`
	`30`	`+ const std::string escapedKey = encode ? std::string{holder.urlEncode(parameter.key)} : parameter.key;`
`31`	`31`	`if (parameter.value.empty()) {`
`32`	`32`	`content += escapedKey;`
`33`	`33`	`} else {`
`34`		`- const std::string escapedValue = encode ? holder.urlEncode(parameter.value) : parameter.value;`
	`34`	`+ const std::string escapedValue = encode ? std::string{holder.urlEncode(parameter.value)} : parameter.value;`
`35`	`35`	`content += escapedKey + "=";`
`36`	`36`	`content += escapedValue;`
`37`	`37`	`}`
`@@ -47,7 +47,7 @@ const std::string CurlContainer<Pair>::GetContent(const CurlHolder& holder) cons`
`47`	`47`	`if (!content.empty()) {`
`48`	`48`	`content += "&";`
`49`	`49`	`}`
`50`		`- const std::string escaped = encode ? holder.urlEncode(element.value) : element.value;`
	`50`	`+ const std::string escaped = encode ? std::string{holder.urlEncode(element.value)} : element.value;`
`51`	`51`	`content += element.key + "=" + escaped;`
`52`	`52`	`}`
`53`	`53`
Original file line number	Diff line number	Diff line change
`@@ -3,29 +3,25 @@`
`3`	`3`	`#include <string>`
`4`	`4`
`5`	`5`	`namespace cpr {`
`6`		`-EncodedAuthentication::~EncodedAuthentication() noexcept {`
`7`		`- util::secureStringClear(username);`
`8`		`- util::secureStringClear(password);`
`9`		`-}`
`10`	`6`
`11`		`-const std::string& EncodedAuthentication::GetUsername() const {`
	`7`	`+std::string_view EncodedAuthentication::GetUsername() const {`
`12`	`8`	`return username;`
`13`	`9`	`}`
`14`	`10`
`15`		`-const std::string& EncodedAuthentication::GetPassword() const {`
	`11`	`+std::string_view EncodedAuthentication::GetPassword() const {`
`16`	`12`	`return password;`
`17`	`13`	`}`
`18`	`14`
`19`	`15`	`bool ProxyAuthentication::has(const std::string& protocol) const {`
`20`	`16`	`return proxyAuth_.count(protocol) > 0;`
`21`	`17`	`}`
`22`	`18`
`23`		`-const char* ProxyAuthentication::GetUsername(const std::string& protocol) {`
`24`		`- return proxyAuth_[protocol].username.c_str();`
	`19`	`+std::string_view ProxyAuthentication::GetUsername(const std::string& protocol) {`
	`20`	`+ return proxyAuth_[protocol].GetUsername();`
`25`	`21`	`}`
`26`	`22`
`27`		`-const char* ProxyAuthentication::GetPassword(const std::string& protocol) {`
`28`		`- return proxyAuth_[protocol].password.c_str();`
	`23`	`+std::string_view ProxyAuthentication::GetPassword(const std::string& protocol) {`
	`24`	`+ return proxyAuth_[protocol].GetPassword();`
`29`	`25`	`}`
`30`	`26`
`31`	`27`	`} // namespace cpr`
Original file line number	Diff line number	Diff line change
`@@ -903,7 +903,7 @@ const std::optional<Response> Session::intercept() {`
`903`	`903`	`if (current_interceptor_ == interceptors_.end()) {`
`904`	`904`	`current_interceptor_ = first_interceptor_;`
`905`	`905`	`} else {`
`906`		`- current_interceptor_++;`
	`906`	`+ ++current_interceptor_;`
`907`	`907`	`}`
`908`	`908`
`909`	`909`	`if (current_interceptor_ != interceptors_.end()) {`