Fixup ASSERT in Bidi handling; Add bidi fuzzer.

This CL converts several asserts in the FX_Bidi code to continue instead
of asserting in the face of unexpected input.

A BIDI fuzzer has been added as well.

Bug: chromium:839695
Change-Id: If61f822bde7442c008d50be58f7cecffb6e5d658
Reviewed-on: https://pdfium-review.googlesource.com/32191
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: dsinclair <dsinclair@chromium.org>

8 files changed, 118 insertions, 9 deletions

diff --git a/BUILD.gn b/BUILD.gn
index c1e127010f..6593935429 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -2946,6 +2946,7 @@ test("pdfium_unittests") {
       "xfa/fde/cfde_texteditengine_unittest.cpp",
       "xfa/fgas/crt/cfgas_formatstring_unittest.cpp",
       "xfa/fgas/layout/cfx_rtfbreak_unittest.cpp",
+      "xfa/fgas/layout/cfx_txtbreak_unittest.cpp",
       "xfa/fwl/cfx_barcode_unittest.cpp",
       "xfa/fxfa/cxfa_ffbarcode_unittest.cpp",
       "xfa/fxfa/cxfa_textparser_unittest.cpp",
diff --git a/core/fxcrt/fx_bidi.cpp b/core/fxcrt/fx_bidi.cpp
index 48504e5821..7261d80af3 100644
--- a/core/fxcrt/fx_bidi.cpp
+++ b/core/fxcrt/fx_bidi.cpp
@@ -329,12 +329,11 @@ class CFX_BidiLine {
 
     int32_t iLevelCur = 0;
     int32_t iState = FX_BWSxl;
-    size_t i = 0;
     size_t iNum = 0;
     int32_t iClsCur;
     int32_t iClsRun;
     int32_t iClsNew;
-    int32_t iAction;
+    size_t i = 0;
     for (; i <= iCount; ++i) {
       CFX_Char* pTC = &(*chars)[i];
       iClsCur = pTC->m_iBidiClass;
@@ -365,9 +364,10 @@ class CFX_BidiLine {
           continue;
         }
       }
+      if (iClsCur > FX_BIDICLASS_BN)
+        continue;
 
-      ASSERT(iClsCur <= FX_BIDICLASS_BN);
-      iAction = gc_FX_BidiWeakActions[iState][iClsCur];
+      int32_t iAction = gc_FX_BidiWeakActions[iState][iClsCur];
       iClsRun = GetDeferredType(iAction);
       if (iClsRun != FX_BWAXX && iNum > 0) {
         SetDeferredRun(chars, true, i, iNum, iClsRun);
@@ -412,8 +412,9 @@ class CFX_BidiLine {
           ++iNum;
         continue;
       }
+      if (iClsCur >= FX_BIDICLASS_AL)
+        continue;
 
-      ASSERT(iClsCur < FX_BIDICLASS_AL);
       iAction = gc_FX_BidiNeutralActions[iState][iClsCur];
       iClsRun = GetDeferredNeutrals(iAction, iLevel);
       if (iClsRun != FX_BIDICLASS_N && iNum > 0) {
@@ -445,8 +446,9 @@ class CFX_BidiLine {
       int32_t iCls = (*chars)[i].m_iBidiClass;
       if (iCls == FX_BIDICLASS_BN)
         continue;
+      if (iCls <= FX_BIDICLASS_ON || iCls >= FX_BIDICLASS_AL)
+        continue;
 
-      ASSERT(iCls > FX_BIDICLASS_ON && iCls < FX_BIDICLASS_AL);
       int32_t iLevel = (*chars)[i].m_iBidiLevel;
       iLevel += gc_FX_BidiAddLevel[FX_IsOdd(iLevel)][iCls - 1];
       (*chars)[i].m_iBidiLevel = (int16_t)iLevel;
diff --git a/testing/libfuzzer/BUILD.gn b/testing/libfuzzer/BUILD.gn
index 2b2c19389b..aeceb259cd 100644
--- a/testing/libfuzzer/BUILD.gn
+++ b/testing/libfuzzer/BUILD.gn
@@ -38,6 +38,7 @@ group("libfuzzer") {
   ]
   if (pdf_enable_xfa) {
     deps += [
+      ":pdf_bidi_fuzzer",
       ":pdf_cfx_barcode_fuzzer",
       ":pdf_codec_jpeg_fuzzer",
       ":pdf_css_fuzzer",
@@ -81,6 +82,12 @@ template("pdfium_fuzzer") {
 }
 
 if (pdf_enable_xfa) {
+  pdfium_fuzzer("pdf_bidi_fuzzer") {
+    sources = [
+      "pdf_bidi_fuzzer.cc",
+    ]
+  }
+
   pdfium_fuzzer("pdf_cfx_barcode_fuzzer") {
     sources = [
       "pdf_cfx_barcode_fuzzer.cc",
diff --git a/testing/libfuzzer/pdf_bidi_fuzzer.cc b/testing/libfuzzer/pdf_bidi_fuzzer.cc
new file mode 100644
index 0000000000..8e52688a10
--- /dev/null
+++ b/testing/libfuzzer/pdf_bidi_fuzzer.cc
@@ -0,0 +1,36 @@
+// Copyright 2018 The 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 <cstdint>
+
+#include "core/fxcrt/fx_bidi.h"
+#include "core/fxcrt/widestring.h"
+#include "core/fxge/cfx_font.h"
+#include "third_party/base/span.h"
+#include "xfa/fgas/font/cfgas_fontmgr.h"
+#include "xfa/fgas/font/cfgas_gefont.h"
+#include "xfa/fgas/layout/cfx_rtfbreak.h"
+
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
+  auto fontmgr = pdfium::MakeUnique<CFGAS_FontMgr>();
+
+  auto font = pdfium::MakeUnique<CFX_Font>();
+  font->LoadSubst("Arial", true, 0, FXFONT_FW_NORMAL, 0, 0, 0);
+  assert(font);
+
+  CFX_RTFBreak rtf_break(FX_LAYOUTSTYLE_ExpandTab);
+  rtf_break.SetLineBreakTolerance(1);
+  rtf_break.SetFont(CFGAS_GEFont::LoadFont(std::move(font), fontmgr.get()));
+  rtf_break.SetFontSize(12);
+
+  WideString input =
+      WideString::FromUTF16LE(reinterpret_cast<const unsigned short*>(data),
+                              size / sizeof(unsigned short));
+  for (auto& ch : input)
+    rtf_break.AppendChar(ch);
+
+  auto chars = rtf_break.GetCurrentLineForTesting()->m_LineChars;
+  FX_BidiLine(&chars, chars.size());
+  return 0;
+}
diff --git a/xfa/fgas/layout/cfx_break.h b/xfa/fgas/layout/cfx_break.h
index 7356cff3ba..cc985b7c59 100644
--- a/xfa/fgas/layout/cfx_break.h
+++ b/xfa/fgas/layout/cfx_break.h
@@ -57,6 +57,7 @@ class CFX_Break {
   void ClearBreakPieces();
 
   CFX_Char* GetLastChar(int32_t index, bool bOmitChar, bool bRichText) const;
+  const CFX_BreakLine* GetCurrentLineForTesting() const { return m_pCurLine; }
 
  protected:
   explicit CFX_Break(uint32_t dwLayoutStyles);
diff --git a/xfa/fgas/layout/cfx_rtfbreak.h b/xfa/fgas/layout/cfx_rtfbreak.h
index 3f302ca7a8..c2320fab7f 100644
--- a/xfa/fgas/layout/cfx_rtfbreak.h
+++ b/xfa/fgas/layout/cfx_rtfbreak.h
@@ -63,8 +63,6 @@ class CFX_RTFBreak : public CFX_Break {
 
   CFX_BreakType AppendChar(wchar_t wch);
 
-  CFX_BreakLine* GetCurrentLineForTesting() const { return m_pCurLine; }
-
  private:
   void AppendChar_Combination(CFX_Char* pCurChar);
   void AppendChar_Tab(CFX_Char* pCurChar);
diff --git a/xfa/fgas/layout/cfx_rtfbreak_unittest.cpp b/xfa/fgas/layout/cfx_rtfbreak_unittest.cpp
index 5f24631315..6a0cd3f187 100644
--- a/xfa/fgas/layout/cfx_rtfbreak_unittest.cpp
+++ b/xfa/fgas/layout/cfx_rtfbreak_unittest.cpp
@@ -7,7 +7,10 @@
 #include "xfa/fgas/layout/cfx_rtfbreak.h"
 
 #include <memory>
+#include <utility>
 
+#include "core/fxcrt/fx_bidi.h"
+#include "core/fxge/cfx_font.h"
 #include "core/fxge/cfx_gemodule.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/test_support.h"
@@ -20,7 +23,7 @@ class CFX_RTFBreakTest : public testing::Test {
   void SetUp() override {
     font_ =
         CFGAS_GEFont::LoadFont(L"Arial Black", 0, 0, GetGlobalFontManager());
-    ASSERT(font_.Get() != nullptr);
+    ASSERT(font_.Get());
   }
 
   std::unique_ptr<CFX_RTFBreak> CreateBreak(int32_t args) {
@@ -72,3 +75,17 @@ TEST_F(CFX_RTFBreakTest, ControlCharacters) {
   ASSERT_EQ(1, b->CountBreakPieces());
   EXPECT_EQ(L"\v", b->GetBreakPieceUnstable(0)->GetString());
 }
+
+TEST_F(CFX_RTFBreakTest, BidiLine) {
+  auto rtf_break = CreateBreak(FX_LAYOUTSTYLE_ExpandTab);
+  rtf_break->SetLineBreakTolerance(1);
+  rtf_break->SetFontSize(12);
+
+  WideString input = WideString::FromUTF8(ByteStringView("\xa\x0\xa\xa", 4));
+  for (auto& ch : input)
+    rtf_break->AppendChar(ch);
+
+  auto chars = rtf_break->GetCurrentLineForTesting()->m_LineChars;
+  FX_BidiLine(&chars, chars.size());
+  EXPECT_EQ(3u, chars.size());
+}
diff --git a/xfa/fgas/layout/cfx_txtbreak_unittest.cpp b/xfa/fgas/layout/cfx_txtbreak_unittest.cpp
new file mode 100644
index 0000000000..8cac2fa036
--- /dev/null
+++ b/xfa/fgas/layout/cfx_txtbreak_unittest.cpp
@@ -0,0 +1,47 @@
+// Copyright 2018 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 "xfa/fgas/layout/cfx_txtbreak.h"
+
+#include <utility>
+
+#include "core/fxcrt/fx_bidi.h"
+#include "core/fxge/cfx_font.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "testing/test_support.h"
+#include "third_party/base/ptr_util.h"
+#include "xfa/fgas/font/cfgas_fontmgr.h"
+#include "xfa/fgas/font/cfgas_gefont.h"
+
+class CFX_TxtBreakTest : public testing::Test {
+ public:
+  void SetUp() override {
+    font_ =
+        CFGAS_GEFont::LoadFont(L"Arial Black", 0, 0, GetGlobalFontManager());
+    ASSERT(font_.Get());
+  }
+
+  std::unique_ptr<CFX_TxtBreak> CreateBreak() {
+    auto b = pdfium::MakeUnique<CFX_TxtBreak>();
+    b->SetFont(font_);
+    return b;
+  }
+
+ private:
+  RetainPtr<CFGAS_GEFont> font_;
+};
+
+TEST_F(CFX_TxtBreakTest, BidiLine) {
+  auto txt_break = CreateBreak();
+  txt_break->SetLineBreakTolerance(1);
+  txt_break->SetFontSize(12);
+
+  WideString input = WideString::FromUTF8(ByteStringView("\xa\x0\xa\xa", 4));
+  for (auto& ch : input)
+    txt_break->AppendChar(ch);
+
+  auto chars = txt_break->GetCurrentLineForTesting()->m_LineChars;
+  FX_BidiLine(&chars, chars.size());
+  EXPECT_EQ(3u, chars.size());
+}