From 0ab73e594275cf064e09b9df2e1df337a589745d Mon Sep 17 00:00:00 2001
From: Garret Rieger <grieger@google.com>
Date: Tue, 20 Feb 2018 15:33:03 -0800
Subject: [PATCH] [subset] Sanity check that loca writes are inbounds.

---
 src/hb-subset-glyf.cc | 55 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 41 insertions(+), 14 deletions(-)

diff --git a/src/hb-subset-glyf.cc b/src/hb-subset-glyf.cc
index 7dfe0ba4..c337e65b 100644
--- a/src/hb-subset-glyf.cc
+++ b/src/hb-subset-glyf.cc
@@ -62,15 +62,34 @@ _calculate_glyf_and_loca_prime_size (const OT::glyf::accelerator_t &glyf,
   return true;
 }
 
-static void
-_write_loca_entry (unsigned int id, unsigned int offset, bool is_short, void *loca_prime) {
-  if (is_short) {
-    ((OT::HBUINT16*) loca_prime) [id].set (offset / 2);
-  } else {
-    ((OT::HBUINT32*) loca_prime) [id].set (offset);
+static bool
+_write_loca_entry (unsigned int  id,
+                   unsigned int  offset,
+                   bool          is_short,
+                   void         *loca_prime,
+                   unsigned int  loca_size)
+{
+  unsigned int entry_size = is_short ? sizeof (OT::HBUINT16) : sizeof (OT::HBUINT32);
+  if ((id + 1) * entry_size <= loca_size)
+  {
+    if (is_short) {
+      ((OT::HBUINT16*) loca_prime) [id].set (offset / 2);
+    } else {
+      ((OT::HBUINT32*) loca_prime) [id].set (offset);
+    }
+    return true;
   }
+
+  // Offset was not written because the write is out of bounds.
+  DEBUG_MSG (SUBSET,
+             nullptr,
+             "WARNING: Attempted to write an out of bounds loca entry at index %d. Loca size is %d.",
+             id,
+             loca_size);
+  return false;
 }
 
+
 static void
 _update_components (hb_subset_plan_t * plan,
 		    char * glyph_start,
@@ -100,14 +119,16 @@ _write_glyf_and_loca_prime (hb_subset_plan_t              *plan,
 			    const OT::glyf::accelerator_t &glyf,
                             const char                    *glyf_data,
                             bool                           use_short_loca,
-                            int                            glyf_prime_size,
+                            unsigned int                   glyf_prime_size,
                             char                          *glyf_prime_data /* OUT */,
-                            int                            loca_prime_size,
+                            unsigned int                   loca_prime_size,
                             char                          *loca_prime_data /* OUT */)
 {
+  // TODO(grieger): Sanity check writes to make sure they are in-bounds.
   hb_prealloced_array_t<hb_codepoint_t> &glyph_ids = plan->gids_to_retain_sorted;
   char *glyf_prime_data_next = glyf_prime_data;
 
+  bool success = true;
   for (unsigned int i = 0; i < glyph_ids.len; i++)
   {
     unsigned int start_offset, end_offset;
@@ -117,15 +138,22 @@ _write_glyf_and_loca_prime (hb_subset_plan_t              *plan,
     int length = end_offset - start_offset;
     memcpy (glyf_prime_data_next, glyf_data + start_offset, length);
 
-    _write_loca_entry (i, glyf_prime_data_next - glyf_prime_data, use_short_loca, loca_prime_data);
+    success = success && _write_loca_entry (i,
+                                            glyf_prime_data_next - glyf_prime_data,
+                                            use_short_loca,
+                                            loca_prime_data,
+                                            loca_prime_size);
     _update_components (plan, glyf_prime_data_next, end_offset - start_offset);
 
     glyf_prime_data_next += length;
   }
 
-  _write_loca_entry (glyph_ids.len, glyf_prime_data_next - glyf_prime_data, use_short_loca, loca_prime_data);
-
-  return true;
+  success = success && _write_loca_entry (glyph_ids.len,
+                                          glyf_prime_data_next - glyf_prime_data,
+                                          use_short_loca,
+                                          loca_prime_data,
+                                          loca_prime_size);
+  return success;
 }
 
 static bool
@@ -136,9 +164,7 @@ _hb_subset_glyf_and_loca (const OT::glyf::accelerator_t  &glyf,
                           hb_blob_t                     **glyf_prime /* OUT */,
                           hb_blob_t                     **loca_prime /* OUT */)
 {
-  // TODO(grieger): Sanity check writes to make sure they are in-bounds.
   // TODO(grieger): Sanity check allocation size for the new table.
-  // TODO(grieger): Don't fail on bad offsets, just dump them.
   hb_prealloced_array_t<hb_codepoint_t> &glyphs_to_retain = plan->gids_to_retain_sorted;
 
   unsigned int glyf_prime_size;
@@ -159,6 +185,7 @@ _hb_subset_glyf_and_loca (const OT::glyf::accelerator_t  &glyf,
                                              glyf_prime_size, glyf_prime_data,
                                              loca_prime_size, loca_prime_data))) {
     free (glyf_prime_data);
+    free (loca_prime_data);
     return false;
   }
 
-- 
GitLab