More work towards improving collect_glyphs() against bad input

The three "XXXXX"'s should be switched to false.  Doing that separately for ease
of bisecting...
This commit is contained in:
Behdad Esfahbod 2017-12-16 06:12:06 -08:00
parent 5d02572034
commit 81f27df4d9
5 changed files with 48 additions and 61 deletions

View File

@ -156,8 +156,7 @@ struct RangeRecord
template <typename set_t>
inline bool add_coverage (set_t *glyphs) const {
glyphs->add_range (start, end);
return likely (start <= end);
return glyphs->add_range (start, end);
}
GlyphID start; /* First GlyphID in the range */
@ -820,7 +819,7 @@ struct CoverageFormat2
unsigned int count = rangeRecord.len;
for (unsigned int i = 0; i < count; i++)
if (unlikely (!rangeRecord[i].add_coverage (glyphs)))
return false;
return true;//XXXXXXXXXXXXfalse;
return true;
}
@ -935,7 +934,7 @@ struct Coverage
switch (u.format) {
case 1: return u.format1.add_coverage (glyphs);
case 2: return u.format2.add_coverage (glyphs);
default:return false;
default:return true;//XXXXXXXXXXXfalse;
}
}

View File

@ -607,12 +607,7 @@ struct PairSet
unsigned int record_size = UINT16::static_size * (1 + len1 + len2);
const PairValueRecord *record = CastP<PairValueRecord> (arrayZ);
unsigned int count = len;
for (unsigned int i = 0; i < count; i++)
{
c->input->add (record->secondGlyph);
record = &StructAtOffset<PairValueRecord> (record, record_size);
}
c->input->add_array (&record->secondGlyph, len, record_size);
}
inline bool apply (hb_apply_context_t *c,
@ -689,7 +684,7 @@ struct PairPosFormat1
inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
{
TRACE_COLLECT_GLYPHS (this);
(this+coverage).add_coverage (c->input);
if (unlikely (!(this+coverage).add_coverage (c->input))) return;
unsigned int count = pairSet.len;
for (unsigned int i = 0; i < count; i++)
(this+pairSet[i]).collect_glyphs (c, valueFormat);
@ -755,7 +750,7 @@ struct PairPosFormat2
inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
{
TRACE_COLLECT_GLYPHS (this);
(this+coverage).add_coverage (c->input);
if (unlikely (!(this+coverage).add_coverage (c->input))) return;
unsigned int count1 = class1Count;
if (count1)
@ -904,7 +899,7 @@ struct CursivePosFormat1
inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
{
TRACE_COLLECT_GLYPHS (this);
(this+coverage).add_coverage (c->input);
if (unlikely (!(this+coverage).add_coverage (c->input))) return;
}
inline const Coverage &get_coverage (void) const
@ -1062,8 +1057,8 @@ struct MarkBasePosFormat1
inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
{
TRACE_COLLECT_GLYPHS (this);
(this+markCoverage).add_coverage (c->input);
(this+baseCoverage).add_coverage (c->input);
if (unlikely (!(this+markCoverage).add_coverage (c->input))) return;
if (unlikely (!(this+baseCoverage).add_coverage (c->input))) return;
}
inline const Coverage &get_coverage (void) const
@ -1165,8 +1160,8 @@ struct MarkLigPosFormat1
inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
{
TRACE_COLLECT_GLYPHS (this);
(this+markCoverage).add_coverage (c->input);
(this+ligatureCoverage).add_coverage (c->input);
if (unlikely (!(this+markCoverage).add_coverage (c->input))) return;
if (unlikely (!(this+ligatureCoverage).add_coverage (c->input))) return;
}
inline const Coverage &get_coverage (void) const
@ -1278,8 +1273,8 @@ struct MarkMarkPosFormat1
inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
{
TRACE_COLLECT_GLYPHS (this);
(this+mark1Coverage).add_coverage (c->input);
(this+mark2Coverage).add_coverage (c->input);
if (unlikely (!(this+mark1Coverage).add_coverage (c->input))) return;
if (unlikely (!(this+mark2Coverage).add_coverage (c->input))) return;
}
inline const Coverage &get_coverage (void) const

View File

@ -54,13 +54,13 @@ struct SingleSubstFormat1
inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
{
TRACE_COLLECT_GLYPHS (this);
if (unlikely (!(this+coverage).add_coverage (c->input))) return;
Coverage::Iter iter;
for (iter.init (this+coverage); iter.more (); iter.next ())
{
/* TODO Switch to range-based API to work around malicious fonts.
* https://github.com/harfbuzz/harfbuzz/issues/363 */
hb_codepoint_t glyph_id = iter.get_glyph ();
c->input->add (glyph_id);
c->output->add ((glyph_id + deltaGlyphID) & 0xFFFFu);
}
}
@ -139,13 +139,13 @@ struct SingleSubstFormat2
inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
{
TRACE_COLLECT_GLYPHS (this);
if (unlikely (!(this+coverage).add_coverage (c->input))) return;
Coverage::Iter iter;
unsigned int count = substitute.len;
for (iter.init (this+coverage); iter.more (); iter.next ())
{
if (unlikely (iter.get_coverage () >= count))
break; /* Work around malicious fonts. https://github.com/harfbuzz/harfbuzz/issues/363 */
c->input->add (iter.get_glyph ());
c->output->add (substitute[iter.get_coverage ()]);
}
}
@ -269,9 +269,7 @@ struct Sequence
inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
{
TRACE_COLLECT_GLYPHS (this);
unsigned int count = substitute.len;
for (unsigned int i = 0; i < count; i++)
c->output->add (substitute[i]);
c->output->add_array (substitute.array, substitute.len);
}
inline bool apply (hb_apply_context_t *c) const
@ -348,7 +346,7 @@ struct MultipleSubstFormat1
inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
{
TRACE_COLLECT_GLYPHS (this);
(this+coverage).add_coverage (c->input);
if (unlikely (!(this+coverage).add_coverage (c->input))) return;
unsigned int count = sequence.len;
for (unsigned int i = 0; i < count; i++)
(this+sequence[i]).collect_glyphs (c);
@ -474,17 +472,15 @@ struct AlternateSubstFormat1
inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
{
TRACE_COLLECT_GLYPHS (this);
if (unlikely (!(this+coverage).add_coverage (c->input))) return;
Coverage::Iter iter;
unsigned int count = alternateSet.len;
for (iter.init (this+coverage); iter.more (); iter.next ())
{
if (unlikely (iter.get_coverage () >= count))
break; /* Work around malicious fonts. https://github.com/harfbuzz/harfbuzz/issues/363 */
c->input->add (iter.get_glyph ());
const AlternateSet &alt_set = this+alternateSet[iter.get_coverage ()];
unsigned int count = alt_set.len;
for (unsigned int i = 0; i < count; i++)
c->output->add (alt_set[i]);
c->output->add_array (alt_set.array, alt_set.len);
}
}
@ -615,9 +611,7 @@ struct Ligature
inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
{
TRACE_COLLECT_GLYPHS (this);
unsigned int count = component.len;
for (unsigned int i = 1; i < count; i++)
c->input->add (component[i]);
c->input->add_array (component.array, component.len ? component.len - 1 : 0);
c->output->add (ligGlyph);
}
@ -801,13 +795,13 @@ struct LigatureSubstFormat1
inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
{
TRACE_COLLECT_GLYPHS (this);
if (unlikely (!(this+coverage).add_coverage (c->input))) return;
Coverage::Iter iter;
unsigned int count = ligatureSet.len;
for (iter.init (this+coverage); iter.more (); iter.next ())
{
if (unlikely (iter.get_coverage () >= count))
break; /* Work around malicious fonts. https://github.com/harfbuzz/harfbuzz/issues/363 */
c->input->add (iter.get_glyph ());
(this+ligatureSet[iter.get_coverage ()]).collect_glyphs (c);
}
}
@ -970,25 +964,22 @@ struct ReverseChainSingleSubstFormat1
inline void collect_glyphs (hb_collect_glyphs_context_t *c) const
{
TRACE_COLLECT_GLYPHS (this);
const OffsetArrayOf<Coverage> &lookahead = StructAfter<OffsetArrayOf<Coverage> > (backtrack);
if (unlikely (!(this+coverage).add_coverage (c->input))) return;
unsigned int count;
(this+coverage).add_coverage (c->input);
count = backtrack.len;
for (unsigned int i = 0; i < count; i++)
(this+backtrack[i]).add_coverage (c->before);
if (unlikely (!(this+backtrack[i]).add_coverage (c->before))) return;
const OffsetArrayOf<Coverage> &lookahead = StructAfter<OffsetArrayOf<Coverage> > (backtrack);
count = lookahead.len;
for (unsigned int i = 0; i < count; i++)
(this+lookahead[i]).add_coverage (c->after);
if (unlikely (!(this+lookahead[i]).add_coverage (c->after))) return;
const ArrayOf<GlyphID> &substitute = StructAfter<ArrayOf<GlyphID> > (lookahead);
count = substitute.len;
for (unsigned int i = 0; i < count; i++)
c->output->add (substitute[i]);
c->output->add_array (substitute.array, substitute.len);
}
inline const Coverage &get_coverage (void) const

View File

@ -71,7 +71,7 @@ struct hb_set_digest_lowest_bits_t
mask |= mask_for (g);
}
inline void add_range (hb_codepoint_t a, hb_codepoint_t b) {
inline bool add_range (hb_codepoint_t a, hb_codepoint_t b) {
if ((b >> shift) - (a >> shift) >= mask_bits - 1)
mask = (mask_t) -1;
else {
@ -79,6 +79,7 @@ struct hb_set_digest_lowest_bits_t
mask_t mb = mask_for (b);
mask |= mb + (mb - ma) - (mb < ma);
}
return true;
}
template <typename T>
@ -128,9 +129,10 @@ struct hb_set_digest_combiner_t
tail.add (g);
}
inline void add_range (hb_codepoint_t a, hb_codepoint_t b) {
inline bool add_range (hb_codepoint_t a, hb_codepoint_t b) {
head.add_range (a, b);
tail.add_range (a, b);
return true;
}
template <typename T>
inline void add_array (const T *array, unsigned int count, unsigned int stride=sizeof(T))

View File

@ -82,7 +82,6 @@ struct hb_set_t
memset (la, 0xff, (char *) lb - (char *) la);
*lb |= ((mask (b) << 1) - 1);
}
}
@ -230,34 +229,35 @@ struct hb_set_t
if (unlikely (!page)) return;
page->add (g);
}
inline void add_range (hb_codepoint_t a, hb_codepoint_t b)
inline bool add_range (hb_codepoint_t a, hb_codepoint_t b)
{
if (unlikely (in_error || a > b || a == INVALID || b == INVALID)) return;
if (unlikely (in_error || a > b || a == INVALID || b == INVALID)) return true;//XXXXXXXfalse;
unsigned int ma = get_major (a);
unsigned int mb = get_major (b);
if (ma == mb)
{
page_t *page = page_for_insert (a);
if (unlikely (!page)) return;
if (unlikely (!page)) return false;
page->add_range (a, b);
}
else
{
page_t *page = page_for_insert (a);
if (unlikely (!page)) return;
if (unlikely (!page)) return false;
page->add_range (a, major_start (ma + 1) - 1);
for (unsigned int m = ma + 1; m < mb; m++)
{
page = page_for_insert (major_start (m));
if (unlikely (!page)) return;
if (unlikely (!page)) return false;
page->init1 ();
}
page = page_for_insert (b);
if (unlikely (!page)) return;
if (unlikely (!page)) return false;
page->add_range (major_start (mb), b);
}
return true;
}
template <typename T>