From 5e0b271b69355b5692b6afd1cd2c04d08c3b380c Mon Sep 17 00:00:00 2001 From: Dan Sinclair Date: Thu, 10 May 2018 21:21:05 +0000 Subject: 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 Commit-Queue: dsinclair --- BUILD.gn | 1 + core/fxcrt/fx_bidi.cpp | 14 +++++---- testing/libfuzzer/BUILD.gn | 7 +++++ testing/libfuzzer/pdf_bidi_fuzzer.cc | 36 +++++++++++++++++++++++ xfa/fgas/layout/cfx_break.h | 1 + xfa/fgas/layout/cfx_rtfbreak.h | 2 -- xfa/fgas/layout/cfx_rtfbreak_unittest.cpp | 19 ++++++++++++- xfa/fgas/layout/cfx_txtbreak_unittest.cpp | 47 +++++++++++++++++++++++++++++++ 8 files changed, 118 insertions(+), 9 deletions(-) create mode 100644 testing/libfuzzer/pdf_bidi_fuzzer.cc create mode 100644 xfa/fgas/layout/cfx_txtbreak_unittest.cpp 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 + +#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(); + + auto font = pdfium::MakeUnique(); + 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(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 +#include +#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 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 + +#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 CreateBreak() { + auto b = pdfium::MakeUnique(); + b->SetFont(font_); + return b; + } + + private: + RetainPtr 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()); +} -- cgit v1.2.3