Restrict the object types that FPDF_CopyViewerPreferences() copies.
Prevent FPDF_CopyViewerPreferences() from copying bad data. In the
malformed PDF in the bug, the /ViewerPreferences pointed to the catalog,
so FPDF_CopyViewerPreferences() tried to copy the entire (large) PDF
without using references.
Bug: chromium:1404202
Change-Id: Ife065c8bbeafee808d5402dba9dfb07bd4bcd269
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/103131
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/fpdfsdk/fpdf_ppo.cpp b/fpdfsdk/fpdf_ppo.cpp
index f0f62b8..ad8b900 100644
--- a/fpdfsdk/fpdf_ppo.cpp
+++ b/fpdfsdk/fpdf_ppo.cpp
@@ -682,6 +682,33 @@
dest(), pStream->GetObjNum());
}
+// Make sure arrays only contain objects of basic types.
+bool IsValidViewerPreferencesArray(const CPDF_Array* array) {
+ CPDF_ArrayLocker locker(array);
+ for (const auto& obj : locker) {
+ if (obj->IsArray() || obj->IsDictionary() || obj->IsReference() ||
+ obj->IsStream()) {
+ return false;
+ }
+ }
+ return true;
+}
+
+bool IsValidViewerPreferencesObject(const CPDF_Object* obj) {
+ // Per spec, there are no valid entries of these types.
+ if (obj->IsDictionary() || obj->IsNull() || obj->IsReference() ||
+ obj->IsStream()) {
+ return false;
+ }
+
+ const CPDF_Array* array = obj->AsArray();
+ if (!array) {
+ return true;
+ }
+
+ return IsValidViewerPreferencesArray(array);
+}
+
} // namespace
FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV
@@ -836,6 +863,14 @@
if (!pDstDict)
return false;
- pDstDict->SetFor("ViewerPreferences", pPrefDict->CloneDirectObject());
+ auto cloned_dict = pdfium::MakeRetain<CPDF_Dictionary>();
+ CPDF_DictionaryLocker locker(pPrefDict);
+ for (const auto& it : locker) {
+ if (IsValidViewerPreferencesObject(it.second)) {
+ cloned_dict->SetFor(it.first, it.second->Clone());
+ }
+ }
+
+ pDstDict->SetFor("ViewerPreferences", std::move(cloned_dict));
return true;
}
diff --git a/fpdfsdk/fpdf_ppo_embeddertest.cpp b/fpdfsdk/fpdf_ppo_embeddertest.cpp
index 5b5beb0..6f0a2b1 100644
--- a/fpdfsdk/fpdf_ppo_embeddertest.cpp
+++ b/fpdfsdk/fpdf_ppo_embeddertest.cpp
@@ -6,7 +6,13 @@
#include "core/fpdfapi/page/cpdf_form.h"
#include "core/fpdfapi/page/cpdf_formobject.h"
+#include "core/fpdfapi/parser/cpdf_array.h"
+#include "core/fpdfapi/parser/cpdf_dictionary.h"
+#include "core/fpdfapi/parser/cpdf_document.h"
+#include "core/fpdfapi/parser/cpdf_name.h"
+#include "core/fpdfapi/parser/cpdf_number.h"
#include "core/fpdfapi/parser/cpdf_stream.h"
+#include "core/fpdfapi/parser/cpdf_string.h"
#include "core/fxge/cfx_defaultrenderdevice.h"
#include "fpdfsdk/cpdfsdk_helpers.h"
#include "public/cpp/fpdf_scopers.h"
@@ -402,6 +408,46 @@
FPDF_CloseDocument(output_doc);
}
+TEST_F(FPDFPPOEmbedderTest, CopyViewerPrefTypes) {
+ ASSERT_TRUE(OpenDocument("viewer_pref_types.pdf"));
+
+ ScopedFPDFDocument output_doc(FPDF_CreateNewDocument());
+ ASSERT_TRUE(output_doc);
+ EXPECT_TRUE(FPDF_CopyViewerPreferences(output_doc.get(), document()));
+
+ // Peek under the hook to check the result.
+ const CPDF_Document* output_doc_impl =
+ CPDFDocumentFromFPDFDocument(output_doc.get());
+ RetainPtr<const CPDF_Dictionary> prefs =
+ output_doc_impl->GetRoot()->GetDictFor("ViewerPreferences");
+ ASSERT_TRUE(prefs);
+ EXPECT_EQ(6u, prefs->size());
+
+ RetainPtr<const CPDF_Object> bool_obj = prefs->GetObjectFor("Bool");
+ ASSERT_TRUE(bool_obj);
+ EXPECT_TRUE(bool_obj->IsBoolean());
+
+ RetainPtr<const CPDF_Number> num_obj = prefs->GetNumberFor("Num");
+ ASSERT_TRUE(num_obj);
+ EXPECT_TRUE(num_obj->IsInteger());
+ EXPECT_EQ(1, num_obj->GetInteger());
+
+ RetainPtr<const CPDF_String> str_obj = prefs->GetStringFor("Str");
+ ASSERT_TRUE(str_obj);
+ EXPECT_EQ("str", str_obj->GetString());
+
+ EXPECT_EQ("name", prefs->GetNameFor("Name"));
+
+ RetainPtr<const CPDF_Array> empty_array_obj =
+ prefs->GetArrayFor("EmptyArray");
+ ASSERT_TRUE(empty_array_obj);
+ EXPECT_TRUE(empty_array_obj->IsEmpty());
+
+ RetainPtr<const CPDF_Array> good_array_obj = prefs->GetArrayFor("GoodArray");
+ ASSERT_TRUE(good_array_obj);
+ EXPECT_EQ(4u, good_array_obj->size());
+}
+
TEST_F(FPDFPPOEmbedderTest, BadIndices) {
ASSERT_TRUE(OpenDocument("hello_world.pdf"));
diff --git a/testing/resources/viewer_pref_types.in b/testing/resources/viewer_pref_types.in
new file mode 100644
index 0000000..11b8034
--- /dev/null
+++ b/testing/resources/viewer_pref_types.in
@@ -0,0 +1,36 @@
+{{header}}
+{{object 1 0}} <<
+ /Type /Catalog
+ /Pages 2 0 R
+ /ViewerPreferences <<
+ /Bool true
+ /Num 1
+ /Str (str)
+ /Name /name
+ /Null null
+ /Ref 3 0 R
+ /EmptyArray []
+ /GoodArray [true 1 (str) /name]
+ /BadArray1 [true []]
+ /BadArray2 [1 <<>>]
+ /BadArray3 [/name 3 0 R]
+ /Dict <<
+ >>
+ >>
+>>
+endobj
+{{object 2 0}} <<
+ /Type /Pages
+ /Count 1
+ /Kids [3 0 R]
+>>
+endobj
+{{object 3 0}} <<
+ /Type /Page
+ /Parent 2 0 R
+>>
+endobj
+{{xref}}
+{{trailer}}
+{{startxref}}
+%%EOF
diff --git a/testing/resources/viewer_pref_types.pdf b/testing/resources/viewer_pref_types.pdf
new file mode 100644
index 0000000..f74ca6f
--- /dev/null
+++ b/testing/resources/viewer_pref_types.pdf
@@ -0,0 +1,46 @@
+%PDF-1.7
+% ò¤ô
+1 0 obj <<
+ /Type /Catalog
+ /Pages 2 0 R
+ /ViewerPreferences <<
+ /Bool true
+ /Num 1
+ /Str (str)
+ /Name /name
+ /Null null
+ /Ref 3 0 R
+ /EmptyArray []
+ /GoodArray [true 1 (str) /name]
+ /BadArray1 [true []]
+ /BadArray2 [1 <<>>]
+ /BadArray3 [/name 3 0 R]
+ /Dict <<
+ >>
+ >>
+>>
+endobj
+2 0 obj <<
+ /Type /Pages
+ /Count 1
+ /Kids [3 0 R]
+>>
+endobj
+3 0 obj <<
+ /Type /Page
+ /Parent 2 0 R
+>>
+endobj
+xref
+0 4
+0000000000 65535 f
+0000000015 00000 n
+0000000337 00000 n
+0000000400 00000 n
+trailer <<
+ /Root 1 0 R
+ /Size 4
+>>
+startxref
+451
+%%EOF
