Prevent cxfa_contentlayoutitem linked lists from getting entangled.

Implement the basic linked-list primitives, and use them
consistently. Currently the code is doing ad-hoc manipulations
of these pointers, and creating circular lists somewhere. This
causes re-frees as we walk down the list freeing every item.

Use UnownedPtr<> to try to catch any future botches.

Tested against the test case in 925790 (CF fuzzer will subsequently
verify 913564).

Bug: chromium:913564, chromium:925790
Change-Id: I2b735b3137aa715e5bb6b1c4472a1d2fd68ae286
Reviewed-on: https://pdfium-review.googlesource.com/c/49350
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>

xfa/fxfa/parser/cxfa_contentlayoutitem.cpp

diff --git a/xfa/fxfa/parser/cxfa_contentlayoutitem.cpp b/xfa/fxfa/parser/cxfa_contentlayoutitem.cpp
index 6878c8e..79b3cb7 100644
--- a/xfa/fxfa/parser/cxfa_contentlayoutitem.cpp
+++ b/xfa/fxfa/parser/cxfa_contentlayoutitem.cpp
@@ -14,6 +14,7 @@
     : CXFA_LayoutItem(pNode, kContentItem) {}
 
 CXFA_ContentLayoutItem::~CXFA_ContentLayoutItem() {
+  RemoveSelf();
   if (m_pFormNode->JSObject()->GetLayoutItem() == this)
     m_pFormNode->JSObject()->SetLayoutItem(nullptr);
 }
@@ -24,20 +25,36 @@
 
 CXFA_ContentLayoutItem* CXFA_ContentLayoutItem::GetFirst() {
   CXFA_ContentLayoutItem* pCurNode = this;
-  while (pCurNode->m_pPrev)
-    pCurNode = pCurNode->m_pPrev;
+  while (auto* pPrev = pCurNode->GetPrev())
+    pCurNode = pPrev;
 
   return pCurNode;
 }
 
 CXFA_ContentLayoutItem* CXFA_ContentLayoutItem::GetLast() {
   CXFA_ContentLayoutItem* pCurNode = this;
-  while (pCurNode->m_pNext)
-    pCurNode = pCurNode->m_pNext;
+  while (auto* pNext = pCurNode->GetNext())
+    pCurNode = pNext;
 
   return pCurNode;
 }
 
