commit b2ae83e8a2b0ed945f931338d6aa7e10ebb1fae0
author Lei Zhang <thestig@chromium.org> Tue Jan 08 20:41:10 2019 +0000
committer Chromium commit bot <commit-bot@chromium.org> Tue Jan 08 20:41:10 2019 +0000
tree 7ef5e29f1746ceed0d4e48eb77f4281fdd600843
parent 7a16671bd8039f10f566d3bc6e06c51b859b9190

Avoid crashing inside FPDF_LoadCustomDocument().

Also change CPDFSDK_CustomAccess to use the provided FPDF_FILEACCESS
instead of making a copy.

Change-Id: I07a3e6b9e6af721d56e0b041c37ed373cff4acf3
Reviewed-on: https://pdfium-review.googlesource.com/c/47550
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>

fpdfsdk/cpdfsdk_customaccess.cpp

diff --git a/fpdfsdk/cpdfsdk_customaccess.cpp b/fpdfsdk/cpdfsdk_customaccess.cpp
index 1002085..b679858 100644
--- a/fpdfsdk/cpdfsdk_customaccess.cpp
+++ b/fpdfsdk/cpdfsdk_customaccess.cpp
@@ -7,12 +7,14 @@
 #include "fpdfsdk/cpdfsdk_customaccess.h"
 
 CPDFSDK_CustomAccess::CPDFSDK_CustomAccess(FPDF_FILEACCESS* pFileAccess)
-    : m_FileAccess(*pFileAccess) {}
+    : m_pFileAccess(pFileAccess) {
+  ASSERT(m_pFileAccess);
+}
 
 CPDFSDK_CustomAccess::~CPDFSDK_CustomAccess() = default;
 
 FX_FILESIZE CPDFSDK_CustomAccess::GetSize() {
-  return m_FileAccess.m_FileLen;
+  return m_pFileAccess->m_FileLen;
 }
 
 bool CPDFSDK_CustomAccess::ReadBlockAtOffset(void* buffer,
@@ -21,12 +23,13 @@
   if (offset < 0)
     return false;
 
-  FX_SAFE_FILESIZE newPos = pdfium::base::checked_cast<FX_FILESIZE>(size);
-  newPos += offset;
-  if (!newPos.IsValid() ||
-      newPos.ValueOrDie() > static_cast<FX_FILESIZE>(m_FileAccess.m_FileLen)) {
+  FX_SAFE_FILESIZE new_pos = pdfium::base::checked_cast<FX_FILESIZE>(size);
+  new_pos += offset;
+  if (!new_pos.IsValid())
     return false;
-  }
-  return !!m_FileAccess.m_GetBlock(m_FileAccess.m_Param, offset,
-                                   static_cast<uint8_t*>(buffer), size);
+  if (new_pos.ValueOrDie() > static_cast<FX_FILESIZE>(m_pFileAccess->m_FileLen))
+    return false;
+
+  return !!m_pFileAccess->m_GetBlock(m_pFileAccess->m_Param, offset,
+                                     static_cast<uint8_t*>(buffer), size);
 }

fpdfsdk/cpdfsdk_customaccess.h

diff --git a/fpdfsdk/cpdfsdk_customaccess.h b/fpdfsdk/cpdfsdk_customaccess.h
index 76940ce..341d4ab 100644
--- a/fpdfsdk/cpdfsdk_customaccess.h
+++ b/fpdfsdk/cpdfsdk_customaccess.h
@@ -25,7 +25,7 @@
   explicit CPDFSDK_CustomAccess(FPDF_FILEACCESS* pFileAccess);
   ~CPDFSDK_CustomAccess() override;
 
-  FPDF_FILEACCESS m_FileAccess;
+  UnownedPtr<FPDF_FILEACCESS> const m_pFileAccess;
 };
 
 #endif  // FPDFSDK_CPDFSDK_CUSTOMACCESS_H_

fpdfsdk/fpdf_view.cpp

diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp
index f4c1515..571f6eb 100644
--- a/fpdfsdk/fpdf_view.cpp
+++ b/fpdfsdk/fpdf_view.cpp
@@ -267,6 +267,8 @@
 FPDF_EXPORT FPDF_DOCUMENT FPDF_CALLCONV
 FPDF_LoadCustomDocument(FPDF_FILEACCESS* pFileAccess,
                         FPDF_BYTESTRING password) {
+  if (!pFileAccess)
+    return nullptr;
   return LoadDocumentImpl(pdfium::MakeRetain<CPDFSDK_CustomAccess>(pFileAccess),
                           password);
 }

fpdfsdk/fpdf_view_embeddertest.cpp

diff --git a/fpdfsdk/fpdf_view_embeddertest.cpp b/fpdfsdk/fpdf_view_embeddertest.cpp
index 301eaa3..9de00c5 100644
--- a/fpdfsdk/fpdf_view_embeddertest.cpp
+++ b/fpdfsdk/fpdf_view_embeddertest.cpp
@@ -107,6 +107,10 @@
   EXPECT_EQ(16, version);
 }
 
+TEST_F(FPDFViewEmbedderTest, LoadCustomDocumentWithoutFileAccess) {
+  EXPECT_FALSE(FPDF_LoadCustomDocument(nullptr, ""));
+}
+
 TEST_F(FPDFViewEmbedderTest, Page) {
   EXPECT_TRUE(OpenDocument("about_blank.pdf"));
   FPDF_PAGE page = LoadPage(0);