commit 3d05b96181b259593047f592df4df33a3658e472
author Garret Rieger <grieger@google.com> Mon Mar 13 21:34:26 2023 +0000
committer Behdad Esfahbod <behdad@behdad.org> Mon Mar 13 16:03:58 2023 -0600
tree ea6471149662bac84c006750d4052771743e9436
parent 7a87b17742a0cec36ad21d9fddc1c605597eea14

[subset] track which glyphs have allocated memory so we can clean up correctly.

Fixes https://oss-fuzz.com/testcase-detail/5388270411579392

src/OT/glyf/CompositeGlyph.hh

diff --git a/src/OT/glyf/CompositeGlyph.hh b/src/OT/glyf/CompositeGlyph.hh
index edf8cd8..ad9ce82 100644
--- a/src/OT/glyf/CompositeGlyph.hh
+++ b/src/OT/glyf/CompositeGlyph.hh
@@ -330,7 +330,10 @@
     for (const auto &component : it)
     {
       /* last 4 points in deltas are phantom points and should not be included */
-      if (i >= deltas.length - 4) return false;
+      if (i >= deltas.length - 4) {
+        free (o);
+        return false;
+      }
 
       unsigned comp_len = component.get_size ();
       if (component.is_anchored ())

src/OT/glyf/SubsetGlyph.hh

diff --git a/src/OT/glyf/SubsetGlyph.hh b/src/OT/glyf/SubsetGlyph.hh
index 795925b..a38a57a 100644
--- a/src/OT/glyf/SubsetGlyph.hh
+++ b/src/OT/glyf/SubsetGlyph.hh
@@ -18,6 +18,7 @@
   Glyph source_glyph;
   hb_bytes_t dest_start;  /* region of source_glyph to copy first */
   hb_bytes_t dest_end;    /* region of source_glyph to copy second */
+  bool allocated;
 
   bool serialize (hb_serialize_context_t *c,
 		  bool use_short_loca,
@@ -60,12 +61,18 @@
   bool compile_bytes_with_deltas (const hb_subset_plan_t *plan,
                                   hb_font_t *font,
                                   const glyf_accelerator_t &glyf)
-  { return source_glyph.compile_bytes_with_deltas (plan, font, glyf, dest_start, dest_end); }
+  {
+    allocated = source_glyph.compile_bytes_with_deltas (plan, font, glyf, dest_start, dest_end);
+    return allocated;
+  }
 
   void free_compiled_bytes ()
   {
-    dest_start.fini ();
-    dest_end.fini ();
+    if (likely (allocated)) {
+      allocated = false;
+      dest_start.fini ();
+      dest_end.fini ();
+    }
   }
 
   void drop_hints_bytes ()

src/OT/glyf/glyf.hh

diff --git a/src/OT/glyf/glyf.hh b/src/OT/glyf/glyf.hh
index bc5608d..6e9f420 100644
--- a/src/OT/glyf/glyf.hh
+++ b/src/OT/glyf/glyf.hh
@@ -424,7 +424,6 @@
   unsigned num_glyphs = plan->num_output_glyphs ();
   if (!glyphs.resize (num_glyphs)) return false;
 
-  unsigned idx = 0;
   for (auto p : plan->glyph_map->iter ())
   {
     unsigned new_gid = p.second;
@@ -452,11 +451,10 @@
       if (unlikely (!subset_glyph.compile_bytes_with_deltas (plan, font, glyf)))
       {
         // when pinned at default, only bounds are updated, thus no need to free
-        if (!plan->pinned_at_default && idx > 0)
-          _free_compiled_subset_glyphs (glyphs, idx - 1);
+        if (!plan->pinned_at_default)
+          _free_compiled_subset_glyphs (glyphs, new_gid);
         return false;
       }
-      idx++;
     }
   }
   return true;

test/fuzzing/fonts/clusterfuzz-testcase-minimized-hb-subset-fuzzer-5388270411579392

diff --git a/test/fuzzing/fonts/clusterfuzz-testcase-minimized-hb-subset-fuzzer-5388270411579392 b/test/fuzzing/fonts/clusterfuzz-testcase-minimized-hb-subset-fuzzer-5388270411579392
new file mode 100644
index 0000000..d39badd
--- /dev/null
+++ b/test/fuzzing/fonts/clusterfuzz-testcase-minimized-hb-subset-fuzzer-5388270411579392
Binary files differ