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>
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); }
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_
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); }
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);