Return retained results from CPDF_ObjectWalker::GetNext()
Static analysis suggests that the last reference to the object
stored as next_object_ may be removed immediately before returning
it from GetNext(). It is unlikely to be reachable in practice,
since other references can't always be deduced by the analysis.
Bug: chromium:1358732
Change-Id: I235ba38c7085b3d6f0d9310b49a3d12b57bbc07a
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/97510
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfapi/parser/cpdf_crypto_handler.cpp b/core/fpdfapi/parser/cpdf_crypto_handler.cpp
index 8e4bfa7..69c2ac1 100644
--- a/core/fpdfapi/parser/cpdf_crypto_handler.cpp
+++ b/core/fpdfapi/parser/cpdf_crypto_handler.cpp
@@ -230,18 +230,18 @@
struct MayBeSignature {
const CPDF_Dictionary* parent;
- CPDF_Object* contents;
+ RetainPtr<CPDF_Object> contents;
};
std::stack<MayBeSignature> may_be_sign_dictionaries;
const uint32_t obj_num = object->GetObjNum();
const uint32_t gen_num = object->GetGenNum();
- CPDF_Object* object_to_decrypt = object.Get();
+ RetainPtr<CPDF_Object> object_to_decrypt = object;
while (object_to_decrypt) {
- CPDF_NonConstObjectWalker walker(object_to_decrypt);
+ CPDF_NonConstObjectWalker walker(object_to_decrypt.Get());
object_to_decrypt = nullptr;
- while (CPDF_Object* child = walker.GetNext()) {
+ while (RetainPtr<CPDF_Object> child = walker.GetNext()) {
const CPDF_Dictionary* parent_dict =
walker.GetParent() ? walker.GetParent()->GetDict() : nullptr;
if (walker.dictionary_key() == kContentsKey &&
@@ -253,7 +253,7 @@
// Temporary skip it, to prevent signature corruption.
// It will be decrypted on next interations, if this is not contents of
// signature dictionary.
- may_be_sign_dictionaries.push(MayBeSignature({parent_dict, child}));
+ may_be_sign_dictionaries.push({parent_dict, std::move(child)});
walker.SkipWalkIntoCurrentObject();
continue;
}
diff --git a/core/fpdfapi/parser/cpdf_object_avail.cpp b/core/fpdfapi/parser/cpdf_object_avail.cpp
index c47be27..bad6a16 100644
--- a/core/fpdfapi/parser/cpdf_object_avail.cpp
+++ b/core/fpdfapi/parser/cpdf_object_avail.cpp
@@ -113,14 +113,14 @@
return true;
CPDF_ObjectWalker walker(object);
- while (const CPDF_Object* obj = walker.GetNext()) {
+ while (RetainPtr<const CPDF_Object> obj = walker.GetNext()) {
CPDF_ReadValidator::ScopedSession parse_session(validator_);
// Skip if this object if it's an inlined root, the parent object or
// explicitily excluded.
const bool skip = (walker.GetParent() && obj == root_) ||
walker.dictionary_key() == "Parent" ||
- (obj != root_ && ExcludeObject(obj));
+ (obj != root_ && ExcludeObject(obj.Get()));
// We need to parse the object before we can do the exclusion check.
// This is because the exclusion check may check against a referenced
diff --git a/core/fpdfapi/parser/cpdf_object_walker.cpp b/core/fpdfapi/parser/cpdf_object_walker.cpp
index dfebab6..c0551dc 100644
--- a/core/fpdfapi/parser/cpdf_object_walker.cpp
+++ b/core/fpdfapi/parser/cpdf_object_walker.cpp
@@ -132,7 +132,7 @@
CPDF_ObjectWalker::~CPDF_ObjectWalker() = default;
-const CPDF_Object* CPDF_ObjectWalker::GetNext() {
+RetainPtr<const CPDF_Object> CPDF_ObjectWalker::GetNext() {
while (!stack_.empty() || next_object_) {
if (next_object_) {
auto new_iterator = MakeIterator(next_object_.Get());
@@ -140,9 +140,7 @@
// Schedule walk within composite objects.
stack_.push(std::move(new_iterator));
}
- auto* result = next_object_.Get();
- next_object_ = nullptr;
- return result;
+ return std::move(next_object_); // next_object_ now NULL after move.
}
SubobjectIterator* it = stack_.top().get();
diff --git a/core/fpdfapi/parser/cpdf_object_walker.h b/core/fpdfapi/parser/cpdf_object_walker.h
index 92b51a9..56be755 100644
--- a/core/fpdfapi/parser/cpdf_object_walker.h
+++ b/core/fpdfapi/parser/cpdf_object_walker.h
@@ -39,7 +39,7 @@
explicit CPDF_ObjectWalker(const CPDF_Object* root);
~CPDF_ObjectWalker();
- const CPDF_Object* GetNext();
+ RetainPtr<const CPDF_Object> GetNext();
void SkipWalkIntoCurrentObject();
size_t current_depth() const { return current_depth_; }
@@ -62,8 +62,9 @@
explicit CPDF_NonConstObjectWalker(CPDF_Object* root)
: CPDF_ObjectWalker(root) {}
- CPDF_Object* GetNext() {
- return const_cast<CPDF_Object*>(CPDF_ObjectWalker::GetNext());
+ RetainPtr<CPDF_Object> GetNext() {
+ return pdfium::WrapRetain(
+ const_cast<CPDF_Object*>(CPDF_ObjectWalker::GetNext().Get()));
}
};
diff --git a/core/fpdfapi/parser/cpdf_object_walker_unittest.cpp b/core/fpdfapi/parser/cpdf_object_walker_unittest.cpp
index 99c575e..4106d0d 100644
--- a/core/fpdfapi/parser/cpdf_object_walker_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_object_walker_unittest.cpp
@@ -24,7 +24,7 @@
std::string Walk(CPDF_Object* object) {
std::ostringstream result;
CPDF_ObjectWalker walker(object);
- while (const CPDF_Object* obj = walker.GetNext()) {
+ while (RetainPtr<const CPDF_Object> obj = walker.GetNext()) {
if (obj->IsDictionary())
result << " Dict";
else if (obj->IsArray())
@@ -91,9 +91,9 @@
// We have <</Array [ stream( << /Length 0 >>) ]>>
// In this case each step will increase depth.
// And on each step the prev object should be parent for current.
- const CPDF_Object* cur_parent = nullptr;
+ RetainPtr<const CPDF_Object> cur_parent;
CPDF_ObjectWalker walker(level_0.Get());
- while (const CPDF_Object* obj = walker.GetNext()) {
+ while (RetainPtr<const CPDF_Object> obj = walker.GetNext()) {
EXPECT_EQ(cur_parent, walker.GetParent());
cur_parent = obj;
}
@@ -110,7 +110,7 @@
int non_array_objects = 0;
CPDF_ObjectWalker walker(root_array.Get());
- while (const CPDF_Object* obj = walker.GetNext()) {
+ while (RetainPtr<const CPDF_Object> obj = walker.GetNext()) {
if (obj != root_array && obj->IsArray()) {
// skip other array except root.
walker.SkipWalkIntoCurrentObject();
@@ -131,7 +131,7 @@
dict->SetFor("5", pdfium::MakeRetain<CPDF_Null>());
CPDF_ObjectWalker walker(dict.Get());
- while (const CPDF_Object* obj = walker.GetNext()) {
+ while (RetainPtr<const CPDF_Object> obj = walker.GetNext()) {
if (dict == obj) {
// Ignore root dictinary object
continue;
