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>

7 files changed, 35 insertions, 23 deletions

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)