commit 678f5418d36f553f4a908e51a3e5e3fcf5c66a16
author Tom Sepez <tsepez@chromium.org> Thu Aug 30 19:15:49 2018 +0000
committer Chromium commit bot <commit-bot@chromium.org> Thu Aug 30 19:15:49 2018 +0000
tree 057d5cfaf46fba8c88c4c5eb0c9fa00c699874c0
parent e70aff80976ebe8b8a7bc24f2ed2115b681c33c2

Fix lifetime issue in CCodec_ProgressiveDecoder.

The associated bug is caused by failure to clean up context before
returing at line 740 or so. But cleanups are fragile, so instead
do not move context out of a local until success. This in turn
means passing the local to one helper rather that using the stored
value and confirming this is the only helper method invoked during
the time in which the context is not yet stored.

Bug: 879025
Change-Id: I1acb438edc28f7f4dd8acccc2461fe3a5ea7f5c8
Reviewed-on: https://pdfium-review.googlesource.com/41670
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>

core/fxcodec/codec/ccodec_progressivedecoder.cpp

diff --git a/core/fxcodec/codec/ccodec_progressivedecoder.cpp b/core/fxcodec/codec/ccodec_progressivedecoder.cpp
index 3426c66..6faaaa1 100644
--- a/core/fxcodec/codec/ccodec_progressivedecoder.cpp
+++ b/core/fxcodec/codec/ccodec_progressivedecoder.cpp
@@ -579,7 +579,8 @@
 bool CCodec_ProgressiveDecoder::BmpInputImagePositionBuf(uint32_t rcd_pos) {
   m_offSet = rcd_pos;
   FXCODEC_STATUS error_status = FXCODEC_STATUS_ERROR;
-  return BmpReadMoreData(m_pCodecMgr->GetBmpModule(), error_status);
+  return BmpReadMoreData(m_pCodecMgr->GetBmpModule(), m_pBmpContext.get(),
+                         error_status);
 }
 
 void CCodec_ProgressiveDecoder::BmpReadScanline(
@@ -715,8 +716,9 @@
     return false;
   }
 
-  m_pBmpContext = pBmpModule->Start(this);
-  if (!m_pBmpContext) {
+  std::unique_ptr<CCodec_BmpModule::Context> pBmpContext =
+      pBmpModule->Start(this);
+  if (!pBmpContext) {
     m_status = FXCODEC_STATUS_ERR_MEMORY;
     return false;
   }
@@ -727,24 +729,23 @@
   }
 
   m_offSet += size;
-  pBmpModule->Input(m_pBmpContext.get(), {m_pSrcBuf.get(), size});
+  pBmpModule->Input(pBmpContext.get(), {m_pSrcBuf.get(), size});
   std::vector<uint32_t> palette;
   int32_t readResult = pBmpModule->ReadHeader(
-      m_pBmpContext.get(), &m_SrcWidth, &m_SrcHeight, &m_BmpIsTopBottom,
+      pBmpContext.get(), &m_SrcWidth, &m_SrcHeight, &m_BmpIsTopBottom,
       &m_SrcComponents, &m_SrcPaletteNumber, &palette, pAttribute);
   while (readResult == 2) {
     FXCODEC_STATUS error_status = FXCODEC_STATUS_ERR_FORMAT;
-    if (!BmpReadMoreData(pBmpModule, error_status)) {
+    if (!BmpReadMoreData(pBmpModule, pBmpContext.get(), error_status)) {
       m_status = error_status;
       return false;
     }
     readResult = pBmpModule->ReadHeader(
-        m_pBmpContext.get(), &m_SrcWidth, &m_SrcHeight, &m_BmpIsTopBottom,
+        pBmpContext.get(), &m_SrcWidth, &m_SrcHeight, &m_BmpIsTopBottom,
         &m_SrcComponents, &m_SrcPaletteNumber, &palette, pAttribute);
   }
 
   if (readResult != 1) {
-    m_pBmpContext.reset();
     m_status = FXCODEC_STATUS_ERR_FORMAT;
     return false;
   }
@@ -764,7 +765,6 @@
       format = FXDIB_Rgb32;
       break;
     default:
-      m_pBmpContext.reset();
       m_status = FXCODEC_STATUS_ERR_FORMAT;
       return false;
   }
