From 8eba66c1c6d19bcc779a3b4e7b68251511986ee8 Mon Sep 17 00:00:00 2001 From: Ebrahim Byagowi Date: Thu, 27 Feb 2020 15:58:58 +0330 Subject: [PATCH] [gvar] Fix invalid memory access by refactoring GlyphVarData fetch logic Fixes https://crbug.com/oss-fuzz/20906 --- src/hb-ot-var-gvar-table.hh | 70 +++++++++--------- ...-minimized-hb-draw-fuzzer-5088336521986048 | Bin 0 -> 1413 bytes 2 files changed, 36 insertions(+), 34 deletions(-) create mode 100644 test/fuzzing/fonts/clusterfuzz-testcase-minimized-hb-draw-fuzzer-5088336521986048 diff --git a/src/hb-ot-var-gvar-table.hh b/src/hb-ot-var-gvar-table.hh index b7d1b5e42..91c2891c3 100644 --- a/src/hb-ot-var-gvar-table.hh +++ b/src/hb-ot-var-gvar-table.hh @@ -416,19 +416,25 @@ struct gvar unsigned int num_glyphs = c->plan->num_output_glyphs (); out->glyphCount = num_glyphs; + hb_blob_ptr_t table = hb_sanitize_context_t ().reference_table (c->plan->source); + unsigned int subset_data_size = 0; for (hb_codepoint_t gid = 0; gid < num_glyphs; gid++) { hb_codepoint_t old_gid; if (!c->plan->old_gid_for_new_gid (gid, &old_gid)) continue; - subset_data_size += get_glyph_var_data_length (old_gid); + subset_data_size += get_glyph_var_data_bytes (table.get_blob (), old_gid).length; } bool long_offset = subset_data_size & ~0xFFFFu; out->flags = long_offset ? 1 : 0; HBUINT8 *subset_offsets = c->serializer->allocate_size ((long_offset ? 4 : 2) * (num_glyphs + 1)); - if (!subset_offsets) return_trace (false); + if (!subset_offsets) + { + table.destroy (); + return_trace (false); + } /* shared tuples */ if (!sharedTupleCount || !sharedTuples) @@ -437,49 +443,58 @@ struct gvar { unsigned int shared_tuple_size = F2DOT14::static_size * axisCount * sharedTupleCount; F2DOT14 *tuples = c->serializer->allocate_size (shared_tuple_size); - if (!tuples) return_trace (false); + if (!tuples) + { + table.destroy (); + return_trace (false); + } out->sharedTuples = (char *) tuples - (char *) out; memcpy (tuples, &(this+sharedTuples), shared_tuple_size); } char *subset_data = c->serializer->allocate_size (subset_data_size); - if (!subset_data) return_trace (false); + if (!subset_data) + { + table.destroy (); + return_trace (false); + } out->dataZ = subset_data - (char *)out; unsigned int glyph_offset = 0; for (hb_codepoint_t gid = 0; gid < num_glyphs; gid++) { hb_codepoint_t old_gid; - unsigned int length = c->plan->old_gid_for_new_gid (gid, &old_gid) ? get_glyph_var_data_length (old_gid) : 0; + hb_bytes_t var_data_bytes = c->plan->old_gid_for_new_gid (gid, &old_gid) + ? get_glyph_var_data_bytes (table.get_blob (), old_gid) + : hb_bytes_t (); if (long_offset) ((HBUINT32 *) subset_offsets)[gid] = glyph_offset; else ((HBUINT16 *) subset_offsets)[gid] = glyph_offset / 2; - if (length > 0) memcpy (subset_data, &get_glyph_var_data (old_gid), length); - subset_data += length; - glyph_offset += length; + if (var_data_bytes.length > 0) + memcpy (subset_data, var_data_bytes.arrayZ, var_data_bytes.length); + subset_data += var_data_bytes.length; + glyph_offset += var_data_bytes.length; } if (long_offset) ((HBUINT32 *) subset_offsets)[num_glyphs] = glyph_offset; else ((HBUINT16 *) subset_offsets)[num_glyphs] = glyph_offset / 2; + table.destroy (); return_trace (true); } protected: - const GlyphVarData &get_glyph_var_data (hb_codepoint_t glyph) const + const hb_bytes_t get_glyph_var_data_bytes (hb_blob_t *blob, hb_codepoint_t glyph) const { - unsigned int start_offset = get_offset (glyph); - unsigned int end_offset = get_offset (glyph+1); - - if ((start_offset == end_offset) || - unlikely ((start_offset > get_offset (glyphCount)) || - (start_offset + GlyphVarData::min_size > end_offset))) - return Null (GlyphVarData); - return (((unsigned char *) this + start_offset) + dataZ); + unsigned start_offset = get_offset (glyph); + unsigned length = get_offset (glyph+1) - start_offset; + return unlikely (GlyphVarData::min_size > length) + ? hb_bytes_t () + : blob->as_bytes ().sub_array (((unsigned) dataZ) + start_offset, length); } bool is_long_offset () const { return (flags & 1) != 0; } @@ -492,15 +507,6 @@ struct gvar return get_short_offset_array ()[i] * 2; } - unsigned int get_glyph_var_data_length (unsigned int glyph) const - { - unsigned int end_offset = get_offset (glyph + 1); - unsigned int start_offset = get_offset (glyph); - if (unlikely (start_offset > end_offset || end_offset > get_offset (glyphCount))) - return 0; - return end_offset - start_offset; - } - const HBUINT32 * get_long_offset_array () const { return (const HBUINT32 *) &offsetZ; } const HBUINT16 *get_short_offset_array () const { return (const HBUINT16 *) &offsetZ; } @@ -568,12 +574,12 @@ struct gvar coord_count = hb_min (coord_count, gvar_table->axisCount); if (!coord_count || coord_count != gvar_table->axisCount) return true; - const GlyphVarData &var_data = gvar_table->get_glyph_var_data (glyph); - if (!var_data.has_data ()) return true; + hb_bytes_t var_data_bytes = gvar_table->get_glyph_var_data_bytes (gvar_table.get_blob (), glyph); + const GlyphVarData *var_data = var_data_bytes.as (); + if (!var_data->has_data ()) return true; hb_vector_t shared_indices; GlyphVarData::tuple_iterator_t iterator; - if (!GlyphVarData::get_tuple_iterator (&var_data, - gvar_table->get_glyph_var_data_length (glyph), + if (!GlyphVarData::get_tuple_iterator (var_data, var_data_bytes.length, gvar_table->axisCount, shared_indices, &iterator)) @@ -699,10 +705,6 @@ no_more_gaps: unsigned int get_axis_count () const { return gvar_table->axisCount; } - protected: - const GlyphVarData &get_glyph_var_data (hb_codepoint_t glyph) const - { return gvar_table->get_glyph_var_data (glyph); } - private: hb_blob_ptr_t gvar_table; hb_vector_t shared_tuples; diff --git a/test/fuzzing/fonts/clusterfuzz-testcase-minimized-hb-draw-fuzzer-5088336521986048 b/test/fuzzing/fonts/clusterfuzz-testcase-minimized-hb-draw-fuzzer-5088336521986048 new file mode 100644 index 0000000000000000000000000000000000000000..5c23559c2c6227a019cc5ebd02cdc266afbe2790 GIT binary patch literal 1413 zcmZWpZD^ZS6n@UVX_97b`X*`GF=B1kPtf|&j<(jWyctC%ZtHYKL`H48w2h@%LmReM z1RGjVYK0*KLx0pC+mC`Mg(*rI(t(!!7*aoA`%%OnB11wI(K2L9eD0e#1TWls?sLw$ zPtJMXdjkPbg#~D&PQDy{>9M`mXXv@l**AFR$&Oc_P1}I<5RaT0iVju!zW;?dOMEMK zAvQB#m;3@yapIP9$yiE8)hEOcypzn#D6CBA3QPfX2^f4zU}V?e(`{KEv}*1OkE z0B6_@0D^|U9;SD|3x&vRdS=>yxx_@8Mx0oM zv(^-m4V#TPF;vz-%kQ{}GP;(P>E?x^m0@&99lZy%)-J7-i!sgla6|`=4H86gL+usZ z4Bn9dOZm_w$dBqWO+8PT3QT1{H7&b`x;T+YOF8g*IyNO<#w~h#Z~`Mp;~iw3Y>1C_ z!I^*48U{#(7=?&gU(u(;FPDl~{w(<*mg&V{MO8~@gX($_>-vYjYE4BAHrDB{#AkAy z>Xmr7BUx#hoo5SM#)tA|_jvI6+V`5?Zy9g@{K$11w&Qo0j9L+gqiB7^18|{@)=ev{ zi|Q2$#Eva&6{61X(18lGIJw4}?xUq?D!Eab%E`AF7wYSV&uIIvZca~-aw_TQCDKPr8vlhFwQbEEq!@UDukS1-xie}? z*`sfLc6C`mx3|O2K=$`D6&qKsX0unXY+%jfS20!|`t_asZ~WWHvcEoWs&v{g6o*TtFrvKhnF$njoeUSu OtueR9BKO{ZPsu+mz4!nC literal 0 HcmV?d00001