Make potentially dangerous Actions require a user click.

URI and SubmitForm actions are only handled if the event was
ButtonUp or ButtonDown.

Bug: 851821
Change-Id: If6eb0ff44f6d62ac6df50b552c0bdc582885ab5d
Reviewed-on: https://pdfium-review.googlesource.com/42731
Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>

9 files changed, 96 insertions, 14 deletions

diff --git a/core/fpdfdoc/cpdf_aaction.cpp b/core/fpdfdoc/cpdf_aaction.cpp
index 9a95340114..8d954697b4 100644
--- a/core/fpdfdoc/cpdf_aaction.cpp
+++ b/core/fpdfdoc/cpdf_aaction.cpp
@@ -32,8 +32,9 @@ constexpr const char* g_sAATypes[] = {
     "DP",  // DocumentPrinted
 };
 
-// |g_sAATypes| should have as many elements as enum AActionType.
-static_assert(FX_ArraySize(g_sAATypes) == CPDF_AAction::NumberOfActions,
+// |g_sAATypes| should have one less element than enum AActionType due to
+// DocumentOpen, which is an artificial type.
+static_assert(FX_ArraySize(g_sAATypes) == CPDF_AAction::NumberOfActions - 1,
               "g_sAATypes count mismatch");
 
 }  // namespace
@@ -52,3 +53,14 @@ CPDF_Action CPDF_AAction::GetAction(AActionType eType) const {
   return CPDF_Action(m_pDict ? m_pDict->GetDictFor(g_sAATypes[eType])
                              : nullptr);
 }
+
+// static
+bool CPDF_AAction::IsUserClick(AActionType eType) {
+  switch (eType) {
+    case ButtonUp:
+    case ButtonDown:
+      return true;
+    default:
+      return false;
+  }
+}
diff --git a/core/fpdfdoc/cpdf_aaction.h b/core/fpdfdoc/cpdf_aaction.h
index 11eca01f8f..90049f5ac4 100644
--- a/core/fpdfdoc/cpdf_aaction.h
+++ b/core/fpdfdoc/cpdf_aaction.h
@@ -35,6 +35,7 @@ class CPDF_AAction {
     DocumentSaved,
     PrintDocument,
     DocumentPrinted,
+    DocumentOpen,
     NumberOfActions  // Must be last.
   };
 
@@ -46,6 +47,8 @@ class CPDF_AAction {
   CPDF_Action GetAction(AActionType eType) const;
   const CPDF_Dictionary* GetDict() const { return m_pDict.Get(); }
 
+  static bool IsUserClick(AActionType eType);
+
  private:
   UnownedPtr<const CPDF_Dictionary> const m_pDict;
 };
