From 676947ce0204914da1d8fb159730432c0fb0a3a2 Mon Sep 17 00:00:00 2001
From: Tom Sepez <tsepez@chromium.org>
Date: Thu, 24 Mar 2016 11:09:45 -0700
Subject: Added bounds checking to GetNameFromTT to handle corrupt files.

Patch by forshaw.

This patch adds bounds checking to the names buffer passed to GetNameFromTT.
There are observed crashes in this function where data is read outside of
the bounds allocated and passed to GetNameFromTT. There's no reason that
this function should ever try and read outside of the allocated bounds.

BUG=583037
TBR=forshaw@chromium.org

patch from issue 1829013002 at patchset 40001 (http://crrev.com/1829013002#ps40001)

Review URL: https://codereview.chromium.org/1830243003 .
---
 core/fpdfapi/fpdf_edit/fpdf_edit_doc.cpp |  2 +-
 core/fxge/ge/fx_ge_fontmap.cpp           | 56 ++++++++++++++++++++++++--------
 core/include/fxge/fx_font.h              |  4 ++-
 3 files changed, 46 insertions(+), 16 deletions(-)

diff --git a/core/fpdfapi/fpdf_edit/fpdf_edit_doc.cpp b/core/fpdfapi/fpdf_edit/fpdf_edit_doc.cpp
index 7688bb08ce..cf79fdc47d 100644
--- a/core/fpdfapi/fpdf_edit/fpdf_edit_doc.cpp
+++ b/core/fpdfapi/fpdf_edit/fpdf_edit_doc.cpp
@@ -234,7 +234,7 @@ CFX_ByteString _FPDF_GetPSNameFromTT(HDC hDC) {
   if (size != GDI_ERROR) {
     LPBYTE buffer = FX_Alloc(BYTE, size);
     ::GetFontData(hDC, 'eman', 0, buffer, size);
-    result = GetNameFromTT(buffer, 6);
+    result = GetNameFromTT(buffer, size, 6);
     FX_Free(buffer);
   }
   return result;
diff --git a/core/fxge/ge/fx_ge_fontmap.cpp b/core/fxge/ge/fx_ge_fontmap.cpp
index 441c87b18d..27b82f5803 100644
--- a/core/fxge/ge/fx_ge_fontmap.cpp
+++ b/core/fxge/ge/fx_ge_fontmap.cpp
@@ -681,20 +681,44 @@ void CFX_FontMapper::SetSystemFontInfo(IFX_SystemFontInfo* pFontInfo) {
   }
   m_pFontInfo = pFontInfo;
 }
+static CFX_ByteString GetStringFromTable(const uint8_t* string_ptr,
+                                         FX_DWORD string_ptr_length,
+                                         uint16_t offset,
+                                         uint16_t length) {
+  if (string_ptr_length < offset + length) {
+    return CFX_ByteString();
+  }
+  return CFX_ByteStringC(string_ptr + offset, length);
+}
+CFX_ByteString GetNameFromTT(const uint8_t* name_table,
+                             FX_DWORD name_table_size,
+                             FX_DWORD name_id) {
+  if (!name_table || name_table_size < 6) {
+    return CFX_ByteString();
+  }
+  FX_DWORD name_count = GET_TT_SHORT(name_table + 2);
+  FX_DWORD string_offset = GET_TT_SHORT(name_table + 4);
+  // We will ignore the possibility of overlap of structures and
+  // string table as if it's all corrupt there's not a lot we can do.
+  if (name_table_size < string_offset) {
+    return CFX_ByteString();
+  }
 
-CFX_ByteString GetNameFromTT(const uint8_t* name_table, FX_DWORD name_id) {
-  const uint8_t* ptr = name_table + 2;
-  int name_count = GET_TT_SHORT(ptr);
-  int string_offset = GET_TT_SHORT(ptr + 2);
   const uint8_t* string_ptr = name_table + string_offset;
-  ptr += 4;
-  for (int i = 0; i < name_count; i++) {
-    if (GET_TT_SHORT(ptr + 6) == name_id && GET_TT_SHORT(ptr) == 1 &&
-        GET_TT_SHORT(ptr + 2) == 0) {
-      return CFX_ByteStringC(string_ptr + GET_TT_SHORT(ptr + 10),
-                             GET_TT_SHORT(ptr + 8));
+  FX_DWORD string_ptr_size = name_table_size - string_offset;
+  name_table += 6;
+  name_table_size -= 6;
+  if (name_table_size < name_count * 12) {
+    return CFX_ByteString();
+  }
+
+  for (FX_DWORD i = 0; i < name_count; i++, name_table += 12) {
+    if (GET_TT_SHORT(name_table + 6) == name_id &&
+        GET_TT_SHORT(name_table) == 1 && GET_TT_SHORT(name_table + 2) == 0) {
+      return GetStringFromTable(string_ptr, string_ptr_size,
+                                GET_TT_SHORT(name_table + 10),
+                                GET_TT_SHORT(name_table + 8));
     }
-    ptr += 12;
   }
   return CFX_ByteString();
 }
@@ -711,7 +735,8 @@ CFX_ByteString CFX_FontMapper::GetPSNameFromTT(void* hFont) {
   uint8_t* buffer_ptr = buffer.data();
   FX_DWORD bytes_read =
       m_pFontInfo->GetFontData(hFont, kTableNAME, buffer_ptr, size);
-  return (bytes_read == size) ? GetNameFromTT(buffer_ptr, 6) : CFX_ByteString();
+  return bytes_read == size ? GetNameFromTT(buffer_ptr, bytes_read, 6)
+                            : CFX_ByteString();
 }
 
 void CFX_FontMapper::AddInstalledFont(const CFX_ByteString& name, int charset) {
@@ -1442,8 +1467,11 @@ void CFX_FolderFontInfo::ReportFace(const CFX_ByteString& path,
   if (names.IsEmpty()) {
     return;
   }
-  CFX_ByteString facename = GetNameFromTT(names, 1);
-  CFX_ByteString style = GetNameFromTT(names, 2);
+  CFX_ByteString facename = GetNameFromTT(names, names.GetLength(), 1);
+  if (facename.IsEmpty()) {
+    return;
+  }
+  CFX_ByteString style = GetNameFromTT(names, names.GetLength(), 2);
   if (style != "Regular") {
     facename += " " + style;
   }
diff --git a/core/include/fxge/fx_font.h b/core/include/fxge/fx_font.h
index 964a3793ca..7bfe0c22f3 100644
--- a/core/include/fxge/fx_font.h
+++ b/core/include/fxge/fx_font.h
@@ -541,7 +541,9 @@ class IFX_GSUBTable {
   virtual ~IFX_GSUBTable() {}
 };
 
-CFX_ByteString GetNameFromTT(const uint8_t* name_table, FX_DWORD name);
+CFX_ByteString GetNameFromTT(const uint8_t* name_table,
+                             FX_DWORD name_table_size,
+                             FX_DWORD name);
 
 int PDF_GetStandardFontName(CFX_ByteString* name);
 
-- 
cgit v1.2.3