From b8627c9d13884d48943d8a7a5381eaf0bb2c08d9 Mon Sep 17 00:00:00 2001 From: ochang Date: Mon, 11 Apr 2016 13:47:41 -0700 Subject: Fix integer issues leading to out of bounds access in fx_ge_text.cpp. - Using |-skew| to get positive index, which doesn't work when skew is INT_MIN - Incorrect logic when determining when to use |-skew| as an index. R=tsepez@chromium.org,weili@chromium.org BUG=chromium:601362 Review URL: https://codereview.chromium.org/1875673004 --- BUILD.gn | 1 + core/fxge/ge/fx_ge_text.cpp | 26 +++++--- core/fxge/ge/fx_ge_text_embeddertest.cpp | 18 ++++++ pdfium.gyp | 1 + testing/resources/bug_601362.pdf | 105 +++++++++++++++++++++++++++++++ 5 files changed, 143 insertions(+), 8 deletions(-) create mode 100644 core/fxge/ge/fx_ge_text_embeddertest.cpp create mode 100644 testing/resources/bug_601362.pdf diff --git a/BUILD.gn b/BUILD.gn index 380eee9fdd..eecdb5ac4d 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -1687,6 +1687,7 @@ test("pdfium_embeddertests") { "core/fpdfapi/fpdf_parser/fpdf_parser_decode_embeddertest.cpp", "core/fpdfapi/fpdf_render/fpdf_render_loadimage_embeddertest.cpp", "core/fpdfapi/fpdf_render/fpdf_render_pattern_embeddertest.cpp", + "core/fxge/ge/fx_ge_text_embeddertest.cpp", "fpdfsdk/fpdf_dataavail_embeddertest.cpp", "fpdfsdk/fpdfdoc_embeddertest.cpp", "fpdfsdk/fpdfedit_embeddertest.cpp", diff --git a/core/fxge/ge/fx_ge_text.cpp b/core/fxge/ge/fx_ge_text.cpp index 44ab9f7ab5..e074fa4f26 100644 --- a/core/fxge/ge/fx_ge_text.cpp +++ b/core/fxge/ge/fx_ge_text.cpp @@ -4,6 +4,8 @@ // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com +#include + #include "core/fxcodec/include/fx_codec.h" #include "core/fxge/ge/fx_text_int.h" #include "core/fxge/include/fx_freetype.h" @@ -1567,10 +1569,14 @@ CFX_GlyphBitmap* CFX_FaceCache::RenderGlyph(CFX_Font* pFont, skew = pSubstFont->m_ItalicAngle; } if (skew) { - // skew is nonpositive so -skew is used as the index. - skew = -skew <= static_cast(ANGLESKEW_ARRAY_SIZE) - ? -58 - : -g_AngleSkew[-skew]; + // |skew| is nonpositive so |-skew| is used as the index. We need to make + // sure |skew| != INT_MIN since -INT_MIN is undefined. + if (skew <= 0 && skew != std::numeric_limits::min() && + static_cast(-skew) < ANGLESKEW_ARRAY_SIZE) { + skew = -g_AngleSkew[-skew]; + } else { + skew = -58; + } if (pFont->IsVertical()) ft_matrix.yx += ft_matrix.yy * skew / 100; else @@ -1833,10 +1839,14 @@ CFX_PathData* CFX_Font::LoadGlyphPath(uint32_t glyph_index, int dest_width) { if (m_pSubstFont) { if (m_pSubstFont->m_ItalicAngle) { int skew = m_pSubstFont->m_ItalicAngle; - // skew is nonpositive so -skew is used as the index. - skew = -skew <= static_cast(ANGLESKEW_ARRAY_SIZE) - ? -58 - : -g_AngleSkew[-skew]; + // |skew| is nonpositive so |-skew| is used as the index. We need to make + // sure |skew| != INT_MIN since -INT_MIN is undefined. + if (skew <= 0 && skew != std::numeric_limits::min() && + static_cast(-skew) < ANGLESKEW_ARRAY_SIZE) { + skew = -g_AngleSkew[-skew]; + } else { + skew = -58; + } if (m_bVertical) ft_matrix.yx += ft_matrix.yy * skew / 100; else diff --git a/core/fxge/ge/fx_ge_text_embeddertest.cpp b/core/fxge/ge/fx_ge_text_embeddertest.cpp new file mode 100644 index 0000000000..045b6dc869 --- /dev/null +++ b/core/fxge/ge/fx_ge_text_embeddertest.cpp @@ -0,0 +1,18 @@ +// 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 "testing/embedder_test.h" +#include "testing/gtest/include/gtest/gtest.h" + +class FXGETextEmbedderTest : public EmbedderTest {}; + +TEST_F(FXGETextEmbedderTest, BadItalic) { + // Shouldn't crash. + EXPECT_TRUE(OpenDocument("bug_601362.pdf")); + FPDF_PAGE page = LoadPage(0); + EXPECT_NE(nullptr, page); + FPDF_BITMAP bitmap = RenderPage(page); + FPDFBitmap_Destroy(bitmap); + UnloadPage(page); +} diff --git a/pdfium.gyp b/pdfium.gyp index 02a6f1eacb..17384309cb 100644 --- a/pdfium.gyp +++ b/pdfium.gyp @@ -972,6 +972,7 @@ 'core/fpdfapi/fpdf_parser/fpdf_parser_decode_embeddertest.cpp', 'core/fpdfapi/fpdf_render/fpdf_render_loadimage_embeddertest.cpp', 'core/fpdfapi/fpdf_render/fpdf_render_pattern_embeddertest.cpp', + 'core/fxge/ge/fx_ge_text_embeddertest.cpp', 'fpdfsdk/fpdf_dataavail_embeddertest.cpp', 'fpdfsdk/fpdfdoc_embeddertest.cpp', 'fpdfsdk/fpdfedit_embeddertest.cpp', diff --git a/testing/resources/bug_601362.pdf b/testing/resources/bug_601362.pdf new file mode 100644 index 0000000000..d21a83405e --- /dev/null +++ b/testing/resources/bug_601362.pdf @@ -0,0 +1,105 @@ +%PDF-1.5 +%¿÷¢þ +1 0 obj +<< /Lang (en-US) /MarkInfo << /Marked true >> /Pages 3 0 R /Type /Catalog >> +endobj +2 0 obj +<< /CreationDate (D:20160411190039+00'00') /Creator (Microsoft Word) /ModDate (D:20160411190039+00'00') >> +endobj +3 0 obj +<< /Count 1 /Kids [ 4 0 R ] /Type /Pages >> +endobj +4 0 obj +<< /Contents 5 0 R /Group << /CS /DeviceRGB /S /Transparency /Type /Group >> /MediaBox [ 0 0 612 792 ] /Parent 3 0 R /Resources << /ExtGState << /GS7 6 0 R /GS8 7 0 R >> /Font << /F1 8 0 R /F2 9 0 R >> /ProcSet [ /PDF /Text /ImageB /ImageC /ImageI ] >> /StructParents 0 /Tabs /S /Type /Page >> +endobj +5 0 obj +<< /Length 464 >> +stream + /P <> BDC q +0.00000912 0 612 792 re +W* n +BT +/F1 11 Tf +1 0 0 1 72.025 709.72 Tm +/GS7 gs +0 g +/GS8 gs +0 G +[(T)10(est)] TJ +ET +Q +q +0.00000912 0 612 792 re +W* n +BT +/F2 11 Tf +1 0 0 1 90.8 709.72 Tm +0 g +0 G +[( )] TJ +ET +Q +q +0.00000912 0 612 792 re +W* n +BT +/F2 11 Tf +1 0 0 1 93.3 709.72 Tm +0 g +0 G +[(f)10(il)5(e.)] TJ +ET +Q +q +0.00000912 0 612 792 re +W* n +BT +/F1 11 Tf +1 0 0 1 109.8 709.72 Tm +0 g +0 G +[( )] TJ +ET +Q + EMC endstream +endobj +6 0 obj +<< /BM /Normal /Type /ExtGState /ca 1 >> +endobj +7 0 obj +<< /BM /Normal /CA 1 /Type /ExtGState >> +endobj +8 0 obj +<< /BaseFont /ABCDEE+Calibri /Encoding /WinAnsiEncoding /FirstChar 32 /FontDescriptor 10 0 R /LastChar 116 /Name /F1 /Subtype /TrueType /Type /Font /Widths 11 0 R >> +endobj +9 0 obj +<< /BaseFont /ABCDEE+Calibri,Italic /Encoding /WinAnsiEncoding /FirstChar 32 /FontDescriptor 12 0 R /LastChar 108 /Name /F2 /Subtype /TrueType /Type /Font >> +endobj +10 0 obj +<< /Ascent 750 /AvgWidth 521 /CapHeight 750 /Descent -250 /Flags 32 /FontBBox [ -503 -250 1240 750 ] /FontName /ABCDEE+Calibri /FontWeight 400 /ItalicAngle 0 /MaxWidth 1743 /StemV 52 /Type /FontDescriptor /XHeight 250 >> +endobj +11 0 obj +[ 226 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 487 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 498 0 0 0 0 0 0 0 0 0 0 0 0 0 391 335 ] +endobj +12 0 obj +<< /Ascent 750 /AvgWidth 521 /CapHeight 750 /Descent -250 /Flags 32 /FontBBox [ -725 -250 1260 750 ] /FontName /ABCDEE+Calibri,Italic /FontWeight 400 /ItalicAngle 2147483649 /MaxWidth 1984 /StemV 52 /Type /FontDescriptor /XHeight 250 >> +endobj +xref +0 13 +0000000000 65535 f +0000000015 00000 n +0000000107 00000 n +0000000229 00000 n +0000000288 00000 n +0000000597 00000 n +0000001111 00000 n +0000001167 00000 n +0000001223 00000 n +0000001404 00000 n +0000001577 00000 n +0000001814 00000 n +0000002014 00000 n +trailer << /Info 2 0 R /Root 1 0 R /Size 13 /ID [<205a1ea7cf6e0a45ac68694d38f6b9f7><32b8937a599b7da08d5dc591fd416f1b>] >> +startxref +2267 +%%EOF -- cgit v1.2.3