commit c1d4c192ebfa8f84e7d7ac2005b23e36c3365d69
author Lei Zhang <thestig@chromium.org> Tue Mar 29 00:53:10 2022 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Mar 29 00:53:10 2022 +0000
tree 8a6f0ae27de7f1f562f3d69c9e4a8232ba761366
parent 4485263fee62e0f3ee57dcb254921182ae33343a

Make sure some array accesses in CFX_FontMapper never go out of bounds.

Existing callers to GetFaceName() and RawBytesForIndex() never pass in
an out of bound index value. Nevertheless, use CHECK_LT() to ensure to
make sure it stays that way.

Also remove an existing check in RawBytesForIndex() for a null
`m_pFontInfo`, which can never be true because that would imply there
are no font faces. In which case, RawBytesForIndex() should never be
called.

Change-Id: I4b48bf4881308ca31653e8579f544c7c6a30e716
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/91932
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>

core/fxge/cfx_fontmapper.cpp

diff --git a/core/fxge/cfx_fontmapper.cpp b/core/fxge/cfx_fontmapper.cpp
index 02fbbb5..b9c9dc8 100644
--- a/core/fxge/cfx_fontmapper.cpp
+++ b/core/fxge/cfx_fontmapper.cpp
@@ -22,7 +22,7 @@
 #include "core/fxge/cfx_substfont.h"
 #include "core/fxge/fx_font.h"
 #include "core/fxge/systemfontinfo_iface.h"
-#include "third_party/base/check.h"
+#include "third_party/base/check_op.h"
 #include "third_party/base/containers/contains.h"
 #include "third_party/base/cxx17_backports.h"
 
@@ -691,6 +691,11 @@
   return m_FaceArray.size();
 }
 
+ByteString CFX_FontMapper::GetFaceName(size_t index) const {
+  CHECK_LT(index, m_FaceArray.size());
+  return m_FaceArray[index].name;
+}
+
 bool CFX_FontMapper::HasInstalledFont(ByteStringView name) const {
   for (const auto& font : m_InstalledTTFonts) {
     if (font == name)
@@ -731,8 +736,7 @@
 std::unique_ptr<uint8_t, FxFreeDeleter> CFX_FontMapper::RawBytesForIndex(
     size_t index,
     size_t* returned_length) {
-  if (!m_pFontInfo)
-    return nullptr;
+  CHECK_LT(index, m_FaceArray.size());
 
   void* hFont = m_pFontInfo->MapFont(0, false, FX_Charset::kDefault, 0,
                                      GetFaceName(index));

core/fxge/cfx_fontmapper.h

diff --git a/core/fxge/cfx_fontmapper.h b/core/fxge/cfx_fontmapper.h
index 2ad6c3e..2a7d7c8 100644
--- a/core/fxge/cfx_fontmapper.h
+++ b/core/fxge/cfx_fontmapper.h
@@ -71,7 +71,8 @@
                                     CFX_SubstFont* pSubstFont);
 
   size_t GetFaceSize() const;
-  ByteString GetFaceName(size_t index) const { return m_FaceArray[index].name; }
+  // `index` must be less than GetFaceSize().
+  ByteString GetFaceName(size_t index) const;
   bool HasInstalledFont(ByteStringView name) const;
   bool HasLocalizedFont(ByteStringView name) const;
 
@@ -83,6 +84,7 @@
 #endif  // BUILDFLAG(IS_WIN)
 
 #ifdef PDF_ENABLE_XFA
+  // `index` must be less than GetFaceSize().
   std::unique_ptr<uint8_t, FxFreeDeleter> RawBytesForIndex(
       size_t index,
       size_t* returned_length);