Fix heap use after free in Document::DoFieldDelay and Document::delay
authorTom Sepez <tsepez@chromium.org>
Tue, 2 Jun 2015 17:09:49 +0000 (10:09 -0700)
committerTom Sepez <tsepez@chromium.org>
Tue, 2 Jun 2015 17:09:49 +0000 (10:09 -0700)
This fix removes CJS_DelayData object from m_DelayData array and copies them to
a new array, before processing them. So contents of m_DelayData array cannot be
used after they get freed.

BUG=487928

R=tsepez@chromium.org

TEST= Chrome pdf plugin should not crash when poc_stable,testuafdocument1.pdf
      and testuafdocument2.pdf are viewed.
      see crbug.com/487928 and crbug.com/487928#c18 for more details.

Review URL: https://codereview.chromium.org/1163823002

fpdfsdk/src/javascript/Document.cpp

index b821f1e..6a3d6e1 100644 (file)
@@ -988,15 +988,25 @@ FX_BOOL Document::delay(IFXJS_Context* cc, CJS_PropValue& vp, CFX_WideString& sE
                }
                else
                {
-                       for (int i=0,sz=m_DelayData.GetSize(); i<sz; i++)
+                       CFX_ArrayTemplate<CJS_DelayData*> DelayDataToProcess;
+
+                       for (int i=0,sz=m_DelayData.GetSize(); i < sz; i++)
                        {
                                if (CJS_DelayData* pData = m_DelayData.GetAt(i))
                                {
-                                       Field::DoDelay(m_pDocument, pData);
-                                       delete m_DelayData.GetAt(i);
+                                       DelayDataToProcess.Add(pData);
+                                       m_DelayData.SetAt(i, NULL);
                                }
                        }
                        m_DelayData.RemoveAll();
+
+                       for (int i=0,sz=DelayDataToProcess.GetSize(); i < sz; i++)
+                       {
+                               CJS_DelayData* pData = DelayDataToProcess.GetAt(i);
+                               Field::DoDelay(m_pDocument, pData);
+                               DelayDataToProcess.SetAt(i,NULL);
+                               delete pData;
+                       }
                }
 
                return TRUE;
@@ -1927,6 +1937,7 @@ void Document::AddDelayData(CJS_DelayData* pData)
 void Document::DoFieldDelay(const CFX_WideString& sFieldName, int nControlIndex)
 {
        CFX_DWordArray DelArray;
+       CFX_ArrayTemplate<CJS_DelayData*> DelayDataForFieldAndControlIndex;
 
        for (int i=0,sz=m_DelayData.GetSize(); i<sz; i++)
        {
@@ -1934,8 +1945,7 @@ void Document::DoFieldDelay(const CFX_WideString& sFieldName, int nControlIndex)
                {
                        if (pData->sFieldName == sFieldName && pData->nControlIndex == nControlIndex)
                        {
-                               Field::DoDelay(m_pDocument, pData);
-                               delete pData;
+                               DelayDataForFieldAndControlIndex.Add(pData);
                                m_DelayData.SetAt(i, NULL);
                                DelArray.Add(i);
                        }
@@ -1946,6 +1956,14 @@ void Document::DoFieldDelay(const CFX_WideString& sFieldName, int nControlIndex)
        {
                m_DelayData.RemoveAt(DelArray[j]);
        }
+
+       for (int i=0,sz=DelayDataForFieldAndControlIndex.GetSize(); i < sz; i++)
+       {
+               CJS_DelayData* pData = DelayDataForFieldAndControlIndex.GetAt(i);
+               Field::DoDelay(m_pDocument, pData);
+               DelayDataForFieldAndControlIndex.SetAt(i,NULL);
+               delete pData;
+       }
 }
 
 void Document::AddDelayAnnotData(CJS_AnnotObj *pData)