commit 4deddeb65cb40455be18a162a0a9e830954c182b
author Tom Sepez <tsepez@chromium.org> Mon May 06 19:15:53 2019 +0000
committer Chromium commit bot <commit-bot@chromium.org> Mon May 06 19:15:53 2019 +0000
tree c6325be7d00966822dcd2b98ae33ac4d9712f70e
parent 5c3e3a0cb51178a9e27a971678aa865a410b7b4d

Require string objects for file spec entries.

Follow the spec more closely.

- Also do the same for /Base entries in URL dictionaries.

Bug: chromium:959183
Change-Id: Ie78abd3ad13c4d39db99592efd81829ff6a820fc
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/53970
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>

core/fpdfdoc/cpdf_action.cpp

diff --git a/core/fpdfdoc/cpdf_action.cpp b/core/fpdfdoc/cpdf_action.cpp
index 1621741..18e4927 100644
--- a/core/fpdfdoc/cpdf_action.cpp
+++ b/core/fpdfdoc/cpdf_action.cpp
@@ -94,8 +94,11 @@
   const CPDF_Dictionary* pURI = pRoot->GetDictFor("URI");
   if (pURI) {
     auto result = csURI.Find(":");
-    if (!result.has_value() || result.value() == 0)
-      csURI = pURI->GetStringFor("Base") + csURI;
+    if (!result.has_value() || result.value() == 0) {
+      auto* pBase = pURI->GetDirectObjectFor("Base");
+      if (pBase && (pBase->IsString() || pBase->IsStream()))
+        csURI = pBase->GetString() + csURI;
+    }
   }
   return csURI;
 }

core/fpdfdoc/cpdf_filespec.cpp

diff --git a/core/fpdfdoc/cpdf_filespec.cpp b/core/fpdfdoc/cpdf_filespec.cpp
index 1e22e2f..41c5405 100644
--- a/core/fpdfdoc/cpdf_filespec.cpp
+++ b/core/fpdfdoc/cpdf_filespec.cpp
@@ -98,26 +98,30 @@
 WideString CPDF_FileSpec::GetFileName() const {
   WideString csFileName;
   if (const CPDF_Dictionary* pDict = m_pObj->AsDictionary()) {
-    csFileName = pDict->GetUnicodeTextFor("UF");
+    const CPDF_String* pUF = ToString(pDict->GetDirectObjectFor("UF"));
+    if (pUF)
+      csFileName = pUF->GetUnicodeText();
     if (csFileName.IsEmpty()) {
-      csFileName = WideString::FromDefANSI(
-          pDict->GetStringFor(pdfium::stream::kF).AsStringView());
+      const CPDF_String* pK =
+          ToString(pDict->GetDirectObjectFor(pdfium::stream::kF));
+      if (pK)
+        csFileName = WideString::FromDefANSI(pK->GetString().AsStringView());
     }
     if (pDict->GetStringFor("FS") == "URL")
       return csFileName;
 
     if (csFileName.IsEmpty()) {
-      constexpr const char* keys[] = {"DOS", "Mac", "Unix"};
-      for (const auto* key : keys) {
-        if (pDict->KeyExist(key)) {
+      for (const auto* key : {"DOS", "Mac", "Unix"}) {
+        const CPDF_String* pValue = ToString(pDict->GetDirectObjectFor(key));
+        if (pValue) {
           csFileName =
-              WideString::FromDefANSI(pDict->GetStringFor(key).AsStringView());
+              WideString::FromDefANSI(pValue->GetString().AsStringView());
           break;
         }
       }
     }
-  } else if (m_pObj->IsString()) {
-    csFileName = WideString::FromDefANSI(m_pObj->GetString().AsStringView());
+  } else if (const CPDF_String* pString = m_pObj->AsString()) {
+    csFileName = WideString::FromDefANSI(pString->GetString().AsStringView());
   }
   return DecodeFileName(csFileName);
 }

core/fpdfdoc/cpdf_filespec_unittest.cpp

diff --git a/core/fpdfdoc/cpdf_filespec_unittest.cpp b/core/fpdfdoc/cpdf_filespec_unittest.cpp
index bee6574..30394ff 100644
--- a/core/fpdfdoc/cpdf_filespec_unittest.cpp
+++ b/core/fpdfdoc/cpdf_filespec_unittest.cpp
@@ -122,6 +122,17 @@
     CPDF_FileSpec file_spec(name_obj.Get());
     EXPECT_TRUE(file_spec.GetFileName().IsEmpty());
   }
+  {
+    // Invalid CPDF_Name objects in dictionary. See https://crbug.com/959183
+    auto dict_obj = pdfium::MakeRetain<CPDF_Dictionary>();
+    CPDF_FileSpec file_spec(dict_obj.Get());
+    for (const char* key : {"Unix", "Mac", "DOS", "F", "UF"}) {
+      dict_obj->SetNewFor<CPDF_Name>(key, "http://evil.org");
+      EXPECT_TRUE(file_spec.GetFileName().IsEmpty());
+    }
+    dict_obj->SetNewFor<CPDF_String>("FS", "URL", false);
+    EXPECT_TRUE(file_spec.GetFileName().IsEmpty());
+  }
 }
 
 TEST(cpdf_filespec, SetFileName) {