[subset] Couple of fixes for fuzzer discovered issues. (#924)

* [subset] sanitize individual DeviceRecord's as part of hdmx sanitization.

* [subset] Fix out of bounds read with non-two byte align glyphs.

* [subset] Just use size_device_record >= DeviceRecord::min_size.

* [subset] Add TODO.

* [subset] Re-order checks in hdmx sanitize.

diff --git a/src/hb-ot-hdmx-table.hh b/src/hb-ot-hdmx-table.hh
index 6ad57af..c0b22b2 100644 (file)
--- a/src/hb-ot-hdmx-table.hh
+++ b/src/hb-ot-hdmx-table.hh
@@ -198,6 +198,7 @@ struct hdmx
     TRACE_SANITIZE (this);
     return_trace (c->check_struct (this) && version == 0 &&
                  !_hb_unsigned_int_mul_overflows (num_records, size_device_record) &&
+                 size_device_record >= DeviceRecord::min_size &&
                  c->check_range (this, get_size()));
   }
 

diff --git a/src/hb-subset-glyf.cc b/src/hb-subset-glyf.cc
index 1bbcbdc..b9e6355 100644 (file)
--- a/src/hb-subset-glyf.cc
+++ b/src/hb-subset-glyf.cc
@@ -121,7 +121,6 @@ static void
 _update_components (hb_subset_plan_t * plan,
                    char * glyph_start,
                    unsigned int length)
-
 {
   OT::glyf::CompositeGlyphHeader::Iterator iterator;
   if (OT::glyf::CompositeGlyphHeader::get_iterator (glyph_start,
@@ -176,11 +175,11 @@ _write_glyf_and_loca_prime (hb_subset_plan_t              *plan,
     if (unlikely (!(glyf.get_offsets (glyph_ids[i], &start_offset, &end_offset)
                     && glyf.remove_padding(start_offset, &end_offset))))
       end_offset = start_offset = 0;
+
     unsigned int instruction_start = instruction_ranges[i * 2];
     unsigned int instruction_end = instruction_ranges[i * 2 + 1];
 
     int length = end_offset - start_offset - (instruction_end - instruction_start);
-    length += length % 2;
 
     if (glyf_prime_data_next + length > glyf_prime_data + glyf_prime_size)
     {
@@ -214,7 +213,8 @@ _write_glyf_and_loca_prime (hb_subset_plan_t              *plan,
                                             loca_prime_size);
     _update_components (plan, glyf_prime_data_next, length);
 
-    glyf_prime_data_next += length;
+    // TODO: don't align to two bytes if using long loca.
+    glyf_prime_data_next += length + (length % 2); // Align to 2 bytes for short loca.
   }
 
   success = success && _write_loca_entry (glyph_ids.len,

diff --git a/src/hb-subset.cc b/src/hb-subset.cc
index 4062c9b..2a2f855 100644 (file)
--- a/src/hb-subset.cc
+++ b/src/hb-subset.cc
@@ -89,12 +89,16 @@ _subset (hb_subset_plan_t *plan)
   hb_blob_t *source_blob = sanitizer.sanitize (plan->source->reference_table (TableType::tableTag));
   const TableType *table = OT::Sanitizer<TableType>::lock_instance (source_blob);
 
+  hb_tag_t tag = TableType::tableTag;
   hb_bool_t result = false;
   if (table != &OT::Null(TableType))
+  {
     result = table->subset(plan);
+  } else {
+    DEBUG_MSG(SUBSET, nullptr, "OT::%c%c%c%c::subset sanitize failed on source table.", HB_UNTAG(tag));
+  }
 
   hb_blob_destroy (source_blob);
-  hb_tag_t tag = TableType::tableTag;
   DEBUG_MSG(SUBSET, nullptr, "OT::%c%c%c%c::subset %s", HB_UNTAG(tag), result ? "success" : "FAILED!");
   return result;
 }

diff --git a/test/api/fonts/clusterfuzz-testcase-minimized-hb-subset-fuzzer-5609911946838016 b/test/api/fonts/clusterfuzz-testcase-minimized-hb-subset-fuzzer-5609911946838016
new file mode 100644 (file)
index 0000000..8c647a8
Binary files /dev/null and b/test/api/fonts/clusterfuzz-testcase-minimized-hb-subset-fuzzer-5609911946838016 differ

diff --git a/test/api/fonts/clusterfuzz-testcase-minimized-hb-subset-fuzzer-6651660668502016 b/test/api/fonts/clusterfuzz-testcase-minimized-hb-subset-fuzzer-6651660668502016
new file mode 100644 (file)
index 0000000..6206f07
Binary files /dev/null and b/test/api/fonts/clusterfuzz-testcase-minimized-hb-subset-fuzzer-6651660668502016 differ

diff --git a/test/api/test-subset-hdmx.c b/test/api/test-subset-hdmx.c
index dd20b2a..c78009b 100644 (file)
--- a/test/api/test-subset-hdmx.c
+++ b/test/api/test-subset-hdmx.c
@@ -73,6 +73,28 @@ test_subset_hdmx_invalid (void)
 }
 
 static void
+test_subset_hdmx_fails_sanitize (void)
+{
+  hb_face_t *face = hb_subset_test_open_font("fonts/clusterfuzz-testcase-minimized-hb-subset-fuzzer-5609911946838016");
+
+  hb_subset_input_t *input = hb_subset_input_create_or_fail ();
+  hb_set_t *codepoints = hb_subset_input_unicode_set (input);
+  hb_set_add (codepoints, 'a');
+  hb_set_add (codepoints, 'b');
+  hb_set_add (codepoints, 'c');
+
+  hb_subset_profile_t *profile = hb_subset_profile_create();
+  hb_face_t *subset = hb_subset (face, profile, input);
+  g_assert (subset);
+  g_assert (subset == hb_face_get_empty ());
+
+  hb_subset_input_destroy (input);
+  hb_subset_profile_destroy (profile);
+  hb_face_destroy (subset);
+  hb_face_destroy (face);
+}
+
+static void
 test_subset_hdmx_noop (void)
 {
   hb_face_t *face_abc = hb_subset_test_open_font("fonts/Roboto-Regular.abc.ttf");
@@ -98,6 +120,7 @@ main (int argc, char **argv)
 
   hb_test_add (test_subset_hdmx_simple_subset);
   hb_test_add (test_subset_hdmx_invalid);
+  hb_test_add (test_subset_hdmx_fails_sanitize);
   hb_test_add (test_subset_hdmx_noop);
 
   return hb_test_run();