summaryrefslogtreecommitdiff
path: root/core/fpdfapi/fpdf_parser
diff options
context:
space:
mode:
authortsepez <tsepez@chromium.org>2016-09-20 13:23:21 -0700
committerCommit bot <commit-bot@chromium.org>2016-09-20 13:23:21 -0700
commit5a399de2945d7b244802565d8e9d2f6e662561da (patch)
tree9c25da0dd44043f69b750a9071533596aa92c6e3 /core/fpdfapi/fpdf_parser
parent0d726c0c9931979d9b0594d56b52c861e08e09ba (diff)
downloadpdfium-5a399de2945d7b244802565d8e9d2f6e662561da.tar.xz
Make CPDF_Array not do indirect object creation.
We remove the indirect object holder argument and check that call sites pass ownable objects, adding a reference in one place that always was passing an indirect object. Also check that the invariant isn't violated, we need to fail here in the wild and investigate -- these are existing UAFs. Review-Url: https://codereview.chromium.org/2355083002
Diffstat (limited to 'core/fpdfapi/fpdf_parser')
-rw-r--r--core/fpdfapi/fpdf_parser/cpdf_array.cpp37
-rw-r--r--core/fpdfapi/fpdf_parser/cpdf_array_unittest.cpp3
-rw-r--r--core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp3
-rw-r--r--core/fpdfapi/fpdf_parser/cpdf_document.cpp2
-rw-r--r--core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp3
-rw-r--r--core/fpdfapi/fpdf_parser/include/cpdf_array.h10
6 files changed, 20 insertions, 38 deletions
diff --git a/core/fpdfapi/fpdf_parser/cpdf_array.cpp b/core/fpdfapi/fpdf_parser/cpdf_array.cpp
index 5e103fa423..da935af47d 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_array.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_array.cpp
@@ -13,6 +13,7 @@
#include "core/fpdfapi/fpdf_parser/include/cpdf_reference.h"
#include "core/fpdfapi/fpdf_parser/include/cpdf_stream.h"
#include "core/fpdfapi/fpdf_parser/include/cpdf_string.h"
+#include "third_party/base/logging.h"
#include "third_party/base/stl_util.h"
CPDF_Array::CPDF_Array() {}
@@ -145,30 +146,22 @@ void CPDF_Array::RemoveAt(size_t i, size_t nCount) {
m_Objects.erase(m_Objects.begin() + i, m_Objects.begin() + i + nCount);
}
-void CPDF_Array::SetAt(size_t i,
- CPDF_Object* pObj,
- CPDF_IndirectObjectHolder* pObjs) {
+void CPDF_Array::SetAt(size_t i, CPDF_Object* pObj) {
ASSERT(IsArray());
+ CHECK(!pObj || pObj->GetObjNum() == 0);
if (i >= m_Objects.size()) {
ASSERT(false);
return;
}
if (CPDF_Object* pOld = m_Objects[i])
pOld->Release();
- if (pObj->GetObjNum()) {
- ASSERT(pObjs);
- pObj = new CPDF_Reference(pObjs, pObj->GetObjNum());
- }
+
m_Objects[i] = pObj;
}
-void CPDF_Array::InsertAt(size_t index,
- CPDF_Object* pObj,
- CPDF_IndirectObjectHolder* pObjs) {
- if (pObj->GetObjNum()) {
- ASSERT(pObjs);
- pObj = new CPDF_Reference(pObjs, pObj->GetObjNum());
- }
+void CPDF_Array::InsertAt(size_t index, CPDF_Object* pObj) {
+ ASSERT(IsArray());
+ CHECK(!pObj || pObj->GetObjNum() == 0);
if (index >= m_Objects.size()) {
// Allocate space first.
m_Objects.resize(index + 1, nullptr);
@@ -179,37 +172,29 @@ void CPDF_Array::InsertAt(size_t index,
}
}
-void CPDF_Array::Add(CPDF_Object* pObj, CPDF_IndirectObjectHolder* pObjs) {
- if (pObj->GetObjNum()) {
- ASSERT(pObjs);
- pObj = new CPDF_Reference(pObjs, pObj->GetObjNum());
- }
+void CPDF_Array::Add(CPDF_Object* pObj) {
+ ASSERT(IsArray());
+ CHECK(!pObj || pObj->GetObjNum() == 0);
m_Objects.push_back(pObj);
}
void CPDF_Array::AddName(const CFX_ByteString& str) {
- ASSERT(IsArray());
Add(new CPDF_Name(str));
}
void CPDF_Array::AddString(const CFX_ByteString& str) {
- ASSERT(IsArray());
Add(new CPDF_String(str, FALSE));
}
void CPDF_Array::AddInteger(int i) {
- ASSERT(IsArray());
Add(new CPDF_Number(i));
}
void CPDF_Array::AddNumber(FX_FLOAT f) {
- ASSERT(IsArray());
- CPDF_Number* pNumber = new CPDF_Number(f);
- Add(pNumber);
+ Add(new CPDF_Number(f));
}
void CPDF_Array::AddReference(CPDF_IndirectObjectHolder* pDoc,
uint32_t objnum) {
- ASSERT(IsArray());
Add(new CPDF_Reference(pDoc, objnum));
}
diff --git a/core/fpdfapi/fpdf_parser/cpdf_array_unittest.cpp b/core/fpdfapi/fpdf_parser/cpdf_array_unittest.cpp
index ef9b65c5c4..f08ee6ca63 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_array_unittest.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_array_unittest.cpp
@@ -117,8 +117,7 @@ TEST(cpdf_array, Clone) {
// Starts object number from 1.
int obj_num = i * kNumOfRowElems + j + 1;
obj_holder->ReplaceIndirectObjectIfHigherGeneration(obj_num, obj);
- arr_elem->InsertAt(j, new CPDF_Reference(obj_holder.get(), obj_num),
- obj_holder.get());
+ arr_elem->InsertAt(j, new CPDF_Reference(obj_holder.get(), obj_num));
}
arr->InsertAt(i, arr_elem);
}
diff --git a/core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp b/core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp
index aeee3382f9..1a84fcb142 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp
@@ -17,6 +17,7 @@
#include "core/fpdfapi/fpdf_parser/include/cpdf_stream.h"
#include "core/fpdfapi/fpdf_parser/include/cpdf_string.h"
#include "third_party/base/stl_util.h"
+#include "third_party/base/logging.h"
CPDF_Dictionary::CPDF_Dictionary() {}
@@ -169,7 +170,7 @@ bool CPDF_Dictionary::IsSignatureDict() const {
}
void CPDF_Dictionary::SetFor(const CFX_ByteString& key, CPDF_Object* pObj) {
- ASSERT(!pObj || pObj->GetObjNum() == 0);
+ CHECK(!pObj || pObj->GetObjNum() == 0);
auto it = m_Map.find(key);
if (it == m_Map.end()) {
if (pObj)
diff --git a/core/fpdfapi/fpdf_parser/cpdf_document.cpp b/core/fpdfapi/fpdf_parser/cpdf_document.cpp
index 020e3544e9..bd11ed26e6 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_document.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_document.cpp
@@ -302,7 +302,7 @@ int InsertNewPage(CPDF_Document* pDoc,
pPagesList = new CPDF_Array;
pPages->SetFor("Kids", pPagesList);
}
- pPagesList->Add(pPageDict, pDoc);
+ pPagesList->Add(new CPDF_Reference(pDoc, pPageDict->GetObjNum()));
pPages->SetIntegerFor("Count", nPages + 1);
pPageDict->SetReferenceFor("Parent", pDoc, pPages->GetObjNum());
} else {
diff --git a/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp b/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp
index 5772043367..cbfdf2b9ac 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp
@@ -813,7 +813,8 @@ TEST(PDFObjectTest, CloneCheckLoop) {
objects_holder.AddIndirectObject(dict_obj);
EXPECT_EQ(1u, dict_obj->GetObjNum());
dict_obj->SetFor("arr", arr_obj);
- arr_obj->InsertAt(0, dict_obj, &objects_holder);
+ arr_obj->InsertAt(
+ 0, new CPDF_Reference(&objects_holder, dict_obj->GetObjNum()));
CPDF_Object* elem0 = arr_obj->GetObjectAt(0);
ASSERT_TRUE(elem0);
ASSERT_TRUE(elem0->IsReference());
diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_array.h b/core/fpdfapi/fpdf_parser/include/cpdf_array.h
index 8c89a060eb..1e8c612710 100644
--- a/core/fpdfapi/fpdf_parser/include/cpdf_array.h
+++ b/core/fpdfapi/fpdf_parser/include/cpdf_array.h
@@ -43,15 +43,11 @@ class CPDF_Array : public CPDF_Object {
CFX_Matrix GetMatrix();
CFX_FloatRect GetRect();
- void SetAt(size_t index,
- CPDF_Object* pObj,
- CPDF_IndirectObjectHolder* pObjs = nullptr);
- void InsertAt(size_t index,
- CPDF_Object* pObj,
- CPDF_IndirectObjectHolder* pObjs = nullptr);
+ void SetAt(size_t index, CPDF_Object* pObj);
+ void InsertAt(size_t index, CPDF_Object* pObj);
void RemoveAt(size_t index, size_t nCount = 1);
- void Add(CPDF_Object* pObj, CPDF_IndirectObjectHolder* pObjs = nullptr);
+ void Add(CPDF_Object* pObj);
void AddNumber(FX_FLOAT f);
void AddInteger(int i);
void AddString(const CFX_ByteString& str);