diff --git a/fpdfsdk/cpdfsdk_actionhandler.cpp b/fpdfsdk/cpdfsdk_actionhandler.cpp
index f951bfb9fe..f98585f2a1 100644
--- a/fpdfsdk/cpdfsdk_actionhandler.cpp
+++ b/fpdfsdk/cpdfsdk_actionhandler.cpp
@@ -82,7 +82,7 @@ bool CPDFSDK_ActionHandler::DoAction_BookMark(
     CPDF_AAction::AActionType type,
     CPDFSDK_FormFillEnvironment* pFormFillEnv) {
   std::set<const CPDF_Dictionary*> visited;
-  return ExecuteBookMark(action, pFormFillEnv, pBookMark, &visited);
+  return ExecuteBookMark(action, type, pFormFillEnv, pBookMark, &visited);
 }
 
 bool CPDFSDK_ActionHandler::DoAction_Screen(
@@ -96,9 +96,10 @@ bool CPDFSDK_ActionHandler::DoAction_Screen(
 
 bool CPDFSDK_ActionHandler::DoAction_Link(
     const CPDF_Action& action,
+    CPDF_AAction::AActionType type,
     CPDFSDK_FormFillEnvironment* pFormFillEnv) {
   std::set<const CPDF_Dictionary*> visited;
-  return ExecuteLinkAction(action, pFormFillEnv, &visited);
+  return ExecuteLinkAction(action, type, pFormFillEnv, &visited);
 }
 
 bool CPDFSDK_ActionHandler::DoAction_Field(
@@ -130,7 +131,8 @@ bool CPDFSDK_ActionHandler::ExecuteDocumentOpenAction(
         RunDocumentOpenJavaScript(pFormFillEnv, L"", swJS);
     }
   } else {
-    DoAction_NoJs(action, pFormFillEnv);
+    DoAction_NoJs(action, CPDF_AAction::AActionType::DocumentOpen,
+                  pFormFillEnv);
   }
 
   for (int32_t i = 0, sz = action.GetSubActionsCount(); i < sz; i++) {
@@ -144,6 +146,7 @@ bool CPDFSDK_ActionHandler::ExecuteDocumentOpenAction(
 
 bool CPDFSDK_ActionHandler::ExecuteLinkAction(
     const CPDF_Action& action,
+    CPDF_AAction::AActionType eType,
     CPDFSDK_FormFillEnvironment* pFormFillEnv,
     std::set<const CPDF_Dictionary*>* visited) {
   const CPDF_Dictionary* pDict = action.GetDict();
@@ -159,12 +162,12 @@ bool CPDFSDK_ActionHandler::ExecuteLinkAction(
                          context->OnLink_MouseUp(pFormFillEnv);
                        });
   } else {
-    DoAction_NoJs(action, pFormFillEnv);
+    DoAction_NoJs(action, eType, pFormFillEnv);
   }
 
   for (int32_t i = 0, sz = action.GetSubActionsCount(); i < sz; i++) {
     CPDF_Action subaction = action.GetSubAction(i);
-    if (!ExecuteLinkAction(subaction, pFormFillEnv, visited))
+    if (!ExecuteLinkAction(subaction, eType, pFormFillEnv, visited))
       return false;
   }
 
@@ -190,7 +193,7 @@ bool CPDFSDK_ActionHandler::ExecuteDocumentPageAction(
         RunDocumentPageJavaScript(pFormFillEnv, type, swJS);
     }
   } else {
-    DoAction_NoJs(action, pFormFillEnv);
+    DoAction_NoJs(action, type, pFormFillEnv);
   }
 
   ASSERT(pFormFillEnv);
@@ -238,7 +241,7 @@ bool CPDFSDK_ActionHandler::ExecuteFieldAction(
       }
     }
   } else {
-    DoAction_NoJs(action, pFormFillEnv);
+    DoAction_NoJs(action, type, pFormFillEnv);
   }
 
   for (int32_t i = 0, sz = action.GetSubActionsCount(); i < sz; i++) {
@@ -267,7 +270,7 @@ bool CPDFSDK_ActionHandler::ExecuteScreenAction(
   if (action.GetType() == CPDF_Action::JavaScript)
     RunScriptForAction(action, pFormFillEnv, [](IJS_EventContext*) {});
   else
-    DoAction_NoJs(action, pFormFillEnv);
+    DoAction_NoJs(action, type, pFormFillEnv);
 
   for (int32_t i = 0, sz = action.GetSubActionsCount(); i < sz; i++) {
     CPDF_Action subaction = action.GetSubAction(i);
@@ -280,6 +283,7 @@ bool CPDFSDK_ActionHandler::ExecuteScreenAction(
 
 bool CPDFSDK_ActionHandler::ExecuteBookMark(
     const CPDF_Action& action,
+    CPDF_AAction::AActionType type,
     CPDFSDK_FormFillEnvironment* pFormFillEnv,
     CPDF_Bookmark* pBookmark,
     std::set<const CPDF_Dictionary*>* visited) {
@@ -296,12 +300,12 @@ bool CPDFSDK_ActionHandler::ExecuteBookMark(
                          context->OnBookmark_MouseUp(pBookmark);
                        });
   } else {
-    DoAction_NoJs(action, pFormFillEnv);
+    DoAction_NoJs(action, type, pFormFillEnv);
   }
 
   for (int32_t i = 0, sz = action.GetSubActionsCount(); i < sz; i++) {
     CPDF_Action subaction = action.GetSubAction(i);
-    if (!ExecuteBookMark(subaction, pFormFillEnv, pBookmark, visited))
+    if (!ExecuteBookMark(subaction, type, pFormFillEnv, pBookmark, visited))
       return false;
   }
 
@@ -310,6 +314,7 @@ bool CPDFSDK_ActionHandler::ExecuteBookMark(
 
 void CPDFSDK_ActionHandler::DoAction_NoJs(
     const CPDF_Action& action,
+    CPDF_AAction::AActionType type,
     CPDFSDK_FormFillEnvironment* pFormFillEnv) {
   ASSERT(pFormFillEnv);
 
@@ -318,7 +323,8 @@ void CPDFSDK_ActionHandler::DoAction_NoJs(
       DoAction_GoTo(pFormFillEnv, action);
       break;
     case CPDF_Action::URI:
-      DoAction_URI(pFormFillEnv, action);
+      if (CPDF_AAction::IsUserClick(type))
+        DoAction_URI(pFormFillEnv, action);
       break;
     case CPDF_Action::Hide:
       DoAction_Hide(action, pFormFillEnv);
@@ -327,7 +333,8 @@ void CPDFSDK_ActionHandler::DoAction_NoJs(
       DoAction_Named(pFormFillEnv, action);
       break;
     case CPDF_Action::SubmitForm:
-      DoAction_SubmitForm(action, pFormFillEnv);
+      if (CPDF_AAction::IsUserClick(type))
+        DoAction_SubmitForm(action, pFormFillEnv);
       break;
     case CPDF_Action::ResetForm:
       DoAction_ResetForm(action, pFormFillEnv);
diff --git a/fpdfsdk/cpdfsdk_actionhandler.h b/fpdfsdk/cpdfsdk_actionhandler.h
index 14cbc9c5c5..94b347f630 100644
--- a/fpdfsdk/cpdfsdk_actionhandler.h
+++ b/fpdfsdk/cpdfsdk_actionhandler.h
@@ -44,6 +44,7 @@ class CPDFSDK_ActionHandler {
                        CPDFSDK_FormFillEnvironment* pFormFillEnv,
                        CPDFSDK_Annot* pScreen);
   bool DoAction_Link(const CPDF_Action& action,
+                     CPDF_AAction::AActionType type,
                      CPDFSDK_FormFillEnvironment* pFormFillEnv);
   bool DoAction_Field(const CPDF_Action& action,
                       CPDF_AAction::AActionType type,
@@ -85,14 +86,17 @@ class CPDFSDK_ActionHandler {
                            CPDFSDK_Annot* pScreen,
                            std::set<const CPDF_Dictionary*>* visited);
   bool ExecuteBookMark(const CPDF_Action& action,
+                       CPDF_AAction::AActionType type,
                        CPDFSDK_FormFillEnvironment* pFormFillEnv,
                        CPDF_Bookmark* pBookmark,
                        std::set<const CPDF_Dictionary*>* visited);
   bool ExecuteLinkAction(const CPDF_Action& action,
+                         CPDF_AAction::AActionType type,
                          CPDFSDK_FormFillEnvironment* pFormFillEnv,
                          std::set<const CPDF_Dictionary*>* visited);
 
   void DoAction_NoJs(const CPDF_Action& action,
+                     CPDF_AAction::AActionType type,
                      CPDFSDK_FormFillEnvironment* pFormFillEnv);
   void RunDocumentPageJavaScript(CPDFSDK_FormFillEnvironment* pFormFillEnv,
                                  CPDF_AAction::AActionType type,
diff --git a/fpdfsdk/cpdfsdk_widget.cpp b/fpdfsdk/cpdfsdk_widget.cpp
index 8bd1f10177..82776e3298 100644
--- a/fpdfsdk/cpdfsdk_widget.cpp
+++ b/fpdfsdk/cpdfsdk_widget.cpp
@@ -174,6 +174,7 @@ static XFA_EVENTTYPE GetXFAEventType(CPDF_AAction::AActionType eAAT,
     case CPDF_AAction::PrintDocument:
     case CPDF_AAction::DocumentPrinted:
       break;
+    case CPDF_AAction::DocumentOpen:
     case CPDF_AAction::NumberOfActions:
       NOTREACHED();
       break;
diff --git a/fpdfsdk/fpdf_formfill_embeddertest.cpp b/fpdfsdk/fpdf_formfill_embeddertest.cpp
index 8ff3a84291..3e53753b48 100644
--- a/fpdfsdk/fpdf_formfill_embeddertest.cpp
+++ b/fpdfsdk/fpdf_formfill_embeddertest.cpp
@@ -379,6 +379,25 @@ TEST_F(FPDFFormFillEmbeddertest, BUG_514690) {
   UnloadPage(page);
 }
 
+class DoURIActionBlockedDelegate final : public EmbedderTest::Delegate {
+ public:
+  void DoURIAction(FPDF_BYTESTRING uri) override {
+    FAIL() << "Navigated to " << uri;
+  }
+};
+
+TEST_F(FPDFFormFillEmbeddertest, BUG_851821) {
+  DoURIActionBlockedDelegate delegate;
+  SetDelegate(&delegate);
+
+  EXPECT_TRUE(OpenDocument("redirect.pdf"));
+  FPDF_PAGE page = LoadPage(0);
+  EXPECT_TRUE(page);
+  DoOpenActions();
+
+  UnloadPage(page);
+}
+
 #ifdef PDF_ENABLE_V8
 TEST_F(FPDFFormFillEmbeddertest, DisableJavaScript) {
   // Test that timers and intervals can't fire without JS.
diff --git a/testing/embedder_test.cpp b/testing/embedder_test.cpp
index 215009ceac..43f077ca01 100644
--- a/testing/embedder_test.cpp
+++ b/testing/embedder_test.cpp
@@ -241,6 +241,8 @@ FPDF_FORMHANDLE EmbedderTest::SetupFormFillEnvironment(
   formfillinfo->FFI_SetTimer = SetTimerTrampoline;
   formfillinfo->FFI_KillTimer = KillTimerTrampoline;
   formfillinfo->FFI_GetPage = GetPageTrampoline;
+  formfillinfo->FFI_DoURIAction = DoURIActionTrampoline;
+
   if (javascript_option == JavaScriptOption::kEnableJavaScript)
     formfillinfo->m_pJsPlatform = platform;
 
@@ -478,6 +480,13 @@ FPDF_PAGE EmbedderTest::GetPageTrampoline(FPDF_FORMFILLINFO* info,
 }
 
 // static
+void EmbedderTest::DoURIActionTrampoline(FPDF_FORMFILLINFO* info,
+                                         FPDF_BYTESTRING uri) {
+  EmbedderTest* test = static_cast<EmbedderTest*>(info);
+  return test->delegate_->DoURIAction(uri);
+}
+
+// static
 std::string EmbedderTest::HashBitmap(FPDF_BITMAP bitmap) {
   uint8_t digest[16];
   CRYPT_MD5Generate(static_cast<uint8_t*>(FPDFBitmap_GetBuffer(bitmap)),
diff --git a/testing/embedder_test.h b/testing/embedder_test.h
index 64707631b5..a6a4cf51e4 100644
--- a/testing/embedder_test.h
+++ b/testing/embedder_test.h
@@ -62,6 +62,9 @@ class EmbedderTest : public ::testing::Test,
     virtual FPDF_PAGE GetPage(FPDF_FORMFILLINFO* info,
                               FPDF_DOCUMENT document,
                               int page_index);
+
+    // Equivalent to FPDF_FORMFILLINFO::FFI_DoURIAction().
+    virtual void DoURIAction(FPDF_BYTESTRING uri) {}
   };
 
   EmbedderTest();
@@ -245,6 +248,8 @@ class EmbedderTest : public ::testing::Test,
   static FPDF_PAGE GetPageTrampoline(FPDF_FORMFILLINFO* info,
                                      FPDF_DOCUMENT document,
                                      int page_index);
+  static void DoURIActionTrampoline(FPDF_FORMFILLINFO* info,
+                                    FPDF_BYTESTRING uri);
   static int WriteBlockCallback(FPDF_FILEWRITE* pFileWrite,
                                 const void* data,
                                 unsigned long size);
diff --git a/testing/resources/redirect.pdf b/testing/resources/redirect.pdf
new file mode 100644
index 0000000000..2dcc23b0be
--- /dev/null
+++ b/testing/resources/redirect.pdf
@@ -0,0 +1,22 @@
+%PDF-1.7

+trailer

+<<

+/Root 1 0 R

+>>

+1 0 obj

+<<

+/Type /Catalog

+/Pages 2 0 R

+/OpenAction 2 0 R

+>>

+endobj

+ 

+2 0 obj

+<<

+/Type /Action

+/S /URI

+/URI (http://evilzone.org) // URL HERE

+>>

+endobj

+ 

+%%EOF
\ No newline at end of file