From 0ec10f94ae0ec1927c4a33cd69eac0a5fbdcbd52 Mon Sep 17 00:00:00 2001 From: Artem Strygin Date: Thu, 28 Sep 2017 17:58:18 +0300 Subject: Fix infinite loop on form availability check. The problem was, that the CPDF_SyntaxParser read last block not from requested position. In this case It move down requested position to fill whole buffer. As result this additional data was not requested by DownloadHints. To fix this allow resize data buffer in CPDF_SyntaxParser, to store more small block, and always read from requsted position. Also add reading check into CPDF_Parser::LoadLinearizedMainXRefTable to prevent infinite loops. Change-Id: I14d3f4457393025dca390aa3ceaa940716463534 Reviewed-on: https://pdfium-review.googlesource.com/11891 Commit-Queue: Art Snake Reviewed-by: dsinclair --- core/fpdfapi/parser/cpdf_parser.cpp | 3 +- core/fpdfapi/parser/cpdf_syntax_parser.cpp | 59 ++++++++++++++---------------- core/fpdfapi/parser/cpdf_syntax_parser.h | 11 ++---- fpdfsdk/fpdf_dataavail_embeddertest.cpp | 27 ++++++++++++++ 4 files changed, 60 insertions(+), 40 deletions(-) diff --git a/core/fpdfapi/parser/cpdf_parser.cpp b/core/fpdfapi/parser/cpdf_parser.cpp index 6fb0d5fa1d..6643cf5123 100644 --- a/core/fpdfapi/parser/cpdf_parser.cpp +++ b/core/fpdfapi/parser/cpdf_parser.cpp @@ -1459,7 +1459,8 @@ CPDF_Parser::Error CPDF_Parser::LoadLinearizedMainXRefTable() { (FX_FILESIZE)(m_pSyntax->GetPos() + m_pSyntax->m_HeaderOffset)) { break; } - m_pSyntax->GetNextChar(ch); + if (!m_pSyntax->GetNextChar(ch)) + return HANDLER_ERROR; } m_LastXRefOffset += dwCount; m_ObjectStreamMap.clear(); diff --git a/core/fpdfapi/parser/cpdf_syntax_parser.cpp b/core/fpdfapi/parser/cpdf_syntax_parser.cpp index 96a863e661..ac401b351d 100644 --- a/core/fpdfapi/parser/cpdf_syntax_parser.cpp +++ b/core/fpdfapi/parser/cpdf_syntax_parser.cpp @@ -46,12 +46,9 @@ CPDF_SyntaxParser::CPDF_SyntaxParser() CPDF_SyntaxParser::CPDF_SyntaxParser(const WeakPtr& pPool) : m_MetadataObjnum(0), m_pFileAccess(nullptr), - m_pFileBuf(nullptr), - m_BufSize(CPDF_ModuleMgr::kFileBufSize), m_pPool(pPool) {} CPDF_SyntaxParser::~CPDF_SyntaxParser() { - FX_Free(m_pFileBuf); } bool CPDF_SyntaxParser::GetCharAt(FX_FILESIZE pos, uint8_t& ch) { @@ -60,17 +57,20 @@ bool CPDF_SyntaxParser::GetCharAt(FX_FILESIZE pos, uint8_t& ch) { return GetNextChar(ch); } -bool CPDF_SyntaxParser::ReadChar(FX_FILESIZE read_pos, uint32_t read_size) { - if (static_cast(read_pos + read_size) > m_FileLen) { - if (m_FileLen < static_cast(read_size)) { - read_pos = 0; - read_size = static_cast(m_FileLen); - } else { - read_pos = m_FileLen - read_size; - } - } - if (!m_pFileAccess->ReadBlock(m_pFileBuf, read_pos, read_size)) +bool CPDF_SyntaxParser::ReadBlockAt(FX_FILESIZE read_pos) { + if (read_pos >= m_FileLen) return false; + size_t read_size = CPDF_ModuleMgr::kFileBufSize; + FX_SAFE_FILESIZE safe_end = read_pos; + safe_end += read_size; + if (!safe_end.IsValid() || safe_end.ValueOrDie() > m_FileLen) + read_size = m_FileLen - read_pos; + + m_pFileBuf.resize(read_size); + if (!m_pFileAccess->ReadBlock(m_pFileBuf.data(), read_pos, read_size)) { + m_pFileBuf.clear(); + return false; + } m_BufOffset = read_pos; return true; @@ -81,13 +81,9 @@ bool CPDF_SyntaxParser::GetNextChar(uint8_t& ch) { if (pos >= m_FileLen) return false; - if (CheckPosition(pos)) { - FX_FILESIZE read_pos = pos; - uint32_t read_size = m_BufSize; - read_size = std::min(read_size, static_cast(m_FileLen)); - if (!ReadChar(read_pos, read_size)) - return false; - } + if (!IsPositionRead(pos) && !ReadBlockAt(pos)) + return false; + ch = m_pFileBuf[pos - m_BufOffset]; m_Pos++; return true; @@ -98,14 +94,11 @@ bool CPDF_SyntaxParser::GetCharAtBackward(FX_FILESIZE pos, uint8_t* ch) { if (pos >= m_FileLen) return false; - if (CheckPosition(pos)) { - FX_FILESIZE read_pos; - if (pos < static_cast(m_BufSize)) - read_pos = 0; - else - read_pos = pos - m_BufSize + 1; - uint32_t read_size = m_BufSize; - if (!ReadChar(read_pos, read_size)) + if (!IsPositionRead(pos)) { + FX_FILESIZE block_start = 0; + if (pos >= CPDF_ModuleMgr::kFileBufSize) + block_start = pos - CPDF_ModuleMgr::kFileBufSize + 1; + if (!ReadBlockAt(block_start) || !IsPositionRead(pos)) return false; } *ch = m_pFileBuf[pos - m_BufOffset]; @@ -735,15 +728,12 @@ void CPDF_SyntaxParser::InitParserWithValidator( const RetainPtr& validator, uint32_t HeaderOffset) { ASSERT(validator); - FX_Free(m_pFileBuf); - m_pFileBuf = FX_Alloc(uint8_t, m_BufSize); + m_pFileBuf.clear(); m_HeaderOffset = HeaderOffset; m_FileLen = validator->GetSize(); m_Pos = 0; m_pFileAccess = validator; m_BufOffset = 0; - validator->ReadBlock(m_pFileBuf, 0, - std::min(m_BufSize, static_cast(m_FileLen))); } uint32_t CPDF_SyntaxParser::GetDirectNum() { @@ -852,3 +842,8 @@ void CPDF_SyntaxParser::SetEncrypt( RetainPtr CPDF_SyntaxParser::GetFileAccess() const { return m_pFileAccess; } + +bool CPDF_SyntaxParser::IsPositionRead(FX_FILESIZE pos) const { + return m_BufOffset <= pos && + pos < static_cast(m_BufOffset + m_pFileBuf.size()); +} diff --git a/core/fpdfapi/parser/cpdf_syntax_parser.h b/core/fpdfapi/parser/cpdf_syntax_parser.h index 3a8f7c59ec..2326362692 100644 --- a/core/fpdfapi/parser/cpdf_syntax_parser.h +++ b/core/fpdfapi/parser/cpdf_syntax_parser.h @@ -9,6 +9,7 @@ #include #include +#include #include "core/fxcrt/string_pool_template.h" #include "core/fxcrt/weak_ptr.h" @@ -82,7 +83,7 @@ class CPDF_SyntaxParser { static int s_CurrentRecursionDepth; uint32_t GetDirectNum(); - bool ReadChar(FX_FILESIZE read_pos, uint32_t read_size); + bool ReadBlockAt(FX_FILESIZE read_pos); bool GetNextChar(uint8_t& ch); bool GetCharAtBackward(FX_FILESIZE pos, uint8_t* ch); void GetNextWordInternal(bool* bIsNumber); @@ -99,10 +100,7 @@ class CPDF_SyntaxParser { uint32_t objnum, uint32_t gennum); - inline bool CheckPosition(FX_FILESIZE pos) { - return m_BufOffset >= pos || - static_cast(m_BufOffset + m_BufSize) <= pos; - } + bool IsPositionRead(FX_FILESIZE pos) const; std::unique_ptr GetObjectBodyInternal( CPDF_IndirectObjectHolder* pObjList, @@ -116,8 +114,7 @@ class CPDF_SyntaxParser { RetainPtr m_pFileAccess; FX_FILESIZE m_HeaderOffset; FX_FILESIZE m_FileLen; - uint8_t* m_pFileBuf; - uint32_t m_BufSize; + std::vector m_pFileBuf; FX_FILESIZE m_BufOffset; RetainPtr m_pCryptoHandler; uint8_t m_WordBuffer[257]; diff --git a/fpdfsdk/fpdf_dataavail_embeddertest.cpp b/fpdfsdk/fpdf_dataavail_embeddertest.cpp index 8f37984c9b..281f08640e 100644 --- a/fpdfsdk/fpdf_dataavail_embeddertest.cpp +++ b/fpdfsdk/fpdf_dataavail_embeddertest.cpp @@ -63,6 +63,13 @@ class TestAsyncLoader : public FX_DOWNLOADHINTS, FX_FILEAVAIL { return available_ranges_.empty() ? 0 : available_ranges_.rbegin()->second; } + void FlushRequestedData() { + for (const auto& it : requested_segments_) { + SetDataAvailable(it.first, it.second); + } + ClearRequestedSegments(); + } + private: void SetDataAvailable(size_t start, size_t size) { if (size == 0) @@ -199,6 +206,26 @@ TEST_F(FPDFDataAvailEmbeddertest, LoadUsingHintTables) { FPDF_ClosePage(page); } +TEST_F(FPDFDataAvailEmbeddertest, CheckFormAvailIfLinearized) { + TestAsyncLoader loader("feature_linearized_loading.pdf"); + avail_ = FPDFAvail_Create(loader.file_avail(), loader.file_access()); + ASSERT_EQ(PDF_DATA_AVAIL, FPDFAvail_IsDocAvail(avail_, loader.hints())); + document_ = FPDFAvail_GetDocument(avail_, nullptr); + ASSERT_TRUE(document_); + + // Prevent access to non requested data to coerce the parser to send new + // request for non available (non requested before) data. + loader.set_is_new_data_available(false); + loader.ClearRequestedSegments(); + + int status = PDF_FORM_NOTAVAIL; + while (status == PDF_FORM_NOTAVAIL) { + loader.FlushRequestedData(); + status = FPDFAvail_IsFormAvail(avail_, loader.hints()); + } + EXPECT_NE(PDF_FORM_ERROR, status); +} + TEST_F(FPDFDataAvailEmbeddertest, DoNotLoadMainCrossRefForFirstPageIfLinearized) { TestAsyncLoader loader("feature_linearized_loading.pdf"); -- cgit v1.2.3