[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.

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()));
   }
 

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,

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;
 }

test/api/test-subset-hdmx.c

@@ -72,6 +72,28 @@ test_subset_hdmx_invalid (void)
   hb_face_destroy (face);
 }
 
+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)
 {
@@ -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();