From b347afcc5b4c5228a425508d96e99b85cac7a1d7 Mon Sep 17 00:00:00 2001
From: seelabs <scott.determan@yahoo.com>
Date: Thu, 28 Mar 2019 16:34:12 -0400
Subject: [PATCH] Better error checking in CachedViewImpl::read:

* Prevent null pointer dereferences
* Alway check for correct sle type before returning sle
* Reformat code
---
 src/ripple/ledger/impl/CachedView.cpp | 38 +++++++++++++--------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/src/ripple/ledger/impl/CachedView.cpp b/src/ripple/ledger/impl/CachedView.cpp
index d7f1c8f62..14bfe9431 100644
--- a/src/ripple/ledger/impl/CachedView.cpp
+++ b/src/ripple/ledger/impl/CachedView.cpp
@@ -31,38 +31,38 @@ CachedViewImpl::exists (Keylet const& k) const
 }
 
 std::shared_ptr<SLE const>
-CachedViewImpl::read (Keylet const& k) const
+CachedViewImpl::read(Keylet const& k) const
 {
     {
-        std::lock_guard<
-            std::mutex> lock(mutex_);
+        std::lock_guard<std::mutex> lock(mutex_);
         auto const iter = map_.find(k.key);
         if (iter != map_.end())
         {
-            if (! k.check(*iter->second))
+            if (!iter->second || !k.check(*iter->second))
                 return nullptr;
             return iter->second;
         }
     }
-    auto const digest =
-        base_.digest(k.key);
-    if (! digest)
+    auto const digest = base_.digest(k.key);
+    if (!digest)
         return nullptr;
-    auto sle = cache_.fetch(*digest,
-        [&]() { return base_.read(k); });
-    std::lock_guard<
-        std::mutex> lock(mutex_);
-    auto const iter =
-        map_.find(k.key);
-    if (iter == map_.end())
+    auto sle = cache_.fetch(*digest, [&]() { return base_.read(k); });
+    std::lock_guard<std::mutex> lock(mutex_);
+    auto const er = map_.emplace(k.key, sle);
+    auto const& iter = er.first;
+    bool const inserted = er.second;
+    if (iter->second && !k.check(*iter->second))
     {
-        map_.emplace(k.key, sle);
-        return sle;
+        if (!inserted)
+        {
+            // On entry, this function did not find this key in map_. Now something
+            // (another thread?) has inserted the sle into the map and it has
+            // the wrong type.
+            LogicError("CachedView::read: wrong type");
+        }
+        return nullptr;
     }
-    if (! k.check(*iter->second))
-        LogicError("CachedView::read: wrong type");
     return iter->second;
-
 }
 
 } // detail