From 695aac5f1f53088659f9b525a692002044e3b098 Mon Sep 17 00:00:00 2001
From: thestig <thestig@chromium.org>
Date: Thu, 25 Aug 2016 09:13:52 -0700
Subject: Fix infinite loops in FPDF_GetFullName().

BUG=444446

Review-Url: https://codereview.chromium.org/2271373003
---
 BUILD.gn                                 |  1 +
 core/fpdfdoc/cpdf_formfield.cpp          | 15 +++++++---
 core/fpdfdoc/cpdf_formfield_unittest.cpp | 50 ++++++++++++++++++++++++++++++++
 3 files changed, 62 insertions(+), 4 deletions(-)
 create mode 100644 core/fpdfdoc/cpdf_formfield_unittest.cpp

diff --git a/BUILD.gn b/BUILD.gn
index 0a4df450f1..ca2338f6fd 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -1595,6 +1595,7 @@ test("pdfium_unittests") {
     "core/fpdfapi/fpdf_parser/cpdf_syntax_parser_unittest.cpp",
     "core/fpdfapi/fpdf_parser/fpdf_parser_decode_unittest.cpp",
     "core/fpdfdoc/cpdf_filespec_unittest.cpp",
+    "core/fpdfdoc/cpdf_formfield_unittest.cpp",
     "core/fpdftext/fpdf_text_int_unittest.cpp",
     "core/fxcodec/codec/fx_codec_jpx_unittest.cpp",
     "core/fxcodec/jbig2/JBig2_Image_unittest.cpp",
diff --git a/core/fpdfdoc/cpdf_formfield.cpp b/core/fpdfdoc/cpdf_formfield.cpp
index ac6c01bae2..8d7d0b4dc0 100644
--- a/core/fpdfdoc/cpdf_formfield.cpp
+++ b/core/fpdfdoc/cpdf_formfield.cpp
@@ -6,6 +6,8 @@
 
 #include "core/fpdfdoc/include/cpdf_formfield.h"
 
+#include <set>
+
 #include "core/fpdfapi/fpdf_parser/include/cfdf_document.h"
 #include "core/fpdfapi/fpdf_parser/include/cpdf_array.h"
 #include "core/fpdfapi/fpdf_parser/include/cpdf_document.h"
@@ -16,6 +18,7 @@
 #include "core/fpdfdoc/cpvt_generateap.h"
 #include "core/fpdfdoc/include/cpdf_formcontrol.h"
 #include "core/fpdfdoc/include/cpdf_interform.h"
+#include "third_party/base/stl_util.h"
 
 namespace {
 
@@ -65,16 +68,20 @@ CPDF_Object* FPDF_GetFieldAttr(CPDF_Dictionary* pFieldDict,
 
 CFX_WideString FPDF_GetFullName(CPDF_Dictionary* pFieldDict) {
   CFX_WideString full_name;
+  std::set<CPDF_Dictionary*> visited;
   CPDF_Dictionary* pLevel = pFieldDict;
   while (pLevel) {
+    visited.insert(pLevel);
     CFX_WideString short_name = pLevel->GetUnicodeTextBy("T");
-    if (short_name != L"") {
-      if (full_name == L"")
+    if (!short_name.IsEmpty()) {
+      if (full_name.IsEmpty())
         full_name = short_name;
       else
         full_name = short_name + L"." + full_name;
     }
     pLevel = pLevel->GetDictBy("Parent");
+    if (pdfium::ContainsKey(visited, pLevel))
+      break;
   }
   return full_name;
 }
@@ -679,8 +686,8 @@ int CPDF_FormField::InsertOption(CFX_WideString csOptLabel,
     m_pDict->SetAt("Opt", pOpt);
   }
 
-  int iCount = (int)pOpt->GetCount();
-  if (index < 0 || index >= iCount) {
+  int iCount = pdfium::base::checked_cast<int, size_t>(pOpt->GetCount());
+  if (index >= iCount) {
     pOpt->AddString(csStr);
     index = iCount;
   } else {
diff --git a/core/fpdfdoc/cpdf_formfield_unittest.cpp b/core/fpdfdoc/cpdf_formfield_unittest.cpp
new file mode 100644
index 0000000000..33a21185cd
--- /dev/null
+++ b/core/fpdfdoc/cpdf_formfield_unittest.cpp
@@ -0,0 +1,50 @@
+// Copyright 2016 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h"
+#include "core/fpdfapi/fpdf_parser/include/cpdf_indirect_object_holder.h"
+#include "core/fpdfdoc/include/cpdf_formfield.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+TEST(cpdf_formfield, FPDF_GetFullName) {
+  CFX_WideString name = FPDF_GetFullName(nullptr);
+  EXPECT_TRUE(name.IsEmpty());
+
+  CPDF_IndirectObjectHolder obj_holder;
+  CPDF_Dictionary* root = new CPDF_Dictionary;
+  obj_holder.AddIndirectObject(root);
+  root->SetAtName("T", "foo");
+  name = FPDF_GetFullName(root);
+  EXPECT_STREQ("foo", name.UTF8Encode().c_str());
+
+  CPDF_Dictionary* dict1 = new CPDF_Dictionary;
+  obj_holder.AddIndirectObject(dict1);
+  dict1->SetAtName("T", "bar");
+  root->SetAtReference("Parent", &obj_holder, dict1);
+  name = FPDF_GetFullName(root);
+  EXPECT_STREQ("bar.foo", name.UTF8Encode().c_str());
+
+  CPDF_Dictionary* dict2 = new CPDF_Dictionary;
+  obj_holder.AddIndirectObject(dict2);
+  dict1->SetAt("Parent", dict2);
+  name = FPDF_GetFullName(root);
+  EXPECT_STREQ("bar.foo", name.UTF8Encode().c_str());
+
+  CPDF_Dictionary* dict3 = new CPDF_Dictionary;
+  obj_holder.AddIndirectObject(dict3);
+  dict3->SetAtName("T", "qux");
+  dict2->SetAtReference("Parent", &obj_holder, dict3);
+  name = FPDF_GetFullName(root);
+  EXPECT_STREQ("qux.bar.foo", name.UTF8Encode().c_str());
+
+  dict3->SetAtReference("Parent", &obj_holder, root);
+  name = FPDF_GetFullName(root);
+  EXPECT_STREQ("qux.bar.foo", name.UTF8Encode().c_str());
+  name = FPDF_GetFullName(dict1);
+  EXPECT_STREQ("foo.qux.bar", name.UTF8Encode().c_str());
+  name = FPDF_GetFullName(dict2);
+  EXPECT_STREQ("bar.foo.qux", name.UTF8Encode().c_str());
+  name = FPDF_GetFullName(dict3);
+  EXPECT_STREQ("bar.foo.qux", name.UTF8Encode().c_str());
+}
-- 
cgit v1.2.3