@@ -773,20 +773,19 @@
   uint32_t neededData = 0;
   if (!CFX_DIBitmap::CalculatePitchAndSize(m_SrcWidth, m_SrcHeight, format,
                                            &pitch, &neededData)) {
-    m_pBmpContext.reset();
     m_status = FXCODEC_STATUS_ERR_FORMAT;
     return false;
   }
 
   uint32_t availableData = m_SrcSize > m_offSet ? m_SrcSize - m_offSet : 0;
   if (neededData > availableData) {
-    m_pBmpContext.reset();
     m_status = FXCODEC_STATUS_ERR_FORMAT;
     return false;
   }
 
   m_SrcBPC = 8;
   m_clipBox = FX_RECT(0, 0, m_SrcWidth, m_SrcHeight);
+  m_pBmpContext = std::move(pBmpContext);
   if (m_SrcPaletteNumber) {
     m_pSrcPalette.reset(FX_Alloc(FX_ARGB, m_SrcPaletteNumber));
     memcpy(m_pSrcPalette.get(), palette.data(),
@@ -797,15 +796,16 @@
   return true;
 }
 
-bool CCodec_ProgressiveDecoder::BmpReadMoreData(CCodec_BmpModule* pBmpModule,
-                                                FXCODEC_STATUS& err_status) {
+bool CCodec_ProgressiveDecoder::BmpReadMoreData(
+    CCodec_BmpModule* pBmpModule,
+    CCodec_BmpModule::Context* pBmpContext,
+    FXCODEC_STATUS& err_status) {
   uint32_t dwSize = (uint32_t)m_pFile->GetSize();
   if (dwSize <= m_offSet)
     return false;
 
   dwSize = dwSize - m_offSet;
-  FX_SAFE_UINT32 avail_input =
-      pBmpModule->GetAvailInput(m_pBmpContext.get(), nullptr);
+  FX_SAFE_UINT32 avail_input = pBmpModule->GetAvailInput(pBmpContext, nullptr);
   if (!avail_input.IsValid())
     return false;
 
@@ -835,7 +835,7 @@
     return false;
   }
   m_offSet += dwSize;
-  pBmpModule->Input(m_pBmpContext.get(), {m_pSrcBuf.get(), dwSize + dwAvail});
+  pBmpModule->Input(pBmpContext, {m_pSrcBuf.get(), dwSize + dwAvail});
   return true;
 }
 
@@ -869,7 +869,7 @@
     int32_t readRes = pBmpModule->LoadImage(m_pBmpContext.get());
     while (readRes == 2) {
       FXCODEC_STATUS error_status = FXCODEC_STATUS_DECODE_FINISH;
-      if (!BmpReadMoreData(pBmpModule, error_status)) {
+      if (!BmpReadMoreData(pBmpModule, m_pBmpContext.get(), error_status)) {
         m_pDeviceBitmap = nullptr;
         m_pFile = nullptr;
         m_status = error_status;

core/fxcodec/codec/ccodec_progressivedecoder.h

diff --git a/core/fxcodec/codec/ccodec_progressivedecoder.h b/core/fxcodec/codec/ccodec_progressivedecoder.h
index 4216a84..1f72d9f 100644
--- a/core/fxcodec/codec/ccodec_progressivedecoder.h
+++ b/core/fxcodec/codec/ccodec_progressivedecoder.h
@@ -182,6 +182,7 @@
  private:
 #ifdef PDF_ENABLE_XFA_BMP
   bool BmpReadMoreData(CCodec_BmpModule* pBmpModule,
+                       CCodec_BmpModule::Context* pBmpContext,
                        FXCODEC_STATUS& err_status);
   bool BmpDetectImageType(CFX_DIBAttribute* pAttribute, uint32_t size);
   FXCODEC_STATUS BmpStartDecode(const RetainPtr<CFX_DIBitmap>& pDIBitmap);