GNOME Bug 613015 - [HB] Does not sanitize Device tables referenced from ValueRecords

1 files changed, 116 insertions, 31 deletions

diff --git a/pango/opentype/hb-ot-layout-gpos-private.hh b/pango/opentype/hb-ot-layout-gpos-private.hh
index dcbbf0f5..1624e83e 100644
--- a/pango/opentype/hb-ot-layout-gpos-private.hh
+++ b/pango/opentype/hb-ot-layout-gpos-private.hh
@@ -51,30 +51,13 @@ struct ValueFormat : USHORT
     xAdvDevice	= 0x0040,	/* Includes horizontal Device table for advance */
     yAdvDevice	= 0x0080,	/* Includes vertical Device table for advance */
     ignored	= 0x0F00,	/* Was used in TrueType Open for MM fonts */
-    reserved	= 0xF000 	/* For future use */
-  };
-
-  inline unsigned int get_len () const
-  { return _hb_popcount32 ((unsigned int) *this); }
-  inline unsigned int get_size () const
-  { return get_len () * Value::get_size (); }
-
-  void apply_value (hb_ot_layout_context_t *context,
-		    const char          *base,
-		    const Value         *values,
-		    hb_internal_glyph_position_t *glyph_pos) const
-  {
-    unsigned int x_ppem, y_ppem;
-    hb_16dot16_t x_scale, y_scale;
-    unsigned int format = *this;
+    reserved	= 0xF000,	/* For future use */
 
-    if (!format)
-      return;
+    devices	= 0x00F0,	/* Mask for having any Device table */
+  };
 
-    /* All fields are options.  Only those available advance the value
-     * pointer. */
+/* All fields are options.  Only those available advance the value pointer. */
 #if 0
-struct ValueRecord {
   SHORT		xPlacement;		/* Horizontal adjustment for
 					 * placement--in design units */
   SHORT		yPlacement;		/* Vertical adjustment for
@@ -97,9 +80,24 @@ struct ValueRecord {
   Offset	yAdvDevice;		/* Offset to Device table for vertical
 					 * advance--measured from beginning of
 					 * PosTable (may be NULL) */
-};
 #endif
 
+  inline unsigned int get_len () const
+  { return _hb_popcount32 ((unsigned int) *this); }
+  inline unsigned int get_size () const
+  { return get_len () * Value::get_size (); }
+
+  void apply_value (hb_ot_layout_context_t       *context,
+		    const char                   *base,
+		    const Value                  *values,
+		    hb_internal_glyph_position_t *glyph_pos) const
+  {
+    unsigned int x_ppem, y_ppem;
+    hb_16dot16_t x_scale, y_scale;
+    unsigned int format = *this;
+
+    if (!format) return;
+
     x_scale = context->font->x_scale;
     y_scale = context->font->y_scale;
     /* design units -> fractional pixel */
@@ -140,6 +138,68 @@ struct ValueRecord {
         values++;
     }
   }
+
+  private:
+  inline bool sanitize_value_devices (SANITIZE_ARG_DEF, void *base, const Value *values) {
+    unsigned int format = *this;
+
+    if (format & xPlacement) values++;
+    if (format & yPlacement) values++;
+    if (format & xAdvance)   values++;
+    if (format & yAdvance)   values++;
+
+    if ((format & xPlaDevice) && !SANITIZE_BASE (*(OffsetTo<Device>*)values++, base)) return false;
+    if ((format & yPlaDevice) && !SANITIZE_BASE (*(OffsetTo<Device>*)values++, base)) return false;
+    if ((format & xAdvDevice) && !SANITIZE_BASE (*(OffsetTo<Device>*)values++, base)) return false;
+    if ((format & yAdvDevice) && !SANITIZE_BASE (*(OffsetTo<Device>*)values++, base)) return false;
+
+    return true;
+  }
+
+  public:
+
+  inline bool has_device () {
+    unsigned int format = *this;
+    return (format & devices) != 0;
+  }
+
+  inline bool sanitize_value (SANITIZE_ARG_DEF, void *base, const Value *values) {
+    TRACE_SANITIZE ();
+
+    return SANITIZE_MEM (values, get_size ()) &&
+	   (!has_device () || sanitize_value_devices (SANITIZE_ARG, base, values));
+  }
+
+  inline bool sanitize_values (SANITIZE_ARG_DEF, void *base, const Value *values, unsigned int count) {
+    TRACE_SANITIZE ();
+    unsigned int len = get_len ();
+
+    if (!SANITIZE_ARRAY (values, get_size (), count)) return false;
+
+    if (!has_device ()) return true;
+
+    for (unsigned int i = 0; i < count; i++) {
+      if (!sanitize_value_devices (SANITIZE_ARG, base, values))
+        return false;
+      values += len;
+    }
+
+    return true;
+  }
+
+  inline bool sanitize_values_stride_unsafe (SANITIZE_ARG_DEF, void *base, const Value *values, unsigned int count, unsigned int stride) {
+    TRACE_SANITIZE ();
+
+    if (!has_device ()) return true;
+
+    for (unsigned int i = 0; i < count; i++) {
+      if (!sanitize_value_devices (SANITIZE_ARG, base, values))
+        return false;
+      values += stride;
+    }
+
+    return true;
+  }
 };
 ASSERT_SIZE (ValueFormat, 2);
 
@@ -377,7 +437,7 @@ struct SinglePosFormat1
   inline bool sanitize (SANITIZE_ARG_DEF) {
     TRACE_SANITIZE ();
     return SANITIZE_SELF () && SANITIZE_THIS (coverage) &&
-	   SANITIZE_MEM (values, valueFormat.get_size ());
+	   valueFormat.sanitize_value (SANITIZE_ARG, CHARP(this), values);
   }
 
   private:
@@ -419,7 +479,7 @@ struct SinglePosFormat2
   inline bool sanitize (SANITIZE_ARG_DEF) {
     TRACE_SANITIZE ();
     return SANITIZE_SELF () && SANITIZE_THIS (coverage) &&
-	   SANITIZE_MEM (values, valueFormat.get_size () * valueCount);
+	   valueFormat.sanitize_values (SANITIZE_ARG, CHARP(this), values, valueCount);
   }
 
   private:
@@ -486,6 +546,7 @@ struct PairSet
 {
   friend struct PairPosFormat1;
 
+  /* Note: Doesn't sanitize the Device entries in the ValueRecord */
   inline bool sanitize (SANITIZE_ARG_DEF, unsigned int format_len) {
     TRACE_SANITIZE ();
     if (!SANITIZE_SELF ()) return false;
@@ -525,12 +586,11 @@ struct PairPosFormat1
       j++;
     }
 
-    const PairSet &pair_set = this+pairSet[index];
-
     unsigned int len1 = valueFormat1.get_len ();
     unsigned int len2 = valueFormat2.get_len ();
     unsigned int record_size = USHORT::get_size () * (1 + len1 + len2);
 
+    const PairSet &pair_set = this+pairSet[index];
     unsigned int count = pair_set.len;
     const PairValueRecord *record = pair_set.array;
     for (unsigned int i = 0; i < count; i++)
@@ -552,9 +612,29 @@ struct PairPosFormat1
 
   inline bool sanitize (SANITIZE_ARG_DEF) {
     TRACE_SANITIZE ();
-    return SANITIZE_SELF () && SANITIZE_THIS (coverage) &&
-	   pairSet.sanitize (SANITIZE_ARG, CONST_CHARP(this),
-			     valueFormat1.get_len () + valueFormat2.get_len ());
+
+    unsigned int len1 = valueFormat1.get_len ();
+    unsigned int len2 = valueFormat2.get_len ();
+
+    if (!(SANITIZE_SELF () && SANITIZE_THIS (coverage) &&
+	  pairSet.sanitize (SANITIZE_ARG, CONST_CHARP(this), len1 + len2))) return false;
+
+    if (!(valueFormat1.has_device () || valueFormat2.has_device ())) return true;
+
+    unsigned int stride = 1 + len1 + len2;
+    unsigned int count1 = pairSet.len;
+    for (unsigned int i = 0; i < count1; i++)
+    {
+      const PairSet &pair_set = this+pairSet[i];
+
+      unsigned int count2 = pair_set.len;
+      const PairValueRecord *record = pair_set.array;
+      if (!(valueFormat1.sanitize_values_stride_unsafe (SANITIZE_ARG, CHARP(this), &record->values[0], count2, stride) &&
+	    valueFormat2.sanitize_values_stride_unsafe (SANITIZE_ARG, CHARP(this), &record->values[len1], count2, stride)))
+        return false;
+    }
+
+    return true;
   }
 
   private:
@@ -623,9 +703,14 @@ struct PairPosFormat2
     if (!(SANITIZE_SELF () && SANITIZE_THIS (coverage) &&
 	  SANITIZE_THIS2 (classDef1, classDef2))) return false;
 
+    unsigned int len1 = valueFormat1.get_len ();
+    unsigned int len2 = valueFormat2.get_len ();
+    unsigned int stride = len1 + len2;
     unsigned int record_size = valueFormat1.get_size () + valueFormat2.get_size ();
-    unsigned int len = class1Count * class2Count;
-    return SANITIZE_ARRAY (values, record_size, len);
+    unsigned int count = (unsigned int) class1Count * (unsigned int) class2Count;
+    return SANITIZE_ARRAY (values, record_size, count) &&
+	   valueFormat1.sanitize_values_stride_unsafe (SANITIZE_ARG, CHARP(this), &values[0], count, stride) &&
+	   valueFormat2.sanitize_values_stride_unsafe (SANITIZE_ARG, CHARP(this), &values[len1], count, stride);
   }
 
   private: