From 10642b3fbfbc1776e784b190c43a9e0693dd423a Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Sat, 15 Sep 2018 19:43:33 +0200 Subject: [PATCH] Disallow null-enabled offsets to unsized structures... ...like UnsizedArrayOf<>. This fixes a class of crasher bugs, mostly with color and AAT tables. We cannot use nullable offsets to varsized data that does not declare min_size, because it's nost safe to use our fixed-size null pool for types that have their size external. So, use non_null'able offsets for these. A further enhancement would be to make use of min_size in Null<> itself. Will try that after. --- src/hb-aat-layout-common.hh | 8 ++++---- src/hb-aat-layout-feat-table.hh | 2 +- src/hb-aat-layout-morx-table.hh | 10 +++++----- src/hb-aat-layout-trak-table.hh | 4 ++-- src/hb-aat-ltag-table.hh | 2 +- src/hb-open-type.hh | 5 +++++ src/hb-ot-color-cbdt-table.hh | 4 +--- src/hb-ot-color-colr-table.hh | 4 ++-- src/hb-ot-color-cpal-table.hh | 8 ++++---- src/hb-ot-color-svg-table.hh | 2 +- src/hb-ot-layout-common.hh | 18 ++++++++++-------- src/hb-ot-layout-jstf-table.hh | 4 ++-- 12 files changed, 38 insertions(+), 33 deletions(-) diff --git a/src/hb-aat-layout-common.hh b/src/hb-aat-layout-common.hh index af71ebd38..57374b0bc 100644 --- a/src/hb-aat-layout-common.hh +++ b/src/hb-aat-layout-common.hh @@ -243,7 +243,7 @@ struct LookupSegmentArray GlyphID last; /* Last GlyphID in this segment */ GlyphID first; /* First GlyphID in this segment */ - OffsetTo > + OffsetTo, HBUINT16, false> valuesZ; /* A 16-bit offset from the start of * the table to the data. */ public: @@ -522,11 +522,11 @@ struct StateTable protected: HBUINT32 nClasses; /* Number of classes, which is the number of indices * in a single line in the state array. */ - LOffsetTo > + LOffsetTo, false> classTable; /* Offset to the class table. */ - LOffsetTo > + LOffsetTo, false> stateArrayTable;/* Offset to the state array. */ - LOffsetTo > > + LOffsetTo >, false> entryTable; /* Offset to the entry array. */ public: diff --git a/src/hb-aat-layout-feat-table.hh b/src/hb-aat-layout-feat-table.hh index b70076316..b670caab8 100644 --- a/src/hb-aat-layout-feat-table.hh +++ b/src/hb-aat-layout-feat-table.hh @@ -78,7 +78,7 @@ struct FeatureName protected: HBUINT16 feature; /* Feature type. */ HBUINT16 nSettings; /* The number of records in the setting name array. */ - LOffsetTo > + LOffsetTo, false> settingTable; /* Offset in bytes from the beginning of this table to * this feature's setting name array. The actual type of * record this offset refers to will depend on the diff --git a/src/hb-aat-layout-morx-table.hh b/src/hb-aat-layout-morx-table.hh index c544cb376..3bad5517f 100644 --- a/src/hb-aat-layout-morx-table.hh +++ b/src/hb-aat-layout-morx-table.hh @@ -311,7 +311,7 @@ struct ContextualSubtable protected: StateTable machine; - LOffsetTo, HBUINT32> > + LOffsetTo, HBUINT32>, false> substitutionTables; public: DEFINE_SIZE_STATIC (20); @@ -473,11 +473,11 @@ struct LigatureSubtable protected: StateTable machine; - LOffsetTo > + LOffsetTo, false> ligAction; /* Offset to the ligature action table. */ - LOffsetTo > + LOffsetTo, false> component; /* Offset to the component table. */ - LOffsetTo > + LOffsetTo, false> ligature; /* Offset to the actual ligature lists. */ public: DEFINE_SIZE_STATIC (28); @@ -715,7 +715,7 @@ struct InsertionSubtable protected: StateTable machine; - LOffsetTo > + LOffsetTo, false> insertionAction; /* Byte offset from stateHeader to the start of * the insertion glyph table. */ public: diff --git a/src/hb-aat-layout-trak-table.hh b/src/hb-aat-layout-trak-table.hh index c4bec2cab..3b7d43881 100644 --- a/src/hb-aat-layout-trak-table.hh +++ b/src/hb-aat-layout-trak-table.hh @@ -68,7 +68,7 @@ struct TrackTableEntry protected: Fixed track; /* Track value for this record. */ NameID trackNameID; /* The 'name' table index for this track */ - OffsetTo > + OffsetTo, HBUINT16, false> valuesZ; /* Offset from start of tracking table to * per-size tracking values for this track. */ @@ -134,7 +134,7 @@ struct TrackData protected: HBUINT16 nTracks; /* Number of separate tracks included in this table. */ HBUINT16 nSizes; /* Number of point sizes included in this table. */ - LOffsetTo > + LOffsetTo, false> sizeTable; /* Offset to array[nSizes] of size values. */ UnsizedArrayOf trackTable; /* Array[nTracks] of TrackTableEntry records. */ diff --git a/src/hb-aat-ltag-table.hh b/src/hb-aat-ltag-table.hh index e308ab28c..08a1b51a9 100644 --- a/src/hb-aat-ltag-table.hh +++ b/src/hb-aat-ltag-table.hh @@ -46,7 +46,7 @@ struct FTStringRange } protected: - OffsetTo > + OffsetTo, HBUINT16, false> tag; /* Offset from the start of the table to * the beginning of the string */ HBUINT16 length; /* String length (in bytes) */ diff --git a/src/hb-open-type.hh b/src/hb-open-type.hh index 2e1e240c6..beded06ce 100644 --- a/src/hb-open-type.hh +++ b/src/hb-open-type.hh @@ -226,9 +226,14 @@ struct FixedVersion * Use: (base+offset) */ +template struct assert_has_min_size { static_assert (Type::min_size > 0); }; +template struct assert_has_min_size {}; + template struct OffsetTo : Offset { + static_assert (sizeof (assert_has_min_size) || true); + inline const Type& operator () (const void *base) const { if (unlikely (this->is_null ())) return Null(Type); diff --git a/src/hb-ot-color-cbdt-table.hh b/src/hb-ot-color-cbdt-table.hh index 7c172ce43..561c59900 100644 --- a/src/hb-ot-color-cbdt-table.hh +++ b/src/hb-ot-color-cbdt-table.hh @@ -264,8 +264,6 @@ struct IndexSubtableArray protected: UnsizedArrayOf indexSubtablesZ; - public: - DEFINE_SIZE_ARRAY(0, indexSubtablesZ); }; struct BitmapSizeTable @@ -289,7 +287,7 @@ struct BitmapSizeTable } protected: - LOffsetTo + LOffsetTo indexSubtableArrayOffset; HBUINT32 indexTablesSize; HBUINT32 numberOfIndexSubtables; diff --git a/src/hb-ot-color-colr-table.hh b/src/hb-ot-color-colr-table.hh index 070226f18..a59d2bfa9 100644 --- a/src/hb-ot-color-colr-table.hh +++ b/src/hb-ot-color-colr-table.hh @@ -129,9 +129,9 @@ struct COLR protected: HBUINT16 version; /* Table version number */ HBUINT16 numBaseGlyphs; /* Number of Base Glyph Records */ - LOffsetTo > + LOffsetTo, false> baseGlyphsZ; /* Offset to Base Glyph records. */ - LOffsetTo > + LOffsetTo, false> layersZ; /* Offset to Layer Records */ HBUINT16 numLayers; /* Number of Layer Records */ public: diff --git a/src/hb-ot-color-cpal-table.hh b/src/hb-ot-color-cpal-table.hh index 2c9fced09..e354ced5c 100644 --- a/src/hb-ot-color-cpal-table.hh +++ b/src/hb-ot-color-cpal-table.hh @@ -118,15 +118,15 @@ struct CPALV1Tail } protected: - LOffsetTo > + LOffsetTo, false> paletteFlagsZ; /* Offset from the beginning of CPAL table to * the Palette Type Array. Set to 0 if no array * is provided. */ - LOffsetTo > + LOffsetTo, false> paletteLabelZ; /* Offset from the beginning of CPAL table to * the Palette Labels Array. Set to 0 if no * array is provided. */ - LOffsetTo > + LOffsetTo, false> paletteEntryLabelZ; /* Offset from the beginning of CPAL table to * the Palette Entry Label Array. Set to 0 * if no array is provided. */ @@ -207,7 +207,7 @@ struct CPAL HBUINT16 numPalettes; /* Number of palettes in the table. */ HBUINT16 numColorRecords; /* Total number of color records, combined for * all palettes. */ - LOffsetTo > + LOffsetTo, false> colorRecordsZ; /* Offset from the beginning of CPAL table to * the first ColorRecord. */ UnsizedArrayOf diff --git a/src/hb-ot-color-svg-table.hh b/src/hb-ot-color-svg-table.hh index ad9162f7c..53d466846 100644 --- a/src/hb-ot-color-svg-table.hh +++ b/src/hb-ot-color-svg-table.hh @@ -54,7 +54,7 @@ struct SVGDocumentIndexEntry * this index entry. */ HBUINT16 endGlyphID; /* The last glyph ID in the range described by * this index entry. Must be >= startGlyphID. */ - LOffsetTo > + LOffsetTo, false> svgDoc; /* Offset from the beginning of the SVG Document Index * to an SVG document. Must be non-zero. */ HBUINT32 svgDocLength; /* Length of the SVG document. diff --git a/src/hb-ot-layout-common.hh b/src/hb-ot-layout-common.hh index 8a3a703b3..e5e996d4d 100644 --- a/src/hb-ot-layout-common.hh +++ b/src/hb-ot-layout-common.hh @@ -70,6 +70,11 @@ namespace OT { * Script, ScriptList, LangSys, Feature, FeatureList, Lookup, LookupList */ +struct Record_sanitize_closure_t { + hb_tag_t tag; + const void *list_base; +}; + template struct Record { @@ -77,14 +82,10 @@ struct Record return tag.cmp (a); } - struct sanitize_closure_t { - hb_tag_t tag; - const void *list_base; - }; inline bool sanitize (hb_sanitize_context_t *c, const void *base) const { TRACE_SANITIZE (this); - const sanitize_closure_t closure = {tag, base}; + const Record_sanitize_closure_t closure = {tag, base}; return_trace (c->check_struct (this) && offset.sanitize (c, base, &closure)); } @@ -240,7 +241,7 @@ struct LangSys } inline bool sanitize (hb_sanitize_context_t *c, - const Record::sanitize_closure_t * = nullptr) const + const Record_sanitize_closure_t * = nullptr) const { TRACE_SANITIZE (this); return_trace (c->check_struct (this) && featureIndex.sanitize (c)); @@ -291,7 +292,7 @@ struct Script } inline bool sanitize (hb_sanitize_context_t *c, - const Record