From e507dc5004184ae3f8fd1cd19b723b4be69a46da Mon Sep 17 00:00:00 2001 From: tsepez Date: Wed, 18 Jan 2017 10:24:35 -0800 Subject: Bad indexing in CPDF_Document::FindPageIndex when page tree corrupt. Moving to std::vector from the more forgiving CFX_ArrayTemplate revealed the dubious page tree traversal, which depends on the correctness of the /Count entries to properly summarize the total descendants under a given node. The only "correct" thing to do is to throw away these counts as parsed, and re-compute them, perhaps in CountPages(). But I'm not willing to do that since it may break unknown documents in the wild. Pass out-params as pointers while we're at it. BUG=680376 Review-Url: https://codereview.chromium.org/2636403003 --- core/fpdfapi/parser/cpdf_document.cpp | 38 +++++++++++++++++++---------------- core/fpdfapi/parser/cpdf_document.h | 4 ++-- 2 files changed, 23 insertions(+), 19 deletions(-) (limited to 'core/fpdfapi/parser') diff --git a/core/fpdfapi/parser/cpdf_document.cpp b/core/fpdfapi/parser/cpdf_document.cpp index 9e60aaa882..e425cfc328 100644 --- a/core/fpdfapi/parser/cpdf_document.cpp +++ b/core/fpdfapi/parser/cpdf_document.cpp @@ -516,18 +516,18 @@ void CPDF_Document::SetPageObjNum(int iPage, uint32_t objNum) { } int CPDF_Document::FindPageIndex(CPDF_Dictionary* pNode, - uint32_t& skip_count, + uint32_t* skip_count, uint32_t objnum, - int& index, + int* index, int level) { if (!pNode->KeyExist("Kids")) { if (objnum == pNode->GetObjNum()) - return index; + return *index; - if (skip_count) - skip_count--; + if (*skip_count) + (*skip_count)--; - index++; + (*index)++; return -1; } @@ -539,20 +539,17 @@ int CPDF_Document::FindPageIndex(CPDF_Dictionary* pNode, return -1; size_t count = pNode->GetIntegerFor("Count"); - if (count <= skip_count) { - skip_count -= count; - index += count; + if (count <= *skip_count) { + (*skip_count) -= count; + (*index) += count; return -1; } if (count && count == pKidList->GetCount()) { for (size_t i = 0; i < count; i++) { - if (CPDF_Reference* pKid = ToReference(pKidList->GetObjectAt(i))) { - if (pKid->GetRefObjNum() == objnum) { - m_PageList[index + i] = objnum; - return static_cast(index + i); - } - } + CPDF_Reference* pKid = ToReference(pKidList->GetObjectAt(i)); + if (pKid && pKid->GetRefObjNum() == objnum) + return static_cast(*index + i); } } @@ -585,8 +582,15 @@ int CPDF_Document::GetPageIndex(uint32_t objnum) { if (!pPages) return -1; - int index = 0; - return FindPageIndex(pPages, skip_count, objnum, index); + int start_index = 0; + int found_index = FindPageIndex(pPages, &skip_count, objnum, &start_index); + + // Corrupt page tree may yield out-of-range results. + if (found_index < 0 || found_index >= pdfium::CollectionSize(m_PageList)) + return -1; + + m_PageList[found_index] = objnum; + return found_index; } int CPDF_Document::GetPageCount() const { diff --git a/core/fpdfapi/parser/cpdf_document.h b/core/fpdfapi/parser/cpdf_document.h index 65455480a4..3848ad6220 100644 --- a/core/fpdfapi/parser/cpdf_document.h +++ b/core/fpdfapi/parser/cpdf_document.h @@ -106,9 +106,9 @@ class CPDF_Document : public CPDF_IndirectObjectHolder { // When this method is called, m_pTreeTraversal[level] exists. CPDF_Dictionary* TraversePDFPages(int iPage, int* nPagesToGo, size_t level); int FindPageIndex(CPDF_Dictionary* pNode, - uint32_t& skip_count, + uint32_t* skip_count, uint32_t objnum, - int& index, + int* index, int level = 0); std::unique_ptr ParseIndirectObject(uint32_t objnum) override; void LoadDocInternal(); -- cgit v1.2.3