+void CXFA_ContentLayoutItem::InsertAfter(CXFA_ContentLayoutItem* pItem) {
+  pItem->RemoveSelf();
+  pItem->m_pNext = m_pNext;
+  pItem->m_pPrev = this;
+  m_pNext = pItem;
+  if (pItem->m_pNext)
+    pItem->m_pNext->m_pPrev = pItem;
+}
+
+void CXFA_ContentLayoutItem::RemoveSelf() {
+  if (m_pNext)
+    m_pNext->m_pPrev = m_pPrev;
+  if (m_pPrev)
+    m_pPrev->m_pNext = m_pNext;
+}
+
 CFX_RectF CXFA_ContentLayoutItem::GetRect(bool bRelative) const {
   CFX_PointF sPos = m_sPos;
   CFX_SizeF sSize = m_sSize;
@@ -83,8 +100,8 @@
 int32_t CXFA_ContentLayoutItem::GetIndex() const {
   int32_t iIndex = 0;
   const CXFA_ContentLayoutItem* pCurNode = this;
-  while (pCurNode->m_pPrev) {
-    pCurNode = pCurNode->m_pPrev;
+  while (auto* pPrev = pCurNode->GetPrev()) {
+    pCurNode = pPrev;
     ++iIndex;
   }
   return iIndex;
@@ -93,8 +110,8 @@
 int32_t CXFA_ContentLayoutItem::GetCount() const {
   int32_t iCount = GetIndex() + 1;
   const CXFA_ContentLayoutItem* pCurNode = this;
-  while (pCurNode->m_pNext) {
-    pCurNode = pCurNode->m_pNext;
+  while (auto* pNext = pCurNode->GetNext()) {
+    pCurNode = pNext;
     iCount++;
   }
   return iCount;

xfa/fxfa/parser/cxfa_contentlayoutitem.h

diff --git a/xfa/fxfa/parser/cxfa_contentlayoutitem.h b/xfa/fxfa/parser/cxfa_contentlayoutitem.h
index ac2d8f2..7313a1e 100644
--- a/xfa/fxfa/parser/cxfa_contentlayoutitem.h
+++ b/xfa/fxfa/parser/cxfa_contentlayoutitem.h
@@ -7,6 +7,7 @@
 #ifndef XFA_FXFA_PARSER_CXFA_CONTENTLAYOUTITEM_H_
 #define XFA_FXFA_PARSER_CXFA_CONTENTLAYOUTITEM_H_
 
+#include "core/fxcrt/unowned_ptr.h"
 #include "xfa/fxfa/parser/cxfa_layoutitem.h"
 
 class CXFA_FFWidget;
@@ -20,18 +21,23 @@
 
   CXFA_ContentLayoutItem* GetFirst();
   CXFA_ContentLayoutItem* GetLast();
-  CXFA_ContentLayoutItem* GetPrev() const { return m_pPrev; }
-  CXFA_ContentLayoutItem* GetNext() const { return m_pNext; }
+  CXFA_ContentLayoutItem* GetPrev() const { return m_pPrev.Get(); }
+  CXFA_ContentLayoutItem* GetNext() const { return m_pNext.Get(); }
+  void InsertAfter(CXFA_ContentLayoutItem* pNext);
 
   CFX_RectF GetRect(bool bRelative) const;
   int32_t GetIndex() const;
   int32_t GetCount() const;
 
-  CXFA_ContentLayoutItem* m_pPrev = nullptr;
-  CXFA_ContentLayoutItem* m_pNext = nullptr;
   CFX_PointF m_sPos;
   CFX_SizeF m_sSize;
   mutable uint32_t m_dwStatus = 0;
+
+ private:
+  void RemoveSelf();
+
+  UnownedPtr<CXFA_ContentLayoutItem> m_pPrev;
+  UnownedPtr<CXFA_ContentLayoutItem> m_pNext;
 };
 
 inline CXFA_FFWidget* ToFFWidget(CXFA_ContentLayoutItem* item) {

xfa/fxfa/parser/cxfa_itemlayoutprocessor.cpp

diff --git a/xfa/fxfa/parser/cxfa_itemlayoutprocessor.cpp b/xfa/fxfa/parser/cxfa_itemlayoutprocessor.cpp
index db24066..f91f3f0 100644
--- a/xfa/fxfa/parser/cxfa_itemlayoutprocessor.cpp
+++ b/xfa/fxfa/parser/cxfa_itemlayoutprocessor.cpp
@@ -444,7 +444,7 @@
     CXFA_ContentLayoutItem* pCurLayoutItem =
         ToContentLayoutItem(pNode->JSObject()->GetLayoutItem());
     while (pCurLayoutItem) {
-      CXFA_ContentLayoutItem* pNextLayoutItem = pCurLayoutItem->m_pNext;
+      CXFA_ContentLayoutItem* pNextLayoutItem = pCurLayoutItem->GetNext();
       pNotify->OnLayoutItemRemoving(pDocLayout, pCurLayoutItem);
       delete pCurLayoutItem;
       pCurLayoutItem = pNextLayoutItem;
@@ -643,7 +643,7 @@
   CXFA_ContentLayoutItem* pLayoutItem = nullptr;
   if (m_pOldLayoutItem) {
     pLayoutItem = m_pOldLayoutItem;
-    m_pOldLayoutItem = m_pOldLayoutItem->m_pNext;
+    m_pOldLayoutItem = m_pOldLayoutItem->GetNext();
     return pLayoutItem;
   }
   pLayoutItem = pFormNode->GetDocument()
@@ -653,11 +653,7 @@
   CXFA_ContentLayoutItem* pPrevLayoutItem =
       ToContentLayoutItem(pFormNode->JSObject()->GetLayoutItem());
   if (pPrevLayoutItem) {
-    while (pPrevLayoutItem->m_pNext)
-      pPrevLayoutItem = pPrevLayoutItem->m_pNext;
-
-    pPrevLayoutItem->m_pNext = pLayoutItem;
-    pLayoutItem->m_pPrev = pPrevLayoutItem;
+    pPrevLayoutItem->GetLast()->InsertAfter(pLayoutItem);
   } else {
     pFormNode->JSObject()->SetLayoutItem(pLayoutItem);
   }
@@ -818,16 +814,13 @@
     return pLayoutItem;
   }
 
-  if (m_pOldLayoutItem->m_pPrev)
-    m_pOldLayoutItem->m_pPrev->m_pNext = nullptr;
-
   CXFA_FFNotify* pNotify =
       m_pOldLayoutItem->GetFormNode()->GetDocument()->GetNotify();
   CXFA_LayoutProcessor* pDocLayout =
       m_pOldLayoutItem->GetFormNode()->GetDocument()->GetLayoutProcessor();
   CXFA_ContentLayoutItem* pOldLayoutItem = m_pOldLayoutItem;
   while (pOldLayoutItem) {
-    CXFA_ContentLayoutItem* pNextOldLayoutItem = pOldLayoutItem->m_pNext;
+    CXFA_ContentLayoutItem* pNextOldLayoutItem = pOldLayoutItem->GetNext();
     pNotify->OnLayoutItemRemoving(pDocLayout, pOldLayoutItem);
     if (pOldLayoutItem->m_pParent)
       pOldLayoutItem->m_pParent->RemoveChild(pOldLayoutItem);
@@ -1725,7 +1718,7 @@
         if (!pLayoutNext->m_pNextSibling && m_pCurChildPreprocessor &&
             m_pCurChildPreprocessor->GetFormNode() ==
                 pLayoutNext->GetFormNode()) {
-          pLayoutNext->m_pNext = m_pCurChildPreprocessor->m_pLayoutItem;
+          pLayoutNext->InsertAfter(m_pCurChildPreprocessor->m_pLayoutItem);
           m_pCurChildPreprocessor->m_pLayoutItem = pLayoutNext;
           break;
         }