From f4a0da11a2ed19baa74fd45617eb8beab6d9510e Mon Sep 17 00:00:00 2001
From: Roker <roker@pep-project.org>
Date: Wed, 1 Jul 2020 22:38:18 +0200
Subject: [PATCH] avoid possible dangling pointer to member of destructed local
 object. Add documentation. Avoid superfluous string copies.

---
 passphrase_cache.cc | 18 +++++++++++++-----
 passphrase_cache.hh |  8 ++++----
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/passphrase_cache.cc b/passphrase_cache.cc
index 3ba6442..3e6cd0d 100644
--- a/passphrase_cache.cc
+++ b/passphrase_cache.cc
@@ -1,8 +1,13 @@
 #include <cassert>
 #include "passphrase_cache.hh"
 
+namespace
+{
+    const char* const empty_string = "";
+}
+
 namespace pEp {
-    PassphraseCache::cache_entry::cache_entry(std::string p, time_point t) :
+    PassphraseCache::cache_entry::cache_entry(const std::string& p, time_point t) :
             passphrase{p, 0, PassphraseCache::cache_entry::max_len}, tp{t}
     { }
 
@@ -27,18 +32,21 @@ namespace pEp {
         return *this;
     }
 
-    const char *PassphraseCache::add(std::string passphrase)
+    const char *PassphraseCache::add(const std::string& passphrase)
     {
         assert(_which == _cache.end()); // never modify while iterating
         std::lock_guard<std::mutex> lock(_mtx);
 
-        if (passphrase != "") {
+        if (!passphrase.empty()) {
             while (_cache.size() >= _max_size)
                 _cache.pop_front();
-            _cache.emplace_back(cache_entry(passphrase, clock::now()));
+            
+            _cache.emplace_back(passphrase, clock::now());
+            auto back = _cache.back(); // FIXME: In C++17 list::emplace_back() returns the just inserted element already.
+            return back.passphrase.c_str();
         }
 
-        return passphrase.c_str();
+        return empty_string;
     }
 
     bool PassphraseCache::for_each_passphrase(const passphrase_callee& callee)
diff --git a/passphrase_cache.hh b/passphrase_cache.hh
index 97e60fa..525adfc 100644
--- a/passphrase_cache.hh
+++ b/passphrase_cache.hh
@@ -16,7 +16,7 @@ namespace pEp {
 
         struct cache_entry {
             static const size_t max_len = 250 * 4;
-            cache_entry(std::string p, time_point t);
+            cache_entry(const std::string&  p, time_point t);
 
             std::string passphrase;
             time_point tp;
@@ -43,9 +43,9 @@ namespace pEp {
         PassphraseCache(const PassphraseCache& second);
         PassphraseCache& operator=(const PassphraseCache& second);
 
-        // adding a passphrase to the cache, which will timeout
-
-        const char *add(std::string passphrase);
+        // adds the passphrase to the cache, which will timeout
+        // returns a ptr to the passsword entry in the cache. Don't free() it!
+        const char *add(const std::string& passphrase);
 
         // get all passphrases in cache from latest to oldest one by each call
         // this function is throwing PassphraseCache::Empty when cache is empty