commit 8f0455e5a7eddcd347d871391b84964bdc399bb0
author Hui Yingst <nigi@chromium.org> Wed Sep 30 23:32:14 2020 +0000
committer Chromium commit bot <commit-bot@chromium.org> Wed Sep 30 23:32:14 2020 +0000
tree 2cd382984d35f9e9d75b32a4b3d320c53577cd31
parent 60eb21841013d355c1adb06d3984423ad80d5580

Fix potential segmentation faults during annotation rendering.

To fix crbug.com/237527, CPDF_Annot::DrawBorder() needs to be called,
which will trigger the following issues:

- In PDF ISO-32000-1:2008 Specification, table 166 indicates that the
  acceptable values for Key "S" in a border style entry are "S", "D",
  "B", "I", "U", which are all single characters. |style[1]| in
  DrawBorder() will be out of bounds.

- CPDF_Annot::DrawBorder() always gets its parameter |pDevice| as
  nullptr from its caller , which will trigger segmentation fault when
  CFX_RenderDevice::GetDeviceType() is called.

This CL fixes the above issues by giving the correct index number for
accessing |style| in CPDF_Annot::DrawBorder(), and making
CPDF_AnnotList::DisplayAnnots() accept a valid CFX_RenderDevice pointer
as a parameter.

This CL also adds a TODO comment regarding adding support for rendering
"Underline" border style.

Bug: chromium:237527
Change-Id: Ie5a23d7a66fa0daba5d22a03eae5c36aae2d3d31
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/74170
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Hui Yingst <nigi@chromium.org>

core/fpdfdoc/cpdf_annot.cpp

diff --git a/core/fpdfdoc/cpdf_annot.cpp b/core/fpdfdoc/cpdf_annot.cpp
index 4559fc4..14533f5 100644
--- a/core/fpdfdoc/cpdf_annot.cpp
+++ b/core/fpdfdoc/cpdf_annot.cpp
@@ -482,7 +482,7 @@
   } else {
     ByteString style = pBS->GetStringFor("S");
     pDashArray = pBS->GetArrayFor("D");
-    style_char = style[1];
+    style_char = style[0];
     width = pBS->GetNumberFor("W");
   }
   if (width <= 0) {
@@ -498,6 +498,12 @@
   }
   CFX_GraphStateData graph_state;
   graph_state.m_LineWidth = width;
+  if (style_char == 'U') {
+    // TODO(https://crbug.com/237527): Handle the "Underline" border style
+    // instead of drawing the rectangle border.
+    return;
+  }
+
   if (style_char == 'D') {
     if (pDashArray) {
       graph_state.m_DashArray =

core/fpdfdoc/cpdf_annotlist.cpp

diff --git a/core/fpdfdoc/cpdf_annotlist.cpp b/core/fpdfdoc/cpdf_annotlist.cpp
index 64ecffb..6b53b64 100644
--- a/core/fpdfdoc/cpdf_annotlist.cpp
+++ b/core/fpdfdoc/cpdf_annotlist.cpp
@@ -289,6 +289,7 @@
 }
 
 void CPDF_AnnotList::DisplayAnnots(CPDF_Page* pPage,
+                                   CFX_RenderDevice* device,
                                    CPDF_RenderContext* pContext,
                                    bool bPrinting,
                                    const CFX_Matrix* pMatrix,
@@ -297,6 +298,6 @@
   uint32_t dwAnnotFlags = bShowWidget ? pdfium::annotation_flags::kInvisible |
                                             pdfium::annotation_flags::kHidden
                                       : pdfium::annotation_flags::kInvisible;
-  DisplayAnnots(pPage, nullptr, pContext, bPrinting, pMatrix, dwAnnotFlags,
+  DisplayAnnots(pPage, device, pContext, bPrinting, pMatrix, dwAnnotFlags,
                 pOptions, nullptr);
 }

core/fpdfdoc/cpdf_annotlist.h

diff --git a/core/fpdfdoc/cpdf_annotlist.h b/core/fpdfdoc/cpdf_annotlist.h
index 85075cf..583ffce 100644
--- a/core/fpdfdoc/cpdf_annotlist.h
+++ b/core/fpdfdoc/cpdf_annotlist.h
@@ -28,6 +28,7 @@
   ~CPDF_AnnotList() override;
 
   void DisplayAnnots(CPDF_Page* pPage,
+                     CFX_RenderDevice* device,
                      CPDF_RenderContext* pContext,
                      bool bPrinting,
                      const CFX_Matrix* pMatrix,

fpdfsdk/cpdfsdk_renderpage.cpp

diff --git a/fpdfsdk/cpdfsdk_renderpage.cpp b/fpdfsdk/cpdfsdk_renderpage.cpp
index ab3ce61..42fd11e 100644
--- a/fpdfsdk/cpdfsdk_renderpage.cpp
+++ b/fpdfsdk/cpdfsdk_renderpage.cpp
@@ -70,8 +70,9 @@
     pContext->m_pAnnots = std::move(pOwnedList);
     bool bPrinting =
         pContext->m_pDevice->GetDeviceType() != DeviceType::kDisplay;
-    pList->DisplayAnnots(pPage, pContext->m_pContext.get(), bPrinting, &matrix,
-                         false, nullptr);
+    pList->DisplayAnnots(pPage, pContext->m_pDevice.get(),
+                         pContext->m_pContext.get(), bPrinting, &matrix, false,
+                         nullptr);
   }
 
   pContext->m_pRenderer = std::make_unique<CPDF_ProgressiveRenderer>(