From 35c2100a5f6a466635bf99b4e7117d23aeb54d2c Mon Sep 17 00:00:00 2001
From: thestig <thestig@chromium.org>
Date: Wed, 11 May 2016 12:59:05 -0700
Subject: Fix a potential UAF with FPDFAvail_IsLinearized().

Cache the linearized result rather than recalculating it.

BUG=608778

Review-Url: https://codereview.chromium.org/1968743002
---
 core/fpdfapi/fpdf_parser/cpdf_data_avail.cpp | 57 ++++++++++++++--------------
 core/fpdfapi/fpdf_parser/cpdf_hint_tables.h  | 12 ++++--
 2 files changed, 36 insertions(+), 33 deletions(-)

(limited to 'core/fpdfapi')

diff --git a/core/fpdfapi/fpdf_parser/cpdf_data_avail.cpp b/core/fpdfapi/fpdf_parser/cpdf_data_avail.cpp
index f210c49c71..e47d4244ac 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_data_avail.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_data_avail.cpp
@@ -89,6 +89,7 @@ CPDF_DataAvail::CPDF_DataAvail(IPDF_DataAvail::FileAvail* pFileAvail,
   m_bSupportHintTable = bSupportHintTable;
 }
 CPDF_DataAvail::~CPDF_DataAvail() {
+  m_pHintTables.reset();
   if (m_pLinearized)
     m_pLinearized->Release();
 
@@ -620,15 +621,14 @@ FX_BOOL CPDF_DataAvail::CheckPages(IPDF_DataAvail::DownloadHints* pHints) {
 }
 
 FX_BOOL CPDF_DataAvail::CheckHeader(IPDF_DataAvail::DownloadHints* pHints) {
-  uint32_t req_size = 1024;
-  if ((FX_FILESIZE)req_size > m_dwFileLen)
-    req_size = (uint32_t)m_dwFileLen;
+  ASSERT(m_dwFileLen >= 0);
+  const uint32_t kReqSize = std::min(static_cast<uint32_t>(m_dwFileLen), 1024U);
 
-  if (m_pFileAvail->IsDataAvail(0, req_size)) {
+  if (m_pFileAvail->IsDataAvail(0, kReqSize)) {
     uint8_t buffer[1024];
-    m_pFileRead->ReadBlock(buffer, 0, req_size);
+    m_pFileRead->ReadBlock(buffer, 0, kReqSize);
 
-    if (IsLinearizedFile(buffer, req_size)) {
+    if (IsLinearizedFile(buffer, kReqSize)) {
       m_docStatus = PDF_DATAAVAIL_FIRSTPAGE;
     } else {
       if (m_docStatus == PDF_DATAAVAIL_ERROR)
@@ -638,7 +638,7 @@ FX_BOOL CPDF_DataAvail::CheckHeader(IPDF_DataAvail::DownloadHints* pHints) {
     return TRUE;
   }
 
-  pHints->AddSegment(0, req_size);
+  pHints->AddSegment(0, kReqSize);
   return FALSE;
 }
 
@@ -813,25 +813,29 @@ CPDF_Object* CPDF_DataAvail::ParseIndirectObjectAt(
 }
 
 IPDF_DataAvail::DocLinearizationStatus CPDF_DataAvail::IsLinearizedPDF() {
-  uint32_t req_size = 1024;
-  if (!m_pFileAvail->IsDataAvail(0, req_size))
+  const uint32_t kReqSize = 1024;
+  if (!m_pFileAvail->IsDataAvail(0, kReqSize))
     return LinearizationUnknown;
 
   if (!m_pFileRead)
     return NotLinearized;
 
   FX_FILESIZE dwSize = m_pFileRead->GetSize();
-  if (dwSize < (FX_FILESIZE)req_size)
+  if (dwSize < (FX_FILESIZE)kReqSize)
     return LinearizationUnknown;
 
   uint8_t buffer[1024];
-  m_pFileRead->ReadBlock(buffer, 0, req_size);
-  if (IsLinearizedFile(buffer, req_size))
+  m_pFileRead->ReadBlock(buffer, 0, kReqSize);
+  if (IsLinearizedFile(buffer, kReqSize))
     return Linearized;
 
   return NotLinearized;
 }
+
 FX_BOOL CPDF_DataAvail::IsLinearizedFile(uint8_t* pData, uint32_t dwLen) {
+  if (m_pLinearized)
+    return m_bLinearized;
+
   ScopedFileStream file(FX_CreateMemoryStream(pData, (size_t)dwLen, FALSE));
 
   int32_t offset = GetHeaderOffset(file.get());
@@ -850,33 +854,28 @@ FX_BOOL CPDF_DataAvail::IsLinearizedFile(uint8_t* pData, uint32_t dwLen) {
     return FALSE;
 
   uint32_t objnum = FXSYS_atoui(wordObjNum.c_str());
-  if (m_pLinearized) {
-    m_pLinearized->Release();
-    m_pLinearized = nullptr;
-  }
-
   m_pLinearized =
       ParseIndirectObjectAt(m_syntaxParser.m_HeaderOffset + 9, objnum);
   if (!m_pLinearized)
     return FALSE;
 
   CPDF_Dictionary* pDict = m_pLinearized->GetDict();
-  if (pDict && pDict->GetObjectBy("Linearized")) {
-    CPDF_Object* pLen = pDict->GetObjectBy("L");
-    if (!pLen)
-      return FALSE;
+  if (!pDict || !pDict->GetObjectBy("Linearized"))
+    return FALSE;
 
-    if ((FX_FILESIZE)pLen->GetInteger() != m_pFileRead->GetSize())
-      return FALSE;
+  CPDF_Object* pLen = pDict->GetObjectBy("L");
+  if (!pLen)
+    return FALSE;
 
-    m_bLinearized = TRUE;
+  if ((FX_FILESIZE)pLen->GetInteger() != m_pFileRead->GetSize())
+    return FALSE;
 
-    if (CPDF_Number* pNo = ToNumber(pDict->GetObjectBy("P")))
-      m_dwFirstPageNo = pNo->GetInteger();
+  m_bLinearized = TRUE;
 
-    return TRUE;
-  }
-  return FALSE;
+  if (CPDF_Number* pNo = ToNumber(pDict->GetObjectBy("P")))
+    m_dwFirstPageNo = pNo->GetInteger();
+
+  return TRUE;
 }
 
 FX_BOOL CPDF_DataAvail::CheckEnd(IPDF_DataAvail::DownloadHints* pHints) {
diff --git a/core/fpdfapi/fpdf_parser/cpdf_hint_tables.h b/core/fpdfapi/fpdf_parser/cpdf_hint_tables.h
index 28ccccb6fc..33b6b39323 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_hint_tables.h
+++ b/core/fpdfapi/fpdf_parser/cpdf_hint_tables.h
@@ -21,8 +21,8 @@ class CPDF_Stream;
 class CPDF_HintTables {
  public:
   CPDF_HintTables(CPDF_DataAvail* pDataAvail, CPDF_Dictionary* pLinearized)
-      : m_pLinearizedDict(pLinearized),
-        m_pDataAvail(pDataAvail),
+      : m_pDataAvail(pDataAvail),
+        m_pLinearizedDict(pLinearized),
         m_nFirstPageSharedObjs(0),
         m_szFirstPageObjOffset(0) {}
   ~CPDF_HintTables();
@@ -47,8 +47,12 @@ class CPDF_HintTables {
   int ReadPrimaryHintStreamOffset() const;
   int ReadPrimaryHintStreamLength() const;
 
-  CPDF_Dictionary* m_pLinearizedDict;
-  CPDF_DataAvail* m_pDataAvail;
+  // Owner, outlives this object.
+  CPDF_DataAvail* const m_pDataAvail;
+
+  // Owned by |m_pDataAvail|.
+  CPDF_Dictionary* const m_pLinearizedDict;
+
   uint32_t m_nFirstPageSharedObjs;
   FX_FILESIZE m_szFirstPageObjOffset;
   CFX_ArrayTemplate<uint32_t> m_dwDeltaNObjsArray;
-- 
cgit v1.2.3