From 6b5e4d07adb6b739dc294da513c4a7acd03977f7 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 11 Sep 2018 17:26:24 +0200 Subject: [PATCH 01/20] [dfont] Minor --- src/hb-open-file.hh | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/hb-open-file.hh b/src/hb-open-file.hh index 39c6aeb89..35a774ddd 100644 --- a/src/hb-open-file.hh +++ b/src/hb-open-file.hh @@ -363,10 +363,7 @@ struct ResourceMap return ((&reserved[2])+typeList)[i]; } - inline unsigned int get_types_count () const - { - return nTypes + 1; - } + inline unsigned int get_types_count () const { return nTypes + 1; } inline const ResourceRefItem &get_ref_item (const ResourceTypeItem &type, unsigned int i) const @@ -392,7 +389,7 @@ struct ResourceMap OffsetTo > typeList; /* Offset from beginning of map to * resource type list */ - HBUINT16 nameList; /* Offset from beginning of map to + Offset16 nameList; /* Offset from beginning of map to * resource name list */ HBUINT16 nTypes; /* Number of types in the map minus 1 */ public: From 4134ec1307bbaff24972e238bc5e4a403cd3f1c1 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 11 Sep 2018 17:56:03 +0200 Subject: [PATCH 02/20] [dfont] Sanitize only sfnt resources as OpenTypeFontFile --- src/hb-open-file.hh | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/hb-open-file.hh b/src/hb-open-file.hh index 35a774ddd..bbfc8f0e0 100644 --- a/src/hb-open-file.hh +++ b/src/hb-open-file.hh @@ -300,7 +300,7 @@ struct ResourceRefItem HBINT16 id; /* Resource ID, is really should be signed? */ HBINT16 nameOffset; /* Offset from beginning of resource name list - * to resource name, minus means there is none. */ + * to resource name, -1 means there is none. */ HBUINT8 attr; /* Resource attributes */ HBUINT24 dataOffset; /* Offset from beginning of resource data to * data for this resource */ @@ -374,7 +374,7 @@ struct ResourceMap inline const PString& get_name (const ResourceRefItem &item, unsigned int i) const { - if (item.nameOffset == -1) + if (item.nameOffset < 0) return Null (PString); return StructAtOffset (this, nameList + item.nameOffset); @@ -452,8 +452,11 @@ struct ResourceForkHeader for (unsigned int j = 0; j < type.get_resource_count (); ++j) { const LArrayOf& data = get_data (type, j); - if (unlikely (!(data.sanitize (c) && - ((OpenTypeFontFace&) data.arrayZ).sanitize (c)))) + if (unlikely (!data.sanitize (c))) + return_trace (false); + + if (unlikely (type.is_sfnt () && + !((OpenTypeFontFace&) data.arrayZ).sanitize (c))) return_trace (false); } } From bd75fd45cdbd0edb24568326bb7fde59d299a82c Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 11 Sep 2018 18:12:26 +0200 Subject: [PATCH 03/20] [dfont] Some renaming, plus add link to reference doc --- src/hb-open-file.hh | 79 ++++++++++++++++++++------------------------- 1 file changed, 35 insertions(+), 44 deletions(-) diff --git a/src/hb-open-file.hh b/src/hb-open-file.hh index bbfc8f0e0..26e68bb54 100644 --- a/src/hb-open-file.hh +++ b/src/hb-open-file.hh @@ -287,9 +287,11 @@ struct TTCHeader /* * Mac Resource Fork + * + * http://mirror.informatimago.com/next/developer.apple.com/documentation/mac/MoreToolbox/MoreToolbox-99.html */ -struct ResourceRefItem +struct ResourceRecord { inline bool sanitize (hb_sanitize_context_t *c) const { @@ -298,10 +300,10 @@ struct ResourceRefItem return_trace (likely (c->check_struct (this))); } - HBINT16 id; /* Resource ID, is really should be signed? */ + HBUINT16 id; /* Resource ID, is really should be signed? */ HBINT16 nameOffset; /* Offset from beginning of resource name list * to resource name, -1 means there is none. */ - HBUINT8 attr; /* Resource attributes */ + HBUINT8 attrs; /* Resource attributes */ HBUINT24 dataOffset; /* Offset from beginning of resource data to * data for this resource */ HBUINT32 reserved; /* Reserved for handle to resource */ @@ -309,7 +311,7 @@ struct ResourceRefItem DEFINE_SIZE_STATIC (12); }; -struct ResourceTypeItem +struct ResourceTypeRecord { inline bool sanitize (hb_sanitize_context_t *c) const { @@ -318,20 +320,20 @@ struct ResourceTypeItem return_trace (likely (c->check_struct (this))); } - inline unsigned int get_resource_count () const { return numRes + 1; } + inline unsigned int get_resource_count () const { return resCountM1 + 1; } - inline bool is_sfnt () const { return type == HB_TAG ('s','f','n','t'); } + inline bool is_sfnt () const { return tag == HB_TAG ('s','f','n','t'); } - inline const ResourceRefItem& get_ref_item (const void *base, - unsigned int i) const + inline const ResourceRecord& get_resource_record (const void *base, + unsigned int i) const { return (base+refList)[i]; } protected: - Tag type; /* Resource type. */ - HBUINT16 numRes; /* Number of resources minus 1. */ - OffsetTo > + Tag tag; /* Resource type. */ + HBUINT16 resCountM1; /* Number of resources minus 1. */ + OffsetTo > refList; /* Offset from beginning of resource type list * to reference item list for this type. */ public: @@ -347,51 +349,41 @@ struct ResourceMap return_trace (false); for (unsigned int i = 0; i < get_types_count (); ++i) { - const ResourceTypeItem& type = get_type (i); + const ResourceTypeRecord& type = get_type_record (i); if (unlikely (!type.sanitize (c))) return_trace (false); for (unsigned int j = 0; j < type.get_resource_count (); ++j) - if (unlikely (!get_ref_item (type, j).sanitize (c))) + if (unlikely (!get_resource_record (type, j).sanitize (c))) return_trace (false); } return_trace (true); } - inline const ResourceTypeItem& get_type (unsigned int i) const + inline const ResourceTypeRecord& get_type_record (unsigned int i) const { - // Why offset from the second byte of the object? I'm not sure - return ((&reserved[2])+typeList)[i]; + // Why offset from the third byte of the object? I'm not sure + return (((const char *) this + 2)+typeListZ)[i]; } - inline unsigned int get_types_count () const { return nTypes + 1; } + inline unsigned int get_types_count () const { return typeCountM1 + 1; } - inline const ResourceRefItem &get_ref_item (const ResourceTypeItem &type, - unsigned int i) const + inline const ResourceRecord &get_resource_record (const ResourceTypeRecord &type, + unsigned int i) const { - return type.get_ref_item (&(this+typeList), i); - } - - inline const PString& get_name (const ResourceRefItem &item, - unsigned int i) const - { - if (item.nameOffset < 0) - return Null (PString); - - return StructAtOffset (this, nameList + item.nameOffset); + return type.get_resource_record (&(this+typeListZ), i); } protected: - HBUINT8 reserved[16]; /* Reserved for copy of resource header */ - LOffsetTo - reserved1; /* Reserved for handle to next resource map */ - HBUINT16 reserved2; /* Reserved for file reference number */ - HBUINT16 attr; /* Resource fork attribute */ - OffsetTo > - typeList; /* Offset from beginning of map to + HBUINT8 reserved0[16]; /* Reserved for copy of resource header */ + HBUINT32 reserved1; /* Reserved for handle to next resource map */ + HBUINT16 resreved2; /* Reserved for file reference number */ + HBUINT16 attrs; /* Resource fork attribute */ + OffsetTo > + typeListZ; /* Offset from beginning of map to * resource type list */ Offset16 nameList; /* Offset from beginning of map to * resource name list */ - HBUINT16 nTypes; /* Number of types in the map minus 1 */ + HBUINT16 typeCountM1; /* Number of types in the map minus 1 */ public: DEFINE_SIZE_STATIC (30); }; @@ -403,19 +395,18 @@ struct ResourceForkHeader const ResourceMap &resource_map = this+map; for (unsigned int i = 0; i < resource_map.get_types_count (); ++i) { - const ResourceTypeItem& type = resource_map.get_type (i); + const ResourceTypeRecord& type = resource_map.get_type_record (i); if (type.is_sfnt ()) return type.get_resource_count (); } return 0; } - inline const LArrayOf& get_data (const ResourceTypeItem& type, + inline const LArrayOf& get_data (const ResourceTypeRecord& type, unsigned int idx) const { const ResourceMap &resource_map = this+map; - unsigned int offset = dataOffset; - offset += resource_map.get_ref_item (type, idx).dataOffset; + unsigned int offset = dataOffset + resource_map.get_resource_record (type, idx).dataOffset; return StructAtOffset > (this, offset); } @@ -424,7 +415,7 @@ struct ResourceForkHeader const ResourceMap &resource_map = this+map; for (unsigned int i = 0; i < resource_map.get_types_count (); ++i) { - const ResourceTypeItem& type = resource_map.get_type (i); + const ResourceTypeRecord& type = resource_map.get_type_record (i); if (type.is_sfnt () && idx < type.get_resource_count ()) { const OpenTypeFontFace &face = (OpenTypeFontFace&) get_data (type, idx).arrayZ; @@ -448,7 +439,7 @@ struct ResourceForkHeader for (unsigned int i = 0; i < resource_map.get_types_count (); ++i) { - const ResourceTypeItem& type = resource_map.get_type (i); + const ResourceTypeRecord& type = resource_map.get_type_record (i); for (unsigned int j = 0; j < type.get_resource_count (); ++j) { const LArrayOf& data = get_data (type, j); @@ -465,7 +456,7 @@ struct ResourceForkHeader } protected: - HBUINT32 dataOffset; /* Offset from beginning of resource fork + Offset32 dataOffset; /* Offset from beginning of resource fork * to resource data */ LOffsetTo map; /* Offset from beginning of resource fork From b482e5231cd5987082dd2c05fd649c3653f3c67a Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 13 Sep 2018 16:29:49 +0200 Subject: [PATCH 04/20] OffsetTo::sanitize() reshuffling --- src/hb-open-type.hh | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/hb-open-type.hh b/src/hb-open-type.hh index 5d09c5208..2c71f8194 100644 --- a/src/hb-open-type.hh +++ b/src/hb-open-type.hh @@ -260,27 +260,39 @@ struct OffsetTo : Offset this->set (0); } - inline bool sanitize (hb_sanitize_context_t *c, const void *base) const + inline bool sanitize_shallow (hb_sanitize_context_t *c, const void *base) const { TRACE_SANITIZE (this); if (unlikely (!c->check_struct (this))) return_trace (false); unsigned int offset = *this; if (unlikely (!offset)) return_trace (true); if (unlikely (!c->check_range (base, offset))) return_trace (false); - const Type &obj = StructAtOffset (base, offset); + return_trace (true); + } + + inline bool sanitize (hb_sanitize_context_t *c, const void *base) const + { + TRACE_SANITIZE (this); + if (unlikely (!sanitize_shallow (c, base))) return_trace (false); + const Type &obj = StructAtOffset (base, *this); return_trace (likely (obj.sanitize (c)) || neuter (c)); } template inline bool sanitize (hb_sanitize_context_t *c, const void *base, T user_data) const { TRACE_SANITIZE (this); - if (unlikely (!c->check_struct (this))) return_trace (false); - unsigned int offset = *this; - if (unlikely (!offset)) return_trace (true); - if (unlikely (!c->check_range (base, offset))) return_trace (false); - const Type &obj = StructAtOffset (base, offset); + if (unlikely (!sanitize_shallow (c, base))) return_trace (false); + const Type &obj = StructAtOffset (base, *this); return_trace (likely (obj.sanitize (c, user_data)) || neuter (c)); } + template + inline bool sanitize (hb_sanitize_context_t *c, const void *base, T1 user_data1, T2 user_data2) const + { + TRACE_SANITIZE (this); + if (unlikely (!sanitize_shallow (c, base))) return_trace (false); + const Type &obj = StructAtOffset (base, *this); + return_trace (likely (obj.sanitize (c, user_data1, user_data2)) || neuter (c)); + } /* Set the offset to Null */ inline bool neuter (hb_sanitize_context_t *c) const { From a73bea69c599787b4cfeac92a3afd00749e00434 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 13 Sep 2018 16:31:31 +0200 Subject: [PATCH 05/20] OffsetTo::sanitize() more shuffling --- src/hb-open-type.hh | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/hb-open-type.hh b/src/hb-open-type.hh index 2c71f8194..76ce55a76 100644 --- a/src/hb-open-type.hh +++ b/src/hb-open-type.hh @@ -273,25 +273,25 @@ struct OffsetTo : Offset inline bool sanitize (hb_sanitize_context_t *c, const void *base) const { TRACE_SANITIZE (this); - if (unlikely (!sanitize_shallow (c, base))) return_trace (false); - const Type &obj = StructAtOffset (base, *this); - return_trace (likely (obj.sanitize (c)) || neuter (c)); + return_trace (sanitize_shallow (c, base) && + (StructAtOffset (base, *this).sanitize (c) || + neuter (c))); } template inline bool sanitize (hb_sanitize_context_t *c, const void *base, T user_data) const { TRACE_SANITIZE (this); - if (unlikely (!sanitize_shallow (c, base))) return_trace (false); - const Type &obj = StructAtOffset (base, *this); - return_trace (likely (obj.sanitize (c, user_data)) || neuter (c)); + return_trace (sanitize_shallow (c, base) && + (StructAtOffset (base, *this).sanitize (c, user_data) || + neuter (c))); } template inline bool sanitize (hb_sanitize_context_t *c, const void *base, T1 user_data1, T2 user_data2) const { TRACE_SANITIZE (this); - if (unlikely (!sanitize_shallow (c, base))) return_trace (false); - const Type &obj = StructAtOffset (base, *this); - return_trace (likely (obj.sanitize (c, user_data1, user_data2)) || neuter (c)); + return_trace (sanitize_shallow (c, base) && + (StructAtOffset (base, *this).sanitize (c, user_data1, user_data2) || + neuter (c))); } /* Set the offset to Null */ From 4c6b0fb5f6668a6e562260d16f629ad3c41e8961 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 13 Sep 2018 16:39:30 +0200 Subject: [PATCH 06/20] OffsetTo::sanitize() Add version with three user_data --- src/hb-open-type.hh | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/hb-open-type.hh b/src/hb-open-type.hh index 76ce55a76..3926694b0 100644 --- a/src/hb-open-type.hh +++ b/src/hb-open-type.hh @@ -277,20 +277,28 @@ struct OffsetTo : Offset (StructAtOffset (base, *this).sanitize (c) || neuter (c))); } - template - inline bool sanitize (hb_sanitize_context_t *c, const void *base, T user_data) const + template + inline bool sanitize (hb_sanitize_context_t *c, const void *base, T1 d1) const { TRACE_SANITIZE (this); return_trace (sanitize_shallow (c, base) && - (StructAtOffset (base, *this).sanitize (c, user_data) || + (StructAtOffset (base, *this).sanitize (c, d1) || neuter (c))); } template - inline bool sanitize (hb_sanitize_context_t *c, const void *base, T1 user_data1, T2 user_data2) const + inline bool sanitize (hb_sanitize_context_t *c, const void *base, T1 d1, T2 d2) const { TRACE_SANITIZE (this); return_trace (sanitize_shallow (c, base) && - (StructAtOffset (base, *this).sanitize (c, user_data1, user_data2) || + (StructAtOffset (base, *this).sanitize (c, d1, d2) || + neuter (c))); + } + template + inline bool sanitize (hb_sanitize_context_t *c, const void *base, T1 d1, T2 d2, T3 d3) const + { + TRACE_SANITIZE (this); + return_trace (sanitize_shallow (c, base) && + (StructAtOffset (base, *this).sanitize (c, d1, d2, d3) || neuter (c))); } From 361fc2686152ad8c0ebaf19e0522e0fc58ba3953 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 13 Sep 2018 16:47:33 +0200 Subject: [PATCH 07/20] Fix OffsetTo::sanitize() after reshuffling --- src/hb-open-type.hh | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/hb-open-type.hh b/src/hb-open-type.hh index 3926694b0..973dc5e11 100644 --- a/src/hb-open-type.hh +++ b/src/hb-open-type.hh @@ -274,7 +274,8 @@ struct OffsetTo : Offset { TRACE_SANITIZE (this); return_trace (sanitize_shallow (c, base) && - (StructAtOffset (base, *this).sanitize (c) || + (!*this || + StructAtOffset (base, *this).sanitize (c) || neuter (c))); } template @@ -282,7 +283,8 @@ struct OffsetTo : Offset { TRACE_SANITIZE (this); return_trace (sanitize_shallow (c, base) && - (StructAtOffset (base, *this).sanitize (c, d1) || + (!*this || + StructAtOffset (base, *this).sanitize (c, d1) || neuter (c))); } template @@ -290,7 +292,8 @@ struct OffsetTo : Offset { TRACE_SANITIZE (this); return_trace (sanitize_shallow (c, base) && - (StructAtOffset (base, *this).sanitize (c, d1, d2) || + (!*this || + StructAtOffset (base, *this).sanitize (c, d1, d2) || neuter (c))); } template @@ -298,7 +301,8 @@ struct OffsetTo : Offset { TRACE_SANITIZE (this); return_trace (sanitize_shallow (c, base) && - (StructAtOffset (base, *this).sanitize (c, d1, d2, d3) || + (!*this || + StructAtOffset (base, *this).sanitize (c, d1, d2, d3) || neuter (c))); } From dbb764dceb61365b7360a48d581ba5a4b3526e98 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 13 Sep 2018 16:49:26 +0200 Subject: [PATCH 08/20] [dfont] Clean up sanitize() I don't think I broke anything. Fuzzers will let me know.. --- src/hb-dsalgs.hh | 8 +++ src/hb-open-file.hh | 122 ++++++++++++++++++++------------------------ 2 files changed, 64 insertions(+), 66 deletions(-) diff --git a/src/hb-dsalgs.hh b/src/hb-dsalgs.hh index def968eb7..26e8e42a4 100644 --- a/src/hb-dsalgs.hh +++ b/src/hb-dsalgs.hh @@ -532,9 +532,17 @@ struct hb_bytes_t inline hb_bytes_t (void) : bytes (nullptr), len (0) {} inline hb_bytes_t (const char *bytes_, unsigned int len_) : bytes (bytes_), len (len_) {} inline hb_bytes_t (const void *bytes_, unsigned int len_) : bytes ((const char *) bytes_), len (len_) {} + template + inline hb_bytes_t (const T& array) : bytes ((const char *) array.arrayZ), len (array.len) {} inline void free (void) { ::free ((void *) bytes); bytes = nullptr; len = 0; } + template + inline const Type* as (void) const + { + return unlikely (!bytes) ? &Null(Type) : reinterpret_cast (bytes); + } + inline int cmp (const hb_bytes_t &a) const { if (len != a.len) diff --git a/src/hb-open-file.hh b/src/hb-open-file.hh index 26e68bb54..5cff0806b 100644 --- a/src/hb-open-file.hh +++ b/src/hb-open-file.hh @@ -293,18 +293,24 @@ struct TTCHeader struct ResourceRecord { - inline bool sanitize (hb_sanitize_context_t *c) const + inline const hb_bytes_t get_data (const void *data_base) const + { return hb_bytes_t (data_base+offset); } + + inline bool sanitize (hb_sanitize_context_t *c, + const void *data_base) const { TRACE_SANITIZE (this); - // actual data sanitization is done on ResourceForkHeader sanitizer - return_trace (likely (c->check_struct (this))); + return_trace (c->check_struct (this) && + offset.sanitize (c, data_base)); } + protected: HBUINT16 id; /* Resource ID, is really should be signed? */ HBINT16 nameOffset; /* Offset from beginning of resource name list * to resource name, -1 means there is none. */ HBUINT8 attrs; /* Resource attributes */ - HBUINT24 dataOffset; /* Offset from beginning of resource data to + OffsetTo, HBUINT24> + offset; /* Offset from beginning of data block to * data for this resource */ HBUINT32 reserved; /* Reserved for handle to resource */ public: @@ -313,28 +319,33 @@ struct ResourceRecord struct ResourceTypeRecord { - inline bool sanitize (hb_sanitize_context_t *c) const - { - TRACE_SANITIZE (this); - // RefList sanitization is done on ResourceMap sanitizer - return_trace (likely (c->check_struct (this))); - } - inline unsigned int get_resource_count () const { return resCountM1 + 1; } inline bool is_sfnt () const { return tag == HB_TAG ('s','f','n','t'); } - inline const ResourceRecord& get_resource_record (const void *base, - unsigned int i) const + inline const ResourceRecord& get_resource_record (unsigned int i, + const void *type_base) const { - return (base+refList)[i]; + return hb_array_t ((type_base+resourcesZ).arrayZ, + get_resource_count ()) [i]; + } + + inline bool sanitize (hb_sanitize_context_t *c, + const void *type_base, + const void *data_base) const + { + TRACE_SANITIZE (this); + return_trace (c->check_struct (this) && + resourcesZ.sanitize (c, type_base, + get_resource_count (), + data_base)); } protected: Tag tag; /* Resource type. */ HBUINT16 resCountM1; /* Number of resources minus 1. */ OffsetTo > - refList; /* Offset from beginning of resource type list + resourcesZ; /* Offset from beginning of resource type list * to reference item list for this type. */ public: DEFINE_SIZE_STATIC (8); @@ -342,35 +353,30 @@ struct ResourceTypeRecord struct ResourceMap { - inline bool sanitize (hb_sanitize_context_t *c) const - { - TRACE_SANITIZE (this); - if (unlikely (!c->check_struct (this))) - return_trace (false); - for (unsigned int i = 0; i < get_types_count (); ++i) - { - const ResourceTypeRecord& type = get_type_record (i); - if (unlikely (!type.sanitize (c))) - return_trace (false); - for (unsigned int j = 0; j < type.get_resource_count (); ++j) - if (unlikely (!get_resource_record (type, j).sanitize (c))) - return_trace (false); - } - return_trace (true); - } - inline const ResourceTypeRecord& get_type_record (unsigned int i) const { // Why offset from the third byte of the object? I'm not sure - return (((const char *) this + 2)+typeListZ)[i]; + return hb_array_t (((2 + (const char *) this )+typeListZ).arrayZ, + get_type_count ()) [i]; } - inline unsigned int get_types_count () const { return typeCountM1 + 1; } + inline unsigned int get_type_count () const { return typeCountM1 + 1; } inline const ResourceRecord &get_resource_record (const ResourceTypeRecord &type, unsigned int i) const { - return type.get_resource_record (&(this+typeListZ), i); + return type.get_resource_record (i, &(this+typeListZ)); + } + + inline bool sanitize (hb_sanitize_context_t *c, const void *data_base) const + { + TRACE_SANITIZE (this); + const void *type_base = &(this+typeListZ); + return_trace (c->check_struct (this) && + typeListZ.sanitize (c, 2 + (const char *) this, + get_type_count (), + type_base, + data_base)); } protected: @@ -393,7 +399,8 @@ struct ResourceForkHeader inline unsigned int get_face_count () const { const ResourceMap &resource_map = this+map; - for (unsigned int i = 0; i < resource_map.get_types_count (); ++i) + unsigned int count = resource_map.get_type_count (); + for (unsigned int i = 0; i < count; ++i) { const ResourceTypeRecord& type = resource_map.get_type_record (i); if (type.is_sfnt ()) @@ -402,23 +409,24 @@ struct ResourceForkHeader return 0; } - inline const LArrayOf& get_data (const ResourceTypeRecord& type, - unsigned int idx) const + inline const hb_bytes_t get_data (const ResourceTypeRecord& type, + unsigned int idx) const { const ResourceMap &resource_map = this+map; - unsigned int offset = dataOffset + resource_map.get_resource_record (type, idx).dataOffset; - return StructAtOffset > (this, offset); + const void *data_base = &(this+data); + return resource_map.get_resource_record (type, idx).get_data (data_base); } - inline const OpenTypeFontFace& get_face (unsigned int idx, unsigned int *base_offset = nullptr) const + inline const OpenTypeFontFace& get_face (unsigned int idx, + unsigned int *base_offset = nullptr) const { const ResourceMap &resource_map = this+map; - for (unsigned int i = 0; i < resource_map.get_types_count (); ++i) + for (unsigned int i = 0; i < resource_map.get_type_count (); ++i) { const ResourceTypeRecord& type = resource_map.get_type_record (i); if (type.is_sfnt () && idx < type.get_resource_count ()) { - const OpenTypeFontFace &face = (OpenTypeFontFace&) get_data (type, idx).arrayZ; + const OpenTypeFontFace &face = *get_data (type, idx).as (); if (base_offset) *base_offset = (const char *) &face - (const char *) this; return face; @@ -430,33 +438,15 @@ struct ResourceForkHeader inline bool sanitize (hb_sanitize_context_t *c) const { TRACE_SANITIZE (this); - if (unlikely (!c->check_struct (this))) - return_trace (false); + return_trace (c->check_struct (this) && + map.sanitize (c, this, &(this+data))); - const ResourceMap &resource_map = this+map; - if (unlikely (!resource_map.sanitize (c))) - return_trace (false); - - for (unsigned int i = 0; i < resource_map.get_types_count (); ++i) - { - const ResourceTypeRecord& type = resource_map.get_type_record (i); - for (unsigned int j = 0; j < type.get_resource_count (); ++j) - { - const LArrayOf& data = get_data (type, j); - if (unlikely (!data.sanitize (c))) - return_trace (false); - - if (unlikely (type.is_sfnt () && - !((OpenTypeFontFace&) data.arrayZ).sanitize (c))) - return_trace (false); - } - } - - return_trace (true); + // XXX Sanitize OpenTypeFontFace's } protected: - Offset32 dataOffset; /* Offset from beginning of resource fork + LOffsetTo > + data; /* Offset from beginning of resource fork * to resource data */ LOffsetTo map; /* Offset from beginning of resource fork From 07e0ca930c29757217c2f9e4e0e6954657b6b82d Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 13 Sep 2018 17:39:09 +0200 Subject: [PATCH 09/20] [bytes] Rename content to arrayZ --- src/hb-dsalgs.hh | 16 ++++++++-------- src/hb-ot-post-table.hh | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/hb-dsalgs.hh b/src/hb-dsalgs.hh index 26e8e42a4..692002e9a 100644 --- a/src/hb-dsalgs.hh +++ b/src/hb-dsalgs.hh @@ -529,18 +529,18 @@ struct hb_array_t struct hb_bytes_t { - inline hb_bytes_t (void) : bytes (nullptr), len (0) {} - inline hb_bytes_t (const char *bytes_, unsigned int len_) : bytes (bytes_), len (len_) {} - inline hb_bytes_t (const void *bytes_, unsigned int len_) : bytes ((const char *) bytes_), len (len_) {} + inline hb_bytes_t (void) : arrayZ (nullptr), len (0) {} + inline hb_bytes_t (const char *bytes_, unsigned int len_) : arrayZ (bytes_), len (len_) {} + inline hb_bytes_t (const void *bytes_, unsigned int len_) : arrayZ ((const char *) bytes_), len (len_) {} template - inline hb_bytes_t (const T& array) : bytes ((const char *) array.arrayZ), len (array.len) {} + inline hb_bytes_t (const T& array) : arrayZ ((const char *) array.arrayZ), len (array.len) {} - inline void free (void) { ::free ((void *) bytes); bytes = nullptr; len = 0; } + inline void free (void) { ::free ((void *) arrayZ); arrayZ = nullptr; len = 0; } template inline const Type* as (void) const { - return unlikely (!bytes) ? &Null(Type) : reinterpret_cast (bytes); + return unlikely (!arrayZ) ? &Null(Type) : reinterpret_cast (arrayZ); } inline int cmp (const hb_bytes_t &a) const @@ -548,7 +548,7 @@ struct hb_bytes_t if (len != a.len) return (int) a.len - (int) len; - return memcmp (a.bytes, bytes, len); + return memcmp (a.arrayZ, arrayZ, len); } static inline int cmp (const void *pa, const void *pb) { @@ -557,7 +557,7 @@ struct hb_bytes_t return b->cmp (*a); } - const char *bytes; + const char *arrayZ; unsigned int len; }; diff --git a/src/hb-ot-post-table.hh b/src/hb-ot-post-table.hh index 29dc86fa8..5f27fd504 100644 --- a/src/hb-ot-post-table.hh +++ b/src/hb-ot-post-table.hh @@ -143,7 +143,7 @@ struct post return true; if (buf_len <= s.len) /* What to do with truncation? Returning false for now. */ return false; - strncpy (buf, s.bytes, s.len); + strncpy (buf, s.arrayZ, s.len); buf[s.len] = '\0'; return true; } From 82f4d776c21b7c1224dd7073ce69cdf76d85f16b Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 13 Sep 2018 18:27:20 +0200 Subject: [PATCH 10/20] [dfont] Minor --- src/hb-open-file.hh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hb-open-file.hh b/src/hb-open-file.hh index 5cff0806b..d3e87225a 100644 --- a/src/hb-open-file.hh +++ b/src/hb-open-file.hh @@ -305,7 +305,7 @@ struct ResourceRecord } protected: - HBUINT16 id; /* Resource ID, is really should be signed? */ + HBUINT16 id; /* Resource ID. */ HBINT16 nameOffset; /* Offset from beginning of resource name list * to resource name, -1 means there is none. */ HBUINT8 attrs; /* Resource attributes */ @@ -400,7 +400,7 @@ struct ResourceForkHeader { const ResourceMap &resource_map = this+map; unsigned int count = resource_map.get_type_count (); - for (unsigned int i = 0; i < count; ++i) + for (unsigned int i = 0; i < count; i++) { const ResourceTypeRecord& type = resource_map.get_type_record (i); if (type.is_sfnt ()) @@ -421,7 +421,7 @@ struct ResourceForkHeader unsigned int *base_offset = nullptr) const { const ResourceMap &resource_map = this+map; - for (unsigned int i = 0; i < resource_map.get_type_count (); ++i) + for (unsigned int i = 0; i < resource_map.get_type_count (); i++) { const ResourceTypeRecord& type = resource_map.get_type_record (i); if (type.is_sfnt () && idx < type.get_resource_count ()) From 29faebe911a13916aa3d737e93d38deedc53567f Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 13 Sep 2018 18:45:35 +0200 Subject: [PATCH 11/20] Allow Offset<>'s that have no 0==null --- src/hb-open-type.hh | 43 +++++++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/src/hb-open-type.hh b/src/hb-open-type.hh index 973dc5e11..b10d33b16 100644 --- a/src/hb-open-type.hh +++ b/src/hb-open-type.hh @@ -155,10 +155,10 @@ struct Index : HBUINT16 { DECLARE_NULL_NAMESPACE_BYTES (OT, Index); /* Offset, Null offset = 0 */ -template +template struct Offset : Type { - inline bool is_null (void) const { return 0 == *this; } + inline bool is_null (void) const { return has_null && 0 == *this; } inline void *serialize (hb_serialize_context_t *c, const void *base) { @@ -226,20 +226,18 @@ struct FixedVersion * Use: (base+offset) */ -template -struct OffsetTo : Offset +template +struct OffsetTo : Offset { inline const Type& operator () (const void *base) const { - unsigned int offset = *this; - if (unlikely (!offset)) return Null(Type); - return StructAtOffset (base, offset); + if (unlikely (this->is_null ())) return Null(Type); + return StructAtOffset (base, *this); } inline Type& operator () (void *base) const { - unsigned int offset = *this; - if (unlikely (!offset)) return Crap(Type); - return StructAtOffset (base, offset); + if (unlikely (this->is_null ())) return Crap(Type); + return StructAtOffset (base, *this); } inline Type& serialize (hb_serialize_context_t *c, const void *base) @@ -264,9 +262,8 @@ struct OffsetTo : Offset { TRACE_SANITIZE (this); if (unlikely (!c->check_struct (this))) return_trace (false); - unsigned int offset = *this; - if (unlikely (!offset)) return_trace (true); - if (unlikely (!c->check_range (base, offset))) return_trace (false); + if (unlikely (this->is_null ())) return_trace (true); + if (unlikely (!c->check_range (base, *this))) return_trace (false); return_trace (true); } @@ -274,7 +271,7 @@ struct OffsetTo : Offset { TRACE_SANITIZE (this); return_trace (sanitize_shallow (c, base) && - (!*this || + (this->is_null () || StructAtOffset (base, *this).sanitize (c) || neuter (c))); } @@ -283,7 +280,7 @@ struct OffsetTo : Offset { TRACE_SANITIZE (this); return_trace (sanitize_shallow (c, base) && - (!*this || + (this->is_null () || StructAtOffset (base, *this).sanitize (c, d1) || neuter (c))); } @@ -292,7 +289,7 @@ struct OffsetTo : Offset { TRACE_SANITIZE (this); return_trace (sanitize_shallow (c, base) && - (!*this || + (this->is_null () || StructAtOffset (base, *this).sanitize (c, d1, d2) || neuter (c))); } @@ -301,22 +298,24 @@ struct OffsetTo : Offset { TRACE_SANITIZE (this); return_trace (sanitize_shallow (c, base) && - (!*this || + (this->is_null () || StructAtOffset (base, *this).sanitize (c, d1, d2, d3) || neuter (c))); } /* Set the offset to Null */ - inline bool neuter (hb_sanitize_context_t *c) const { + inline bool neuter (hb_sanitize_context_t *c) const + { + if (!has_null) return false; return c->try_set (this, 0); } DEFINE_SIZE_STATIC (sizeof(OffsetType)); }; template struct LOffsetTo : OffsetTo {}; -template -static inline const Type& operator + (const Base &base, const OffsetTo &offset) { return offset (base); } -template -static inline Type& operator + (Base &base, OffsetTo &offset) { return offset (base); } +template +static inline const Type& operator + (const Base &base, const OffsetTo &offset) { return offset (base); } +template +static inline Type& operator + (Base &base, OffsetTo &offset) { return offset (base); } /* From bf852f0e62a8bdbb809af6a975f8ae8eed708d70 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 13 Sep 2018 18:47:53 +0200 Subject: [PATCH 12/20] [dfont] Make test pass Offset 0 is not null in this context. --- src/hb-open-file.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hb-open-file.hh b/src/hb-open-file.hh index d3e87225a..b960d62d0 100644 --- a/src/hb-open-file.hh +++ b/src/hb-open-file.hh @@ -309,7 +309,7 @@ struct ResourceRecord HBINT16 nameOffset; /* Offset from beginning of resource name list * to resource name, -1 means there is none. */ HBUINT8 attrs; /* Resource attributes */ - OffsetTo, HBUINT24> + OffsetTo, HBUINT24, false> offset; /* Offset from beginning of data block to * data for this resource */ HBUINT32 reserved; /* Reserved for handle to resource */ From 3fba41906fba28c5ea01cc0749654de862453bf4 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 13 Sep 2018 18:49:16 +0200 Subject: [PATCH 13/20] [dfont] Minor --- src/hb-open-file.hh | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/hb-open-file.hh b/src/hb-open-file.hh index b960d62d0..92dca3f89 100644 --- a/src/hb-open-file.hh +++ b/src/hb-open-file.hh @@ -411,17 +411,14 @@ struct ResourceForkHeader inline const hb_bytes_t get_data (const ResourceTypeRecord& type, unsigned int idx) const - { - const ResourceMap &resource_map = this+map; - const void *data_base = &(this+data); - return resource_map.get_resource_record (type, idx).get_data (data_base); - } + { return (this+map).get_resource_record (type, idx).get_data (&(this+data)); } inline const OpenTypeFontFace& get_face (unsigned int idx, unsigned int *base_offset = nullptr) const { const ResourceMap &resource_map = this+map; - for (unsigned int i = 0; i < resource_map.get_type_count (); i++) + unsigned int count = resource_map.get_type_count (); + for (unsigned int i = 0; i < count; i++) { const ResourceTypeRecord& type = resource_map.get_type_record (i); if (type.is_sfnt () && idx < type.get_resource_count ()) From 4479d3a2eda57d278700f5c78414ef6ef617d2a9 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 13 Sep 2018 19:05:59 +0200 Subject: [PATCH 14/20] [dfon]t Sanitize OpenTypeFontFace --- src/hb-dsalgs.hh | 6 ------ src/hb-open-file.hh | 21 +++++++++------------ 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/src/hb-dsalgs.hh b/src/hb-dsalgs.hh index 692002e9a..eb15c089e 100644 --- a/src/hb-dsalgs.hh +++ b/src/hb-dsalgs.hh @@ -537,12 +537,6 @@ struct hb_bytes_t inline void free (void) { ::free ((void *) arrayZ); arrayZ = nullptr; len = 0; } - template - inline const Type* as (void) const - { - return unlikely (!arrayZ) ? &Null(Type) : reinterpret_cast (arrayZ); - } - inline int cmp (const hb_bytes_t &a) const { if (len != a.len) diff --git a/src/hb-open-file.hh b/src/hb-open-file.hh index 92dca3f89..ce39bfc9d 100644 --- a/src/hb-open-file.hh +++ b/src/hb-open-file.hh @@ -293,15 +293,16 @@ struct TTCHeader struct ResourceRecord { - inline const hb_bytes_t get_data (const void *data_base) const - { return hb_bytes_t (data_base+offset); } + inline const OpenTypeFontFace & get_face (const void *data_base) const + { return CastR ((data_base+offset).arrayZ); } inline bool sanitize (hb_sanitize_context_t *c, const void *data_base) const { TRACE_SANITIZE (this); return_trace (c->check_struct (this) && - offset.sanitize (c, data_base)); + offset.sanitize (c, data_base) && + get_face (data_base).sanitize (c)); } protected: @@ -317,11 +318,13 @@ struct ResourceRecord DEFINE_SIZE_STATIC (12); }; +#define HB_TAG_sfnt HB_TAG ('s','f','n','t') + struct ResourceTypeRecord { - inline unsigned int get_resource_count () const { return resCountM1 + 1; } + inline unsigned int get_resource_count () const { return tag == HB_TAG_sfnt ? resCountM1 + 1 : 0; } - inline bool is_sfnt () const { return tag == HB_TAG ('s','f','n','t'); } + inline bool is_sfnt () const { return tag == HB_TAG_sfnt; } inline const ResourceRecord& get_resource_record (unsigned int i, const void *type_base) const @@ -409,10 +412,6 @@ struct ResourceForkHeader return 0; } - inline const hb_bytes_t get_data (const ResourceTypeRecord& type, - unsigned int idx) const - { return (this+map).get_resource_record (type, idx).get_data (&(this+data)); } - inline const OpenTypeFontFace& get_face (unsigned int idx, unsigned int *base_offset = nullptr) const { @@ -423,7 +422,7 @@ struct ResourceForkHeader const ResourceTypeRecord& type = resource_map.get_type_record (i); if (type.is_sfnt () && idx < type.get_resource_count ()) { - const OpenTypeFontFace &face = *get_data (type, idx).as (); + const OpenTypeFontFace &face = resource_map.get_resource_record (type, idx).get_face (&(this+data)); if (base_offset) *base_offset = (const char *) &face - (const char *) this; return face; @@ -437,8 +436,6 @@ struct ResourceForkHeader TRACE_SANITIZE (this); return_trace (c->check_struct (this) && map.sanitize (c, this, &(this+data))); - - // XXX Sanitize OpenTypeFontFace's } protected: From 8c9bdcc1feeab321a642bdaac50b716e48ce4263 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 13 Sep 2018 19:08:22 +0200 Subject: [PATCH 15/20] [dfont] Minor --- src/hb-open-file.hh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/hb-open-file.hh b/src/hb-open-file.hh index ce39bfc9d..0c669119e 100644 --- a/src/hb-open-file.hh +++ b/src/hb-open-file.hh @@ -322,9 +322,9 @@ struct ResourceRecord struct ResourceTypeRecord { - inline unsigned int get_resource_count () const { return tag == HB_TAG_sfnt ? resCountM1 + 1 : 0; } + inline unsigned int get_resource_count (void) const { return tag == HB_TAG_sfnt ? resCountM1 + 1 : 0; } - inline bool is_sfnt () const { return tag == HB_TAG_sfnt; } + inline bool is_sfnt (void) const { return tag == HB_TAG_sfnt; } inline const ResourceRecord& get_resource_record (unsigned int i, const void *type_base) const @@ -363,7 +363,7 @@ struct ResourceMap get_type_count ()) [i]; } - inline unsigned int get_type_count () const { return typeCountM1 + 1; } + inline unsigned int get_type_count (void) const { return typeCountM1 + 1; } inline const ResourceRecord &get_resource_record (const ResourceTypeRecord &type, unsigned int i) const @@ -399,7 +399,7 @@ struct ResourceMap struct ResourceForkHeader { - inline unsigned int get_face_count () const + inline unsigned int get_face_count (void) const { const ResourceMap &resource_map = this+map; unsigned int count = resource_map.get_type_count (); From 0ab0f1e5ac5ccb07c57364e9f5be0b991398eb6f Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 13 Sep 2018 19:13:01 +0200 Subject: [PATCH 16/20] [dfont] Push methods further down --- src/hb-open-file.hh | 58 ++++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 25 deletions(-) diff --git a/src/hb-open-file.hh b/src/hb-open-file.hh index 0c669119e..89b76d82d 100644 --- a/src/hb-open-file.hh +++ b/src/hb-open-file.hh @@ -371,6 +371,34 @@ struct ResourceMap return type.get_resource_record (i, &(this+typeListZ)); } + inline unsigned int get_face_count (void) const + { + unsigned int count = get_type_count (); + for (unsigned int i = 0; i < count; i++) + { + const ResourceTypeRecord& type = get_type_record (i); + if (type.is_sfnt ()) + return type.get_resource_count (); + } + return 0; + } + + inline const OpenTypeFontFace& get_face (unsigned int idx, + const void *data_base) const + { + unsigned int count = get_type_count (); + for (unsigned int i = 0; i < count; i++) + { + const ResourceTypeRecord& type = get_type_record (i); + if (type.is_sfnt () && idx < type.get_resource_count ()) + { + const OpenTypeFontFace &face = get_resource_record (type, idx).get_face (data_base); + return face; + } + } + return Null (OpenTypeFontFace); + } + inline bool sanitize (hb_sanitize_context_t *c, const void *data_base) const { TRACE_SANITIZE (this); @@ -400,35 +428,15 @@ struct ResourceMap struct ResourceForkHeader { inline unsigned int get_face_count (void) const - { - const ResourceMap &resource_map = this+map; - unsigned int count = resource_map.get_type_count (); - for (unsigned int i = 0; i < count; i++) - { - const ResourceTypeRecord& type = resource_map.get_type_record (i); - if (type.is_sfnt ()) - return type.get_resource_count (); - } - return 0; - } + { return (this+map).get_face_count (); } inline const OpenTypeFontFace& get_face (unsigned int idx, unsigned int *base_offset = nullptr) const { - const ResourceMap &resource_map = this+map; - unsigned int count = resource_map.get_type_count (); - for (unsigned int i = 0; i < count; i++) - { - const ResourceTypeRecord& type = resource_map.get_type_record (i); - if (type.is_sfnt () && idx < type.get_resource_count ()) - { - const OpenTypeFontFace &face = resource_map.get_resource_record (type, idx).get_face (&(this+data)); - if (base_offset) - *base_offset = (const char *) &face - (const char *) this; - return face; - } - } - return Null (OpenTypeFontFace); + const OpenTypeFontFace &face = (this+map).get_face (idx, &(this+data)); + if (base_offset) + *base_offset = (const char *) &face - (const char *) this; + return face; } inline bool sanitize (hb_sanitize_context_t *c) const From 180a88a96ce327e4103df3635c73559de65d1546 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 13 Sep 2018 19:19:57 +0200 Subject: [PATCH 17/20] [dfont] Some more --- src/hb-open-file.hh | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/hb-open-file.hh b/src/hb-open-file.hh index 89b76d82d..5c653fed2 100644 --- a/src/hb-open-file.hh +++ b/src/hb-open-file.hh @@ -322,7 +322,8 @@ struct ResourceRecord struct ResourceTypeRecord { - inline unsigned int get_resource_count (void) const { return tag == HB_TAG_sfnt ? resCountM1 + 1 : 0; } + inline unsigned int get_resource_count (void) const + { return tag == HB_TAG_sfnt ? resCountM1 + 1 : 0; } inline bool is_sfnt (void) const { return tag == HB_TAG_sfnt; } @@ -363,14 +364,6 @@ struct ResourceMap get_type_count ()) [i]; } - inline unsigned int get_type_count (void) const { return typeCountM1 + 1; } - - inline const ResourceRecord &get_resource_record (const ResourceTypeRecord &type, - unsigned int i) const - { - return type.get_resource_record (i, &(this+typeListZ)); - } - inline unsigned int get_face_count (void) const { unsigned int count = get_type_count (); @@ -390,11 +383,10 @@ struct ResourceMap for (unsigned int i = 0; i < count; i++) { const ResourceTypeRecord& type = get_type_record (i); + /* The check for idx < count is here because ResourceRecord is NOT null-safe. + * Because an offset of 0 there does NOT mean null. */ if (type.is_sfnt () && idx < type.get_resource_count ()) - { - const OpenTypeFontFace &face = get_resource_record (type, idx).get_face (data_base); - return face; - } + return type.get_resource_record (idx, &(this+typeListZ)).get_face (data_base); } return Null (OpenTypeFontFace); } @@ -410,6 +402,9 @@ struct ResourceMap data_base)); } + private: + inline unsigned int get_type_count (void) const { return typeCountM1 + 1; } + protected: HBUINT8 reserved0[16]; /* Reserved for copy of resource header */ HBUINT32 reserved1; /* Reserved for handle to next resource map */ From effc7ced72a6ce0fea328a8b68dc3d55f09774f1 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 13 Sep 2018 20:21:54 +0200 Subject: [PATCH 18/20] Rename HeadlessArrayOf::len to lenP1 So it doesn't accidentally match our templates, etc. --- src/hb-open-type.hh | 14 +++++++------- src/hb-ot-layout-gsub-table.hh | 10 +++++----- src/hb-ot-layout-gsubgpos.hh | 10 +++++----- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/hb-open-type.hh b/src/hb-open-type.hh index b10d33b16..680797387 100644 --- a/src/hb-open-type.hh +++ b/src/hb-open-type.hh @@ -570,16 +570,16 @@ struct HeadlessArrayOf { inline const Type& operator [] (unsigned int i) const { - if (unlikely (i >= len || !i)) return Null(Type); + if (unlikely (i >= lenP1 || !i)) return Null(Type); return arrayZ[i-1]; } inline Type& operator [] (unsigned int i) { - if (unlikely (i >= len || !i)) return Crap(Type); + if (unlikely (i >= lenP1 || !i)) return Crap(Type); return arrayZ[i-1]; } inline unsigned int get_size (void) const - { return len.static_size + (len ? len - 1 : 0) * Type::static_size; } + { return lenP1.static_size + (lenP1 ? lenP1 - 1 : 0) * Type::static_size; } inline bool serialize (hb_serialize_context_t *c, Supplier &items, @@ -587,7 +587,7 @@ struct HeadlessArrayOf { TRACE_SERIALIZE (this); if (unlikely (!c->extend_min (*this))) return_trace (false); - len.set (items_len); /* TODO(serialize) Overflow? */ + lenP1.set (items_len); /* TODO(serialize) Overflow? */ if (unlikely (!items_len)) return_trace (true); if (unlikely (!c->extend (*this))) return_trace (false); for (unsigned int i = 0; i < items_len - 1; i++) @@ -617,12 +617,12 @@ struct HeadlessArrayOf inline bool sanitize_shallow (hb_sanitize_context_t *c) const { TRACE_SANITIZE (this); - return_trace (len.sanitize (c) && - (!len || c->check_array (arrayZ, len - 1))); + return_trace (lenP1.sanitize (c) && + (!lenP1 || c->check_array (arrayZ, lenP1 - 1))); } public: - LenType len; + LenType lenP1; Type arrayZ[VAR]; public: DEFINE_SIZE_ARRAY (sizeof (LenType), arrayZ); diff --git a/src/hb-ot-layout-gsub-table.hh b/src/hb-ot-layout-gsub-table.hh index 4d5db6aec..7e36e0639 100644 --- a/src/hb-ot-layout-gsub-table.hh +++ b/src/hb-ot-layout-gsub-table.hh @@ -710,7 +710,7 @@ struct Ligature { inline bool intersects (const hb_set_t *glyphs) const { - unsigned int count = component.len; + unsigned int count = component.lenP1; for (unsigned int i = 1; i < count; i++) if (!glyphs->has (component[i])) return false; @@ -720,7 +720,7 @@ struct Ligature inline void closure (hb_closure_context_t *c) const { TRACE_CLOSURE (this); - unsigned int count = component.len; + unsigned int count = component.lenP1; for (unsigned int i = 1; i < count; i++) if (!c->glyphs->has (component[i])) return; @@ -730,14 +730,14 @@ struct Ligature inline void collect_glyphs (hb_collect_glyphs_context_t *c) const { TRACE_COLLECT_GLYPHS (this); - c->input->add_array (component.arrayZ, component.len ? component.len - 1 : 0); + c->input->add_array (component.arrayZ, component.lenP1 ? component.lenP1 - 1 : 0); c->output->add (ligGlyph); } inline bool would_apply (hb_would_apply_context_t *c) const { TRACE_WOULD_APPLY (this); - if (c->len != component.len) + if (c->len != component.lenP1) return_trace (false); for (unsigned int i = 1; i < c->len; i++) @@ -750,7 +750,7 @@ struct Ligature inline bool apply (hb_ot_apply_context_t *c) const { TRACE_APPLY (this); - unsigned int count = component.len; + unsigned int count = component.lenP1; if (unlikely (!count)) return_trace (false); diff --git a/src/hb-ot-layout-gsubgpos.hh b/src/hb-ot-layout-gsubgpos.hh index eae73ce33..ff9783edc 100644 --- a/src/hb-ot-layout-gsubgpos.hh +++ b/src/hb-ot-layout-gsubgpos.hh @@ -1879,7 +1879,7 @@ struct ChainRule const ArrayOf &lookahead = StructAfter > (input); return chain_context_intersects (glyphs, backtrack.len, backtrack.arrayZ, - input.len, input.arrayZ, + input.lenP1, input.arrayZ, lookahead.len, lookahead.arrayZ, lookup_context); } @@ -1892,7 +1892,7 @@ struct ChainRule const ArrayOf &lookup = StructAfter > (lookahead); chain_context_closure_lookup (c, backtrack.len, backtrack.arrayZ, - input.len, input.arrayZ, + input.lenP1, input.arrayZ, lookahead.len, lookahead.arrayZ, lookup.len, lookup.arrayZ, lookup_context); @@ -1906,7 +1906,7 @@ struct ChainRule const ArrayOf &lookup = StructAfter > (lookahead); chain_context_collect_glyphs_lookup (c, backtrack.len, backtrack.arrayZ, - input.len, input.arrayZ, + input.lenP1, input.arrayZ, lookahead.len, lookahead.arrayZ, lookup.len, lookup.arrayZ, lookup_context); @@ -1920,7 +1920,7 @@ struct ChainRule const ArrayOf &lookup = StructAfter > (lookahead); return_trace (chain_context_would_apply_lookup (c, backtrack.len, backtrack.arrayZ, - input.len, input.arrayZ, + input.lenP1, input.arrayZ, lookahead.len, lookahead.arrayZ, lookup.len, lookup.arrayZ, lookup_context)); } @@ -1933,7 +1933,7 @@ struct ChainRule const ArrayOf &lookup = StructAfter > (lookahead); return_trace (chain_context_apply_lookup (c, backtrack.len, backtrack.arrayZ, - input.len, input.arrayZ, + input.lenP1, input.arrayZ, lookahead.len, lookahead.arrayZ, lookup.len, lookup.arrayZ, lookup_context)); } From 3789c557ca06aef430726f4942cafecac6fe4eef Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 13 Sep 2018 20:30:04 +0200 Subject: [PATCH 19/20] [dfont] Solve the mystery +2 offset thing! Previously, ResourceForkHeader was defined as 30 bytes, having the typeCountM1 as last member. There was a mysterious offset-by-2 in the code, derived from FontTools and JDK code this was ported from. In testing, I observed that typeListZ offset is actually 28. Suggesting that the typeCountM1 does NOT actually belong to ResourceForkHeader, but belongs to the array itself. Adjusting for that resolves the mystery +2 offset hack, so everything is clean and good now. This, concludes my dfont hacking. The code looks great now, and I'm happy to leave it. Fuzzers might disagree though, we will see! --- src/hb-open-file.hh | 30 ++++++++++++------------------ src/hb-open-type.hh | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 18 deletions(-) diff --git a/src/hb-open-file.hh b/src/hb-open-file.hh index 5c653fed2..cd7d78a3e 100644 --- a/src/hb-open-file.hh +++ b/src/hb-open-file.hh @@ -357,13 +357,6 @@ struct ResourceTypeRecord struct ResourceMap { - inline const ResourceTypeRecord& get_type_record (unsigned int i) const - { - // Why offset from the third byte of the object? I'm not sure - return hb_array_t (((2 + (const char *) this )+typeListZ).arrayZ, - get_type_count ()) [i]; - } - inline unsigned int get_face_count (void) const { unsigned int count = get_type_count (); @@ -386,7 +379,7 @@ struct ResourceMap /* The check for idx < count is here because ResourceRecord is NOT null-safe. * Because an offset of 0 there does NOT mean null. */ if (type.is_sfnt () && idx < type.get_resource_count ()) - return type.get_resource_record (idx, &(this+typeListZ)).get_face (data_base); + return type.get_resource_record (idx, &(this+typeList)).get_face (data_base); } return Null (OpenTypeFontFace); } @@ -394,30 +387,31 @@ struct ResourceMap inline bool sanitize (hb_sanitize_context_t *c, const void *data_base) const { TRACE_SANITIZE (this); - const void *type_base = &(this+typeListZ); + const void *type_base = &(this+typeList); return_trace (c->check_struct (this) && - typeListZ.sanitize (c, 2 + (const char *) this, - get_type_count (), - type_base, - data_base)); + typeList.sanitize (c, this, + type_base, + data_base)); } private: - inline unsigned int get_type_count (void) const { return typeCountM1 + 1; } + inline unsigned int get_type_count (void) const { return (this+typeList).lenM1 + 1; } + + inline const ResourceTypeRecord& get_type_record (unsigned int i) const + { return (this+typeList)[i]; } protected: HBUINT8 reserved0[16]; /* Reserved for copy of resource header */ HBUINT32 reserved1; /* Reserved for handle to next resource map */ HBUINT16 resreved2; /* Reserved for file reference number */ HBUINT16 attrs; /* Resource fork attribute */ - OffsetTo > - typeListZ; /* Offset from beginning of map to + OffsetTo > + typeList; /* Offset from beginning of map to * resource type list */ Offset16 nameList; /* Offset from beginning of map to * resource name list */ - HBUINT16 typeCountM1; /* Number of types in the map minus 1 */ public: - DEFINE_SIZE_STATIC (30); + DEFINE_SIZE_STATIC (28); }; struct ResourceForkHeader diff --git a/src/hb-open-type.hh b/src/hb-open-type.hh index 680797387..4f16c7d34 100644 --- a/src/hb-open-type.hh +++ b/src/hb-open-type.hh @@ -628,6 +628,50 @@ struct HeadlessArrayOf DEFINE_SIZE_ARRAY (sizeof (LenType), arrayZ); }; +/* An array storing length-1. */ +template +struct ArrayOfM1 +{ + inline const Type& operator [] (unsigned int i) const + { + if (unlikely (i > lenM1)) return Null(Type); + return arrayZ[i]; + } + inline Type& operator [] (unsigned int i) + { + if (unlikely (i > lenM1)) return Crap(Type); + return arrayZ[i]; + } + inline unsigned int get_size (void) const + { return lenM1.static_size + (lenM1 + 1) * Type::static_size; } + + template + inline bool sanitize (hb_sanitize_context_t *c, const void *base, T user_data) const + { + TRACE_SANITIZE (this); + if (unlikely (!sanitize_shallow (c))) return_trace (false); + unsigned int count = lenM1 + 1; + for (unsigned int i = 0; i < count; i++) + if (unlikely (!arrayZ[i].sanitize (c, base, user_data))) + return_trace (false); + return_trace (true); + } + + private: + inline bool sanitize_shallow (hb_sanitize_context_t *c) const + { + TRACE_SANITIZE (this); + return_trace (lenM1.sanitize (c) && + (c->check_array (arrayZ, lenM1 + 1))); + } + + public: + LenType lenM1; + Type arrayZ[VAR]; + public: + DEFINE_SIZE_ARRAY (sizeof (LenType), arrayZ); +}; + /* An array with sorted elements. Supports binary searching. */ template struct SortedArrayOf : ArrayOf From ca746f261e1e54cec2f9c8bc7a6f930491e19418 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 13 Sep 2018 20:35:21 +0200 Subject: [PATCH 20/20] [dfont] Also check dataLen range in sanitize Just to disagree with myself re being done with this code... --- src/hb-open-file.hh | 1 + 1 file changed, 1 insertion(+) diff --git a/src/hb-open-file.hh b/src/hb-open-file.hh index cd7d78a3e..a1f931d3c 100644 --- a/src/hb-open-file.hh +++ b/src/hb-open-file.hh @@ -432,6 +432,7 @@ struct ResourceForkHeader { TRACE_SANITIZE (this); return_trace (c->check_struct (this) && + data.sanitize (c, this, dataLen) && map.sanitize (c, this, &(this+data))); }