Add CPDF_{Array,Dictionary}Locker to catch illegal iteration patterns.

Move begin/end methods onto locker object which tracks whether
iterators are in existence.

Change-Id: Ia869f313fce48d10a0d0180d0cc083eed6ea1584
Reviewed-on: https://pdfium-review.googlesource.com/c/44070
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>

27 files changed, 202 insertions, 89 deletions

diff --git a/core/fpdfapi/edit/cpdf_creator.cpp b/core/fpdfapi/edit/cpdf_creator.cpp
index c4bc698acc..7eb7673d69 100644
--- a/core/fpdfapi/edit/cpdf_creator.cpp
+++ b/core/fpdfapi/edit/cpdf_creator.cpp
@@ -445,7 +445,8 @@ CPDF_Creator::Stage CPDF_Creator::WriteDoc_Stage4() {
 
   if (m_pParser) {
     std::unique_ptr<CPDF_Dictionary> p = m_pParser->GetCombinedTrailer();
-    for (const auto& it : *p) {
+    CPDF_DictionaryLocker locker(p.get());
+    for (const auto& it : locker) {
       const ByteString& key = it.first;
       CPDF_Object* pValue = it.second.get();
       if (key == "Encrypt" || key == "Size" || key == "Filter" ||
diff --git a/core/fpdfapi/page/cpdf_allstates.cpp b/core/fpdfapi/page/cpdf_allstates.cpp
index 9af904cb5d..3e6692b223 100644
--- a/core/fpdfapi/page/cpdf_allstates.cpp
+++ b/core/fpdfapi/page/cpdf_allstates.cpp
@@ -39,7 +39,8 @@ void CPDF_AllStates::SetLineDash(CPDF_Array* pArray, float phase, float scale) {
 
 void CPDF_AllStates::ProcessExtGS(CPDF_Dictionary* pGS,
                                   CPDF_StreamContentParser* pParser) {
-  for (const auto& it : *pGS) {
+  CPDF_DictionaryLocker locker(pGS);
+  for (const auto& it : locker) {
     const ByteString& key_str = it.first;
     CPDF_Object* pElement = it.second.get();
     CPDF_Object* pObject = pElement ? pElement->GetDirect() : nullptr;
diff --git a/core/fpdfapi/page/cpdf_colorspace.cpp b/core/fpdfapi/page/cpdf_colorspace.cpp
index 2266dec9e8..bcfd42d72f 100644
--- a/core/fpdfapi/page/cpdf_colorspace.cpp
+++ b/core/fpdfapi/page/cpdf_colorspace.cpp
@@ -461,7 +461,8 @@ std::unique_ptr<CPDF_ColorSpace> CPDF_ColorSpace::Load(
     if (!pDict)
       return nullptr;
 
-    for (const auto& it : *pDict) {
+    CPDF_DictionaryLocker locker(pDict);
+    for (const auto& it : locker) {
       std::unique_ptr<CPDF_ColorSpace> pRet;
       CPDF_Object* pValue = it.second.get();
       if (ToName(pValue))
diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.cpp b/core/fpdfapi/page/cpdf_streamcontentparser.cpp
index 79fa3d2890..beba208739 100644
--- a/core/fpdfapi/page/cpdf_streamcontentparser.cpp
+++ b/core/fpdfapi/page/cpdf_streamcontentparser.cpp
@@ -174,36 +174,39 @@ ByteStringView FindFullName(const AbbrPair* table,
 void ReplaceAbbr(CPDF_Object* pObj) {
   switch (pObj->GetType()) {
     case CPDF_Object::DICTIONARY: {
-      CPDF_Dictionary* pDict = pObj->AsDictionary();
       std::vector<AbbrReplacementOp> replacements;
-      for (const auto& it : *pDict) {
-        ByteString key = it.first;
-        CPDF_Object* value = it.second.get();
-        ByteStringView fullname = FindFullName(
-            InlineKeyAbbr, FX_ArraySize(InlineKeyAbbr), key.AsStringView());
-        if (!fullname.IsEmpty()) {
-          AbbrReplacementOp op;
-          op.is_replace_key = true;
-          op.key = std::move(key);
-          op.replacement = fullname;
-          replacements.push_back(op);
-          key = fullname;
-        }
-
-        if (value->IsName()) {
-          ByteString name = value->GetString();
-          fullname =
-              FindFullName(InlineValueAbbr, FX_ArraySize(InlineValueAbbr),
-                           name.AsStringView());
+      CPDF_Dictionary* pDict = pObj->AsDictionary();
+      {
+        CPDF_DictionaryLocker locker(pDict);
+        for (const auto& it : locker) {
+          ByteString key = it.first;
+          CPDF_Object* value = it.second.get();
+          ByteStringView fullname = FindFullName(
+              InlineKeyAbbr, FX_ArraySize(InlineKeyAbbr), key.AsStringView());
           if (!fullname.IsEmpty()) {
             AbbrReplacementOp op;
-            op.is_replace_key = false;
-            op.key = key;
+            op.is_replace_key = true;
+            op.key = std::move(key);
             op.replacement = fullname;
             replacements.push_back(op);
+            key = fullname;
+          }
+
+          if (value->IsName()) {
+            ByteString name = value->GetString();
+            fullname =
+                FindFullName(InlineValueAbbr, FX_ArraySize(InlineValueAbbr),
+                             name.AsStringView());
+            if (!fullname.IsEmpty()) {
+              AbbrReplacementOp op;
+              op.is_replace_key = false;
+              op.key = key;
+              op.replacement = fullname;
+              replacements.push_back(op);
+            }
+          } else {
+            ReplaceAbbr(value);
           }
-        } else {
-          ReplaceAbbr(value);
         }
       }
       for (const auto& op : replacements) {
diff --git a/core/fpdfapi/parser/cpdf_array.cpp b/core/fpdfapi/parser/cpdf_array.cpp
index f2eca1be70..e0ac34169f 100644
--- a/core/fpdfapi/parser/cpdf_array.cpp
+++ b/core/fpdfapi/parser/cpdf_array.cpp
@@ -175,16 +175,19 @@ const CPDF_Array* CPDF_Array::GetArrayAt(size_t i) const {
 }
 
 void CPDF_Array::Clear() {
+  CHECK(!IsLocked());
   m_Objects.clear();
 }
 
 void CPDF_Array::RemoveAt(size_t i) {
+  CHECK(!IsLocked());
   if (i < m_Objects.size())
     m_Objects.erase(m_Objects.begin() + i);
 }
 
 void CPDF_Array::ConvertToIndirectObjectAt(size_t i,
                                            CPDF_IndirectObjectHolder* pHolder) {
+  CHECK(!IsLocked());
   if (i >= m_Objects.size())
     return;
 
@@ -196,6 +199,7 @@ void CPDF_Array::ConvertToIndirectObjectAt(size_t i,
 }
 
 CPDF_Object* CPDF_Array::SetAt(size_t i, std::unique_ptr<CPDF_Object> pObj) {
+  CHECK(!IsLocked());
   ASSERT(IsArray());
   ASSERT(!pObj || pObj->IsInline());
   if (i >= m_Objects.size()) {
@@ -209,6 +213,7 @@ CPDF_Object* CPDF_Array::SetAt(size_t i, std::unique_ptr<CPDF_Object> pObj) {
 
 CPDF_Object* CPDF_Array::InsertAt(size_t index,
                                   std::unique_ptr<CPDF_Object> pObj) {
+  CHECK(!IsLocked());
   ASSERT(IsArray());
   CHECK(!pObj || pObj->IsInline());
   CPDF_Object* pRet = pObj.get();
@@ -224,6 +229,7 @@ CPDF_Object* CPDF_Array::InsertAt(size_t index,
 }
 
 CPDF_Object* CPDF_Array::Add(std::unique_ptr<CPDF_Object> pObj) {
+  CHECK(!IsLocked());
   ASSERT(IsArray());
   CHECK(!pObj || pObj->IsInline());
   CPDF_Object* pRet = pObj.get();
@@ -242,3 +248,12 @@ bool CPDF_Array::WriteTo(IFX_ArchiveStream* archive,
   }
   return archive->WriteString("]");
 }
+
+CPDF_ArrayLocker::CPDF_ArrayLocker(const CPDF_Array* pArray)
+    : m_pArray(pArray) {
+  m_pArray->m_LockCount++;
+}
+
+CPDF_ArrayLocker::~CPDF_ArrayLocker() {
+  m_pArray->m_LockCount--;
+}
diff --git a/core/fpdfapi/parser/cpdf_array.h b/core/fpdfapi/parser/cpdf_array.h
index 0905251382..f7b4a1fc9e 100644
--- a/core/fpdfapi/parser/cpdf_array.h
+++ b/core/fpdfapi/parser/cpdf_array.h
@@ -108,17 +108,38 @@ class CPDF_Array final : public CPDF_Object {
   void Clear();
   void RemoveAt(size_t index);
   void ConvertToIndirectObjectAt(size_t index, CPDF_IndirectObjectHolder* pDoc);
-
-  const_iterator begin() const { return m_Objects.begin(); }
-  const_iterator end() const { return m_Objects.end(); }
+  bool IsLocked() const { return !!m_LockCount; }
 
  private:
+  friend class CPDF_ArrayLocker;
+
   std::unique_ptr<CPDF_Object> CloneNonCyclic(
       bool bDirect,
       std::set<const CPDF_Object*>* pVisited) const override;
 
   std::vector<std::unique_ptr<CPDF_Object>> m_Objects;
   WeakPtr<ByteStringPool> m_pPool;
+  mutable uint32_t m_LockCount = 0;
+};
+
+class CPDF_ArrayLocker {
+ public:
+  using const_iterator = CPDF_Array::const_iterator;
+
+  explicit CPDF_ArrayLocker(const CPDF_Array* pArray);
+  ~CPDF_ArrayLocker();
+
+  const_iterator begin() const {
+    CHECK(m_pArray->IsLocked());
+    return m_pArray->m_Objects.begin();
+  }
+  const_iterator end() const {
+    CHECK(m_pArray->IsLocked());
+    return m_pArray->m_Objects.end();
+  }
+
+ private:
+  UnownedPtr<const CPDF_Array> const m_pArray;
 };
 
 inline CPDF_Array* ToArray(CPDF_Object* obj) {
diff --git a/core/fpdfapi/parser/cpdf_array_unittest.cpp b/core/fpdfapi/parser/cpdf_array_unittest.cpp
index a70f9910ed..fbc5c393b2 100644
--- a/core/fpdfapi/parser/cpdf_array_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_array_unittest.cpp
@@ -178,7 +178,9 @@ TEST(cpdf_array, Iterator) {
   for (size_t i = 0; i < FX_ArraySize(elems); ++i)
     arr->InsertNewAt<CPDF_Number>(i, elems[i]);
   size_t index = 0;
-  for (const auto& it : *arr)
+
+  CPDF_ArrayLocker locker(arr.get());
+  for (const auto& it : locker)
     EXPECT_EQ(elems[index++], it->AsNumber()->GetInteger());
   EXPECT_EQ(FX_ArraySize(elems), index);
 }
diff --git a/core/fpdfapi/parser/cpdf_cross_ref_table.cpp b/core/fpdfapi/parser/cpdf_cross_ref_table.cpp
index 4f7c482c9f..31e9fd3f62 100644
--- a/core/fpdfapi/parser/cpdf_cross_ref_table.cpp
+++ b/core/fpdfapi/parser/cpdf_cross_ref_table.cpp
@@ -5,6 +5,7 @@
 #include "core/fpdfapi/parser/cpdf_cross_ref_table.h"
 
 #include <utility>
+#include <vector>
 
 #include "core/fpdfapi/parser/cpdf_dictionary.h"
 #include "core/fpdfapi/parser/cpdf_parser.h"
@@ -153,9 +154,6 @@ void CPDF_CrossRefTable::UpdateTrailer(
   new_trailer->SetFor("XRefStm", trailer_->RemoveFor("XRefStm"));
   new_trailer->SetFor("Prev", trailer_->RemoveFor("Prev"));
 
-  for (auto it = new_trailer->begin(); it != new_trailer->end();) {
-    const ByteString key = it->first;
-    ++it;
+  for (const auto& key : new_trailer->GetKeys())
     trailer_->SetFor(key, new_trailer->RemoveFor(key));
-  }
 }
diff --git a/core/fpdfapi/parser/cpdf_data_avail.cpp b/core/fpdfapi/parser/cpdf_data_avail.cpp
index c046996498..b685214f6f 100644
--- a/core/fpdfapi/parser/cpdf_data_avail.cpp
+++ b/core/fpdfapi/parser/cpdf_data_avail.cpp
@@ -310,7 +310,8 @@ bool CPDF_DataAvail::CheckPage() {
     }
     CPDF_Array* pArray = ToArray(pObj.get());
     if (pArray) {
-      for (const auto& pArrayObj : *pArray) {
+      CPDF_ArrayLocker locker(pArray);
+      for (const auto& pArrayObj : locker) {
         if (CPDF_Reference* pRef = ToReference(pArrayObj.get()))
           UnavailObjList.push_back(pRef->GetRefObjNum());
       }
diff --git a/core/fpdfapi/parser/cpdf_dictionary.cpp b/core/fpdfapi/parser/cpdf_dictionary.cpp
index 9c06e42804..6c005c8db2 100644
--- a/core/fpdfapi/parser/cpdf_dictionary.cpp
+++ b/core/fpdfapi/parser/cpdf_dictionary.cpp
@@ -72,7 +72,8 @@ std::unique_ptr<CPDF_Object> CPDF_Dictionary::CloneNonCyclic(
     std::set<const CPDF_Object*>* pVisited) const {
   pVisited->insert(this);
   auto pCopy = pdfium::MakeUnique<CPDF_Dictionary>(m_pPool);
-  for (const auto& it : *this) {
+  CPDF_DictionaryLocker locker(this);
+  for (const auto& it : locker) {
     if (!pdfium::ContainsKey(*pVisited, it.second.get())) {
       std::set<const CPDF_Object*> visited(*pVisited);
       if (auto obj = it.second->CloneNonCyclic(bDirect, &visited))
@@ -195,8 +196,17 @@ bool CPDF_Dictionary::KeyExist(const ByteString& key) const {
   return pdfium::ContainsKey(m_Map, key);
 }
 
+std::vector<ByteString> CPDF_Dictionary::GetKeys() const {
+  std::vector<ByteString> result;
+  CPDF_DictionaryLocker locker(this);
+  for (const auto& item : locker)
+    result.push_back(item.first);
+  return result;
+}
+
 CPDF_Object* CPDF_Dictionary::SetFor(const ByteString& key,
                                      std::unique_ptr<CPDF_Object> pObj) {
+  CHECK(!IsLocked());
   if (!pObj) {
     m_Map.erase(key);
     return nullptr;
@@ -210,6 +220,7 @@ CPDF_Object* CPDF_Dictionary::SetFor(const ByteString& key,
 void CPDF_Dictionary::ConvertToIndirectObjectFor(
     const ByteString& key,
     CPDF_IndirectObjectHolder* pHolder) {
+  CHECK(!IsLocked());
   auto it = m_Map.find(key);
   if (it == m_Map.end() || it->second->IsReference())
     return;
@@ -219,6 +230,7 @@ void CPDF_Dictionary::ConvertToIndirectObjectFor(
 }
 
 std::unique_ptr<CPDF_Object> CPDF_Dictionary::RemoveFor(const ByteString& key) {
+  CHECK(!IsLocked());
   std::unique_ptr<CPDF_Object> result;
   auto it = m_Map.find(key);
   if (it != m_Map.end()) {
@@ -230,6 +242,7 @@ std::unique_ptr<CPDF_Object> CPDF_Dictionary::RemoveFor(const ByteString& key) {
 
 void CPDF_Dictionary::ReplaceKey(const ByteString& oldkey,
                                  const ByteString& newkey) {
+  CHECK(!IsLocked());
   auto old_it = m_Map.find(oldkey);
   if (old_it == m_Map.end())
     return;
@@ -273,7 +286,8 @@ bool CPDF_Dictionary::WriteTo(IFX_ArchiveStream* archive,
 
   const bool is_signature = CPDF_CryptoHandler::IsSignatureDictionary(this);
 
-  for (const auto& it : *this) {
+  CPDF_DictionaryLocker locker(this);
+  for (const auto& it : locker) {
     const ByteString& key = it.first;
     CPDF_Object* pValue = it.second.get();
     if (!archive->WriteString("/") ||
@@ -288,3 +302,12 @@ bool CPDF_Dictionary::WriteTo(IFX_ArchiveStream* archive,
   }
   return archive->WriteString(">>");
 }
+
+CPDF_DictionaryLocker::CPDF_DictionaryLocker(const CPDF_Dictionary* pDictionary)
+    : m_pDictionary(pDictionary) {
+  m_pDictionary->m_LockCount++;
+}
+
+CPDF_DictionaryLocker::~CPDF_DictionaryLocker() {
+  m_pDictionary->m_LockCount--;
+}
diff --git a/core/fpdfapi/parser/cpdf_dictionary.h b/core/fpdfapi/parser/cpdf_dictionary.h
index 2e972d0577..e6abd26cc2 100644
--- a/core/fpdfapi/parser/cpdf_dictionary.h
+++ b/core/fpdfapi/parser/cpdf_dictionary.h
@@ -11,12 +11,14 @@
 #include <memory>
 #include <set>
 #include <utility>
+#include <vector>
 
 #include "core/fpdfapi/parser/cpdf_object.h"
 #include "core/fxcrt/fx_coordinates.h"
 #include "core/fxcrt/fx_string.h"
 #include "core/fxcrt/string_pool_template.h"
 #include "core/fxcrt/weak_ptr.h"
+#include "third_party/base/logging.h"
 #include "third_party/base/ptr_util.h"
 
 class CPDF_IndirectObjectHolder;
@@ -41,6 +43,8 @@ class CPDF_Dictionary final : public CPDF_Object {
   bool WriteTo(IFX_ArchiveStream* archive,
                const CPDF_Encryptor* encryptor) const override;
 
+  bool IsLocked() const { return !!m_LockCount; }
+
   size_t size() const { return m_Map.size(); }
   const CPDF_Object* GetObjectFor(const ByteString& key) const;
   CPDF_Object* GetObjectFor(const ByteString& key);
@@ -65,6 +69,7 @@ class CPDF_Dictionary final : public CPDF_Object {
   float GetFloatFor(const ByteString& key) const { return GetNumberFor(key); }
 
   bool KeyExist(const ByteString& key) const;
+  std::vector<ByteString> GetKeys() const;
 
   // Set* functions invalidate iterators for the element with the key |key|.
   // Takes ownership of |pObj|, returns an unowned pointer to it.
@@ -76,6 +81,7 @@ class CPDF_Dictionary final : public CPDF_Object {
   typename std::enable_if<!CanInternStrings<T>::value, T*>::type SetNewFor(
       const ByteString& key,
       Args&&... args) {
+    CHECK(!IsLocked());
     return static_cast<T*>(
         SetFor(key, pdfium::MakeUnique<T>(std::forward<Args>(args)...)));
   }
@@ -83,6 +89,7 @@ class CPDF_Dictionary final : public CPDF_Object {
   typename std::enable_if<CanInternStrings<T>::value, T*>::type SetNewFor(
       const ByteString& key,
       Args&&... args) {
+    CHECK(!IsLocked());
     return static_cast<T*>(SetFor(
         key, pdfium::MakeUnique<T>(m_pPool, std::forward<Args>(args)...)));
   }
@@ -100,21 +107,41 @@ class CPDF_Dictionary final : public CPDF_Object {
   // Invalidates iterators for the element with the key |oldkey|.
   void ReplaceKey(const ByteString& oldkey, const ByteString& newkey);
 
-  const_iterator begin() const { return m_Map.begin(); }
-  const_iterator end() const { return m_Map.end(); }
-
   WeakPtr<ByteStringPool> GetByteStringPool() const { return m_pPool; }
 
  private:
+  friend class CPDF_DictionaryLocker;
+
   ByteString MaybeIntern(const ByteString& str);
   std::unique_ptr<CPDF_Object> CloneNonCyclic(
       bool bDirect,
       std::set<const CPDF_Object*>* visited) const override;
 
+  mutable uint32_t m_LockCount = 0;
   WeakPtr<ByteStringPool> m_pPool;
   std::map<ByteString, std::unique_ptr<CPDF_Object>> m_Map;
 };
 
+class CPDF_DictionaryLocker {
+ public:
+  using const_iterator = CPDF_Dictionary::const_iterator;
+
+  explicit CPDF_DictionaryLocker(const CPDF_Dictionary* pDictionary);
+  ~CPDF_DictionaryLocker();
+
+  const_iterator begin() const {
+    CHECK(m_pDictionary->IsLocked());
+    return m_pDictionary->m_Map.begin();
+  }
+  const_iterator end() const {
+    CHECK(m_pDictionary->IsLocked());
+    return m_pDictionary->m_Map.end();
+  }
+
+ private:
+  UnownedPtr<const CPDF_Dictionary> const m_pDictionary;
+};
+
 inline CPDF_Dictionary* ToDictionary(CPDF_Object* obj) {
   return obj ? obj->AsDictionary() : nullptr;
 }
diff --git a/core/fpdfapi/parser/cpdf_object.h b/core/fpdfapi/parser/cpdf_object.h
index 6c6b583963..4f64aec265 100644
--- a/core/fpdfapi/parser/cpdf_object.h
+++ b/core/fpdfapi/parser/cpdf_object.h
@@ -115,16 +115,13 @@ class CPDF_Object {
       CPDF_IndirectObjectHolder* holder) const;
 
  protected:
-  CPDF_Object() : m_ObjNum(0), m_GenNum(0) {}
+  CPDF_Object() = default;
+  CPDF_Object(const CPDF_Object& src) = delete;
 
   std::unique_ptr<CPDF_Object> CloneObjectNonCyclic(bool bDirect) const;
 
-  uint32_t m_ObjNum;
-
- private:
-  CPDF_Object(const CPDF_Object& src) {}
-
-  uint32_t m_GenNum;
+  uint32_t m_ObjNum = 0;
+  uint32_t m_GenNum = 0;
 };
 
 template <typename T>
diff --git a/core/fpdfapi/parser/cpdf_object_unittest.cpp b/core/fpdfapi/parser/cpdf_object_unittest.cpp
index e5b322ff42..c3ec3aeddb 100644
--- a/core/fpdfapi/parser/cpdf_object_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_object_unittest.cpp
@@ -138,9 +138,9 @@ class PDFObjectsTest : public testing::Test {
         const CPDF_Dictionary* dict2 = obj2->AsDictionary();
         if (dict1->size() != dict2->size())
           return false;
-        for (CPDF_Dictionary::const_iterator it = dict1->begin();
-             it != dict1->end(); ++it) {
-          if (!Equal(it->second.get(), dict2->GetObjectFor(it->first)))
+        CPDF_DictionaryLocker locker1(dict1);
+        for (const auto& item : locker1) {
+          if (!Equal(item.second.get(), dict2->GetObjectFor(item.first)))
             return false;
         }
         return true;
diff --git a/core/fpdfapi/parser/cpdf_object_walker.cpp b/core/fpdfapi/parser/cpdf_object_walker.cpp
index 6d6d489e02..f3e1062d83 100644
--- a/core/fpdfapi/parser/cpdf_object_walker.cpp
+++ b/core/fpdfapi/parser/cpdf_object_walker.cpp
@@ -37,11 +37,11 @@ class StreamIterator final : public CPDF_ObjectWalker::SubobjectIterator {
 class DictionaryIterator final : public CPDF_ObjectWalker::SubobjectIterator {
  public:
   explicit DictionaryIterator(const CPDF_Dictionary* dictionary)
-      : SubobjectIterator(dictionary) {}
+      : SubobjectIterator(dictionary), locker_(dictionary) {}
   ~DictionaryIterator() override {}
 
   bool IsFinished() const override {
-    return IsStarted() && dict_iterator_ == object()->GetDict()->end();
+    return IsStarted() && dict_iterator_ == locker_.end();
   }
 
   const CPDF_Object* IncrementImpl() override {
@@ -55,24 +55,26 @@ class DictionaryIterator final : public CPDF_ObjectWalker::SubobjectIterator {
 
   void Start() override {
     ASSERT(!IsStarted());
-    dict_iterator_ = object()->GetDict()->begin();
+    dict_iterator_ = locker_.begin();
   }
 
   ByteString dict_key() const { return dict_key_; }
 
  private:
   CPDF_Dictionary::const_iterator dict_iterator_;
+  CPDF_DictionaryLocker locker_;
   ByteString dict_key_;
 };
 
 class ArrayIterator final : public CPDF_ObjectWalker::SubobjectIterator {
  public:
-  explicit ArrayIterator(const CPDF_Array* array) : SubobjectIterator(array) {}
+  explicit ArrayIterator(const CPDF_Array* array)
+      : SubobjectIterator(array), locker_(array) {}
 
   ~ArrayIterator() override {}
 
   bool IsFinished() const override {
-    return IsStarted() && arr_iterator_ == object()->AsArray()->end();
+    return IsStarted() && arr_iterator_ == locker_.end();
   }
 
   const CPDF_Object* IncrementImpl() override {
@@ -83,10 +85,11 @@ class ArrayIterator final : public CPDF_ObjectWalker::SubobjectIterator {
     return result;
   }
 
-  void Start() override { arr_iterator_ = object()->AsArray()->begin(); }
+  void Start() override { arr_iterator_ = locker_.begin(); }
 
  public:
   CPDF_Array::const_iterator arr_iterator_;
+  CPDF_ArrayLocker locker_;
 };
 
 }  // namespace
diff --git a/core/fpdfapi/parser/fpdf_parser_utility.cpp b/core/fpdfapi/parser/fpdf_parser_utility.cpp
index 4cd02f009e..e6972984ea 100644
--- a/core/fpdfapi/parser/fpdf_parser_utility.cpp
+++ b/core/fpdfapi/parser/fpdf_parser_utility.cpp
@@ -192,9 +192,9 @@ std::ostream& operator<<(std::ostream& buf, const CPDF_Object* pObj) {
       break;
     }
     case CPDF_Object::DICTIONARY: {
-      const CPDF_Dictionary* p = pObj->AsDictionary();
+      CPDF_DictionaryLocker locker(pObj->AsDictionary());
       buf << "<<";
-      for (const auto& it : *p) {
+      for (const auto& it : locker) {
         const ByteString& key = it.first;
         CPDF_Object* pValue = it.second.get();
         buf << "/" << PDF_NameEncode(key);
diff --git a/core/fpdfdoc/cpdf_formcontrol.cpp b/core/fpdfdoc/cpdf_formcontrol.cpp
index edd5fe566c..7ff0914590 100644
--- a/core/fpdfdoc/cpdf_formcontrol.cpp
+++ b/core/fpdfdoc/cpdf_formcontrol.cpp
@@ -48,7 +48,8 @@ ByteString CPDF_FormControl::GetOnStateName() const {
   if (!pN)
     return csOn;
 
-  for (const auto& it : *pN) {
+  CPDF_DictionaryLocker locker(pN);
+  for (const auto& it : locker) {
     if (it.first != "Off")
       return it.first;
   }
diff --git a/core/fpdfdoc/cpdf_formfield.cpp b/core/fpdfdoc/cpdf_formfield.cpp
index 2b06161b06..89030b4132 100644
--- a/core/fpdfdoc/cpdf_formfield.cpp
+++ b/core/fpdfdoc/cpdf_formfield.cpp
@@ -796,7 +796,8 @@ bool CPDF_FormField::IsOptionSelected(int iOptIndex) const {
   if (!pArray)
     return false;
 
-  for (const auto& pObj : *pArray) {
+  CPDF_ArrayLocker locker(pArray);
+  for (const auto& pObj : locker) {
     if (pObj->GetInteger() == iOptIndex)
       return true;
   }
diff --git a/core/fpdfdoc/cpdf_interactiveform.cpp b/core/fpdfdoc/cpdf_interactiveform.cpp
index 403344c0ff..382d3dd314 100644
--- a/core/fpdfdoc/cpdf_interactiveform.cpp
+++ b/core/fpdfdoc/cpdf_interactiveform.cpp
@@ -154,7 +154,8 @@ CPDF_Font* GetNativeFont(CPDF_Dictionary* pFormDict,
   if (!pFonts)
     return nullptr;
 
-  for (const auto& it : *pFonts) {
+  CPDF_DictionaryLocker locker(pFonts);
+  for (const auto& it : locker) {
     const ByteString& csKey = it.first;
     if (!it.second)
       continue;
@@ -194,7 +195,8 @@ bool FindFont(CPDF_Dictionary* pFormDict,
   if (!pFonts)
     return false;
 
-  for (const auto& it : *pFonts) {
+  CPDF_DictionaryLocker locker(pFonts);
+  for (const auto& it : locker) {
     const ByteString& csKey = it.first;
     if (!it.second)
       continue;
@@ -230,7 +232,8 @@ bool FindFont(CPDF_Dictionary* pFormDict,
   if (csFontName.GetLength() > 0)
     csFontName.Remove(' ');
 
-  for (const auto& it : *pFonts) {
+  CPDF_DictionaryLocker locker(pFonts);
+  for (const auto& it : locker) {
     const ByteString& csKey = it.first;
     if (!it.second)
       continue;
diff --git a/fpdfsdk/formfiller/cba_fontmap.cpp b/fpdfsdk/formfiller/cba_fontmap.cpp
index f2e7c840f8..24d1ef9de9 100644
--- a/fpdfsdk/formfiller/cba_fontmap.cpp
+++ b/fpdfsdk/formfiller/cba_fontmap.cpp
@@ -117,7 +117,8 @@ CPDF_Font* CBA_FontMap::FindResFontSameCharset(const CPDF_Dictionary* pResDict,
 
   CPDF_Document* pDocument = GetDocument();
   CPDF_Font* pFind = nullptr;
-  for (const auto& it : *pFonts) {
+  CPDF_DictionaryLocker locker(pFonts);
+  for (const auto& it : locker) {
     const ByteString& csKey = it.first;
     if (!it.second)
       continue;
diff --git a/fpdfsdk/fpdf_annot.cpp b/fpdfsdk/fpdf_annot.cpp
index e4bfab0b70..2c8e68eca7 100644
--- a/fpdfsdk/fpdf_annot.cpp
+++ b/fpdfsdk/fpdf_annot.cpp
@@ -277,17 +277,18 @@ FPDF_EXPORT int FPDF_CALLCONV FPDFPage_GetAnnotIndex(FPDF_PAGE page,
   if (!pAnnots)
     return -1;
 
+  CPDF_ArrayLocker locker(pAnnots);
   CPDF_Dictionary* pDict = pAnnot->GetAnnotDict();
   auto it =
-      std::find_if(pAnnots->begin(), pAnnots->end(),
+      std::find_if(locker.begin(), locker.end(),
                    [pDict](const std::unique_ptr<CPDF_Object>& candidate) {
                      return candidate->GetDirect() == pDict;
                    });
 
-  if (it == pAnnots->end())
+  if (it == locker.end())
     return -1;
 
-  return it - pAnnots->begin();
+  return it - locker.begin();
 }
 
 FPDF_EXPORT void FPDF_CALLCONV FPDFPage_CloseAnnot(FPDF_ANNOTATION annot) {
diff --git a/fpdfsdk/fpdf_editpage.cpp b/fpdfsdk/fpdf_editpage.cpp
index 8c9a33d816..dcb3d5e1f0 100644
--- a/fpdfsdk/fpdf_editpage.cpp
+++ b/fpdfsdk/fpdf_editpage.cpp
@@ -389,7 +389,8 @@ FPDFPageObjMark_GetParamKey(FPDF_PAGEOBJECTMARK mark,
   if (!pParams)
     return false;
 
-  for (auto& it : *pParams) {
+  CPDF_DictionaryLocker locker(pParams);
+  for (auto& it : locker) {
     if (index == 0) {
       *out_buflen = Utf16EncodeMaybeCopyAndReturnLength(
           WideString::FromUTF8(it.first.AsStringView()), buffer, buflen);
diff --git a/fpdfsdk/fpdf_flatten.cpp b/fpdfsdk/fpdf_flatten.cpp
index de396667f5..98529c53c8 100644
--- a/fpdfsdk/fpdf_flatten.cpp
+++ b/fpdfsdk/fpdf_flatten.cpp
@@ -90,7 +90,8 @@ int ParserAnnots(CPDF_Document* pSourceDoc,
   if (!pAnnots)
     return FLATTEN_NOTHINGTODO;
 
-  for (const auto& pAnnot : *pAnnots) {
+  CPDF_ArrayLocker locker(pAnnots);
+  for (const auto& pAnnot : locker) {
     CPDF_Dictionary* pAnnotDic = ToDictionary(pAnnot->GetDirect());
     if (!pAnnotDic)
       continue;
@@ -338,7 +339,8 @@ FPDF_EXPORT int FPDF_CALLCONV FPDFPage_Flatten(FPDF_PAGE page, int nFlag) {
         pAPStream = pAPDic->GetStreamFor(sAnnotState);
       } else {
         if (pAPDic->size() > 0) {
-          CPDF_Object* pFirstObj = pAPDic->begin()->second.get();
+          CPDF_DictionaryLocker locker(pAPDic);
+          CPDF_Object* pFirstObj = locker.begin()->second.get();
           if (pFirstObj) {
             if (pFirstObj->IsReference())
               pFirstObj = pFirstObj->GetDirect();
diff --git a/fpdfsdk/fpdf_ppo.cpp b/fpdfsdk/fpdf_ppo.cpp
index fd270d997c..5c5930e097 100644
--- a/fpdfsdk/fpdf_ppo.cpp
+++ b/fpdfsdk/fpdf_ppo.cpp
@@ -360,18 +360,22 @@ bool CPDF_PageOrganizer::UpdateReference(CPDF_Object* pObj,
     }
     case CPDF_Object::DICTIONARY: {
       CPDF_Dictionary* pDict = pObj->AsDictionary();
-      auto it = pDict->begin();
-      while (it != pDict->end()) {
-        const ByteString& key = it->first;
-        CPDF_Object* pNextObj = it->second.get();
-        ++it;
-        if (key == "Parent" || key == "Prev" || key == "First")
-          continue;
-        if (!pNextObj)
-          return false;
-        if (!UpdateReference(pNextObj, pObjNumberMap))
-          pDict->RemoveFor(key);
+      std::vector<ByteString> bad_keys;
+      {
+        CPDF_DictionaryLocker locker(pDict);
+        for (auto it = locker.begin(); it != locker.end(); ++it) {
+          const ByteString& key = it->first;
+          if (key == "Parent" || key == "Prev" || key == "First")
+            continue;
+          CPDF_Object* pNextObj = it->second.get();
+          if (!pNextObj)
+            return false;
+          if (!UpdateReference(pNextObj, pObjNumberMap))
+            bad_keys.push_back(key);
+        }
       }
+      for (const auto& key : bad_keys)
+        pDict->RemoveFor(key);
       break;
     }
     case CPDF_Object::ARRAY: {
@@ -471,7 +475,8 @@ bool CPDF_PageExporter::ExportPage(const std::vector<uint32_t>& pageNums,
       return false;
 
     // Clone the page dictionary
-    for (const auto& it : *pSrcPageDict) {
+    CPDF_DictionaryLocker locker(pSrcPageDict);
+    for (const auto& it : locker) {
       const ByteString& cbSrcKeyStr = it.first;
       if (cbSrcKeyStr == pdfium::page_object::kType ||
           cbSrcKeyStr == pdfium::page_object::kParent) {
diff --git a/fpdfsdk/fpdf_transformpage.cpp b/fpdfsdk/fpdf_transformpage.cpp
index 8e8c9d5b70..debf38b47e 100644
--- a/fpdfsdk/fpdf_transformpage.cpp
+++ b/fpdfsdk/fpdf_transformpage.cpp
@@ -219,11 +219,12 @@ FPDFPage_TransFormWithClip(FPDF_PAGE page,
   if (!pRes)
     return true;
 
-  CPDF_Dictionary* pPattenDict = pRes->GetDictFor("Pattern");
-  if (!pPattenDict)
+  CPDF_Dictionary* pPatternDict = pRes->GetDictFor("Pattern");
+  if (!pPatternDict)
     return true;
 
-  for (const auto& it : *pPattenDict) {
+  CPDF_DictionaryLocker locker(pPatternDict);
+  for (const auto& it : locker) {
     CPDF_Object* pObj = it.second.get();
     if (pObj->IsReference())
       pObj = pObj->GetDirect();
diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp
index 83d7e4a6cb..136841d566 100644
--- a/fpdfsdk/fpdf_view.cpp
+++ b/fpdfsdk/fpdf_view.cpp
@@ -1152,7 +1152,8 @@ FPDF_EXPORT FPDF_DEST FPDF_CALLCONV FPDF_GetNamedDest(FPDF_DOCUMENT document,
     index -= count;
     int i = 0;
     ByteString bsName;
-    for (const auto& it : *pDest) {
+    CPDF_DictionaryLocker locker(pDest);
+    for (const auto& it : locker) {
       bsName = it.first;
       pDestObj = it.second.get();
       if (!pDestObj)
diff --git a/fxjs/cjs_document.cpp b/fxjs/cjs_document.cpp
index adf36be403..2ecfd424d2 100644
--- a/fxjs/cjs_document.cpp
+++ b/fxjs/cjs_document.cpp
@@ -742,7 +742,8 @@ CJS_Result CJS_Document::get_info(CJS_Runtime* pRuntime) {
 
   // PutObjectProperty() calls below may re-enter JS and change info dict.
   auto pCopy = pDictionary->Clone();
-  for (const auto& it : *ToDictionary(pCopy.get())) {
+  CPDF_DictionaryLocker locker(ToDictionary(pCopy.get()));
+  for (const auto& it : locker) {
     const ByteString& bsKey = it.first;
     CPDF_Object* pValueObj = it.second.get();
     WideString wsKey = WideString::FromUTF8(bsKey.AsStringView());
diff --git a/xfa/fgas/font/cfgas_pdffontmgr.cpp b/xfa/fgas/font/cfgas_pdffontmgr.cpp
index fe4b83ce4c..c1d6ada7a9 100644
--- a/xfa/fgas/font/cfgas_pdffontmgr.cpp
+++ b/xfa/fgas/font/cfgas_pdffontmgr.cpp
@@ -48,7 +48,9 @@ RetainPtr<CFGAS_GEFont> CFGAS_PDFFontMgr::FindFont(const ByteString& strPsName,
 
   ByteString name = strPsName;
   name.Remove(' ');
-  for (const auto& it : *pFontSetDict) {
+
+  CPDF_DictionaryLocker locker(pFontSetDict);
+  for (const auto& it : locker) {
     const ByteString& key = it.first;
     CPDF_Object* pObj = it.second.get();
     if (!PsNameMatchDRFontName(name.AsStringView(), bBold, bItalic, key,