Browse Source

chore: cherry-pick 4af9de9806 and 115eccc0c6 from chromium. (#27495)

* chore: cherry-pick 4af9de9806 and 115eccc0c6 from chromium.

* update patches

Co-authored-by: Electron Bot <[email protected]>
Pedro Pontes 4 years ago
parent
commit
a396489101

+ 2 - 0
patches/chromium/.patches

@@ -134,3 +134,5 @@ cherry-pick-861253f1de98.patch
 cherry-pick-3ca3d70c7af5.patch
 cherry-pick-d74ba931c4b7.patch
 cherry-pick-9ec949913373.patch
+ots_backport_maxp_sanitization.patch
+ots_backport_of_glyf_guard_access_to_maxp_version_1_field.patch

+ 342 - 0
patches/chromium/ots_backport_maxp_sanitization.patch

@@ -0,0 +1,342 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Dominik=20R=C3=B6ttsches?= <[email protected]>
+Date: Mon, 11 Jan 2021 12:27:12 +0000
+Subject: Backport maxp sanitization
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+Backport [1] to perform additional sanitization on maxp values.
+
+[1] https://github.com/khaledhosny/ots/pull/227
+
+(cherry picked from commit 6d0e7d799a46336c6bc297d4075a27dd0e7c235d)
+
+Bug: 1153329
+Change-Id: I4bc2288574802559f6c9c67a52b6dfcd8cc7467a
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2588339
+Auto-Submit: Dominik Röttsches <[email protected]>
+Commit-Queue: Kenichi Ishibashi <[email protected]>
+Reviewed-by: Koji Ishii <[email protected]>
+Reviewed-by: Kenichi Ishibashi <[email protected]>
+Cr-Original-Commit-Position: refs/heads/master@{#836679}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2620800
+Commit-Queue: Dominik Röttsches <[email protected]>
+Commit-Queue: Anders Hartvoll Ruud <[email protected]>
+Reviewed-by: Anders Hartvoll Ruud <[email protected]>
+Cr-Commit-Position: refs/branch-heads/4324@{#1620}
+Cr-Branched-From: c73b5a651d37a6c4d0b8e3262cc4015a5579c6c8-refs/heads/master@{#827102}
+
+diff --git a/third_party/ots/README.chromium b/third_party/ots/README.chromium
+index 8e8aaba85b508110eb9662d5714712dd481e037c..c35d4f71f5af26c19675a3a5d8ef8ee3730c94da 100644
+--- a/third_party/ots/README.chromium
++++ b/third_party/ots/README.chromium
+@@ -11,4 +11,7 @@ Local Modifications:
+ - BUILD.gn: Added.
+ - fuzz/: Added.
+ - ots.cc: Allow CFF2 outlines, upstreamed in
+-  https://github.com/khaledhosny/ots/pull/161
+\ No newline at end of file
++  https://github.com/khaledhosny/ots/pull/161
++- glyf.h, glyf.cc - Backport of "Sanitise values for fonts with invalid
++  maxPoints and maxComponentPoints"
++  https://github.com/khaledhosny/ots/pull/227
+diff --git a/third_party/ots/src/glyf.cc b/third_party/ots/src/glyf.cc
+index b8fb2866da7d16c2cafc470dcc10277a19bd325a..8d2e498ea8f48160e06de8200f2afc6e598252df 100644
+--- a/third_party/ots/src/glyf.cc
++++ b/third_party/ots/src/glyf.cc
+@@ -99,6 +99,11 @@ bool OpenTypeGLYF::ParseSimpleGlyph(Buffer &glyph,
+     num_flags = tmp_index + 1;
+   }
+ 
++  if (num_flags > this->maxp->max_points) {
++    Warning("Number of contour points exceeds maxp maxPoints, adjusting limit.");
++    this->maxp->max_points = num_flags;
++  }
++
+   uint16_t bytecode_length = 0;
+   if (!glyph.ReadU16(&bytecode_length)) {
+     return Error("Can't read bytecode length");
+@@ -143,7 +148,9 @@ bool OpenTypeGLYF::ParseSimpleGlyph(Buffer &glyph,
+ #define WE_HAVE_A_TWO_BY_TWO     (1u << 7)
+ #define WE_HAVE_INSTRUCTIONS     (1u << 8)
+ 
+-bool OpenTypeGLYF::ParseCompositeGlyph(Buffer &glyph) {
++bool OpenTypeGLYF::ParseCompositeGlyph(
++    Buffer &glyph,
++    ComponentPointCount* component_point_count) {
+   uint16_t flags = 0;
+   uint16_t gid = 0;
+   do {
+@@ -192,6 +199,10 @@ bool OpenTypeGLYF::ParseCompositeGlyph(Buffer &glyph) {
+         return Error("Can't read transform");
+       }
+     }
++
++    // Push inital components on stack at level 1
++    // to traverse them in parent function.
++    component_point_count->gid_stack.push_back({gid, 1});
+   } while (flags & MORE_COMPONENTS);
+ 
+   if (flags & WE_HAVE_INSTRUCTIONS) {
+@@ -241,29 +252,16 @@ bool OpenTypeGLYF::Parse(const uint8_t *data, size_t length) {
+   uint32_t current_offset = 0;
+ 
+   for (unsigned i = 0; i < num_glyphs; ++i) {
+-    const unsigned gly_offset = offsets[i];
+-    // The LOCA parser checks that these values are monotonic
+-    const unsigned gly_length = offsets[i + 1] - offsets[i];
+-    if (!gly_length) {
+-      // this glyph has no outline (e.g. the space charactor)
++
++    Buffer glyph(GetGlyphBufferSection(data, length, offsets, i));
++    if (!glyph.buffer())
++      return false;
++
++    if (!glyph.length()) {
+       resulting_offsets[i] = current_offset;
+       continue;
+     }
+ 
+-    if (gly_offset >= length) {
+-      return Error("Glyph %d offset %d too high %ld", i, gly_offset, length);
+-    }
+-    // Since these are unsigned types, the compiler is not allowed to assume
+-    // that they never overflow.
+-    if (gly_offset + gly_length < gly_offset) {
+-      return Error("Glyph %d length (%d < 0)!", i, gly_length);
+-    }
+-    if (gly_offset + gly_length > length) {
+-      return Error("Glyph %d length %d too high", i, gly_length);
+-    }
+-
+-    Buffer glyph(data + gly_offset, gly_length);
+-
+     int16_t num_contours, xmin, ymin, xmax, ymax;
+     if (!glyph.ReadS16(&num_contours) ||
+         !glyph.ReadS16(&xmin) ||
+@@ -300,9 +298,56 @@ bool OpenTypeGLYF::Parse(const uint8_t *data, size_t length) {
+         return Error("Failed to parse glyph %d", i);
+       }
+     } else {
+-      if (!ParseCompositeGlyph(glyph)) {
++
++      ComponentPointCount component_point_count;
++      if (!ParseCompositeGlyph(glyph, &component_point_count)) {
+         return Error("Failed to parse glyph %d", i);
+       }
++
++      // Check maxComponentDepth and validate maxComponentPoints.
++      // ParseCompositeGlyph placed the first set of component glyphs on the
++      // component_point_count.gid_stack, which we start to process below. If a
++      // nested glyph is in turn a component glyph, additional glyphs are placed
++      // on the stack.
++      while (component_point_count.gid_stack.size()) {
++        GidAtLevel stack_top_gid = component_point_count.gid_stack.back();
++        component_point_count.gid_stack.pop_back();
++
++        Buffer points_count_glyph(GetGlyphBufferSection(
++            data,
++            length,
++            offsets,
++            stack_top_gid.gid));
++
++        if (!points_count_glyph.buffer())
++          return false;
++
++        if (!points_count_glyph.length())
++          continue;
++
++        if (!TraverseComponentsCountingPoints(points_count_glyph,
++                                              i,
++                                              stack_top_gid.level,
++                                              &component_point_count)) {
++          return Error("Error validating component points and depth.");
++        }
++
++        if (component_point_count.accumulated_component_points >
++            std::numeric_limits<uint16_t>::max()) {
++          return Error("Illegal composite points value "
++                       "exceeding 0xFFFF for base glyph %d.", i);
++        } else if (component_point_count.accumulated_component_points >
++                   this->maxp->max_c_points) {
++          Warning("Number of composite points in glyph %d exceeds "
++                  "maxp maxCompositePoints: %d vs %d, adjusting limit.",
++                  i,
++                  component_point_count.accumulated_component_points,
++                  this->maxp->max_c_points
++                  );
++          this->maxp->max_c_points =
++              component_point_count.accumulated_component_points;
++        }
++      }
+     }
+ 
+     size_t new_size = glyph.offset();
+@@ -342,6 +387,122 @@ bool OpenTypeGLYF::Parse(const uint8_t *data, size_t length) {
+   return true;
+ }
+ 
++bool OpenTypeGLYF::TraverseComponentsCountingPoints(
++    Buffer &glyph,
++    uint16_t base_glyph_id,
++    uint32_t level,
++    ComponentPointCount* component_point_count) {
++
++  int16_t num_contours;
++  if (!glyph.ReadS16(&num_contours) ||
++      !glyph.Skip(8)) {
++    return Error("Can't read glyph header.");
++  }
++
++  if (num_contours <= -2) {
++    return Error("Bad number of contours %d in glyph.", num_contours);
++  }
++
++  if (num_contours == 0)
++    return true;
++
++  // FontTools counts a component level for each traversed recursion. We start
++  // counting at level 0. If we reach a level that's deeper than
++  // maxComponentDepth, we expand maxComponentDepth unless it's larger than
++  // the maximum possible depth.
++  if (level > std::numeric_limits<uint16_t>::max()) {
++    return Error("Illegal component depth exceeding 0xFFFF in base glyph id %d.",
++                 base_glyph_id);
++  } else if (level > this->maxp->max_c_depth) {
++    this->maxp->max_c_depth = level;
++    Warning("Component depth exceeds maxp maxComponentDepth "
++            "in glyph %d, adjust limit to %d.",
++            base_glyph_id, level);
++  }
++
++  if (num_contours > 0) {
++    uint16_t num_points = 0;
++    for (int i = 0; i < num_contours; ++i) {
++      // Simple glyph, add contour points.
++      uint16_t tmp_index = 0;
++      if (!glyph.ReadU16(&tmp_index)) {
++        return Error("Can't read contour index %d", i);
++      }
++      num_points = tmp_index + 1;
++    }
++
++    component_point_count->accumulated_component_points += num_points;
++    return true;
++  } else  {
++    assert(num_contours == -1);
++
++    // Composite glyph, add gid's to stack.
++    uint16_t flags = 0;
++    uint16_t gid = 0;
++    do {
++      if (!glyph.ReadU16(&flags) || !glyph.ReadU16(&gid)) {
++        return Error("Can't read composite glyph flags or glyphIndex");
++      }
++
++      size_t skip_bytes = 0;
++      skip_bytes += flags & ARG_1_AND_2_ARE_WORDS ? 4 : 2;
++
++      if (flags & WE_HAVE_A_SCALE) {
++        skip_bytes += 2;
++      } else if (flags & WE_HAVE_AN_X_AND_Y_SCALE) {
++        skip_bytes += 4;
++      } else if (flags & WE_HAVE_A_TWO_BY_TWO) {
++        skip_bytes += 8;
++      }
++
++      if (!glyph.Skip(skip_bytes)) {
++        return Error("Failed to parse component glyph.");
++      }
++
++      if (gid >= this->maxp->num_glyphs) {
++        return Error("Invalid glyph id used in composite glyph: %d", gid);
++      }
++
++      component_point_count->gid_stack.push_back({gid, level + 1u});
++    } while (flags & MORE_COMPONENTS);
++    return true;
++  }
++}
++
++Buffer OpenTypeGLYF::GetGlyphBufferSection(
++    const uint8_t *data,
++    size_t length,
++    const std::vector<uint32_t>& loca_offsets,
++    unsigned glyph_id) {
++
++  Buffer null_buffer(nullptr, 0);
++
++  const unsigned gly_offset = loca_offsets[glyph_id];
++  // The LOCA parser checks that these values are monotonic
++  const unsigned gly_length = loca_offsets[glyph_id + 1] - loca_offsets[glyph_id];
++  if (!gly_length) {
++    // this glyph has no outline (e.g. the space character)
++    return Buffer(data + gly_offset, 0);
++  }
++
++  if (gly_offset >= length) {
++    Error("Glyph %d offset %d too high %ld", glyph_id, gly_offset, length);
++    return null_buffer;
++  }
++  // Since these are unsigned types, the compiler is not allowed to assume
++  // that they never overflow.
++  if (gly_offset + gly_length < gly_offset) {
++    Error("Glyph %d length (%d < 0)!", glyph_id, gly_length);
++    return null_buffer;
++  }
++  if (gly_offset + gly_length > length) {
++    Error("Glyph %d length %d too high", glyph_id, gly_length);
++    return null_buffer;
++  }
++
++  return Buffer(data + gly_offset, gly_length);
++}
++
+ bool OpenTypeGLYF::Serialize(OTSStream *out) {
+   for (unsigned i = 0; i < this->iov.size(); ++i) {
+     if (!out->Write(this->iov[i].first, this->iov[i].second)) {
+diff --git a/third_party/ots/src/glyf.h b/third_party/ots/src/glyf.h
+index 1da94e4b9455c47371ffb07a39921be1aaca61e8..08c585df349db447a7b9bfa0fd2e8dde31728e5d 100644
+--- a/third_party/ots/src/glyf.h
++++ b/third_party/ots/src/glyf.h
+@@ -23,12 +23,38 @@ class OpenTypeGLYF : public Table {
+   bool Serialize(OTSStream *out);
+ 
+  private:
++  struct GidAtLevel {
++    uint16_t gid;
++    uint32_t level;
++  };
++
++  struct ComponentPointCount {
++    ComponentPointCount() : accumulated_component_points(0) {};
++    uint32_t accumulated_component_points;
++    std::vector<GidAtLevel> gid_stack;
++  };
++
+   bool ParseFlagsForSimpleGlyph(Buffer &glyph,
+                                 uint32_t num_flags,
+                                 uint32_t *flag_index,
+                                 uint32_t *coordinates_length);
+   bool ParseSimpleGlyph(Buffer &glyph, int16_t num_contours);
+-  bool ParseCompositeGlyph(Buffer &glyph);
++  bool ParseCompositeGlyph(
++      Buffer &glyph,
++      ComponentPointCount* component_point_count);
++
++
++  bool TraverseComponentsCountingPoints(
++      Buffer& glyph,
++      uint16_t base_glyph_id,
++      uint32_t level,
++      ComponentPointCount* component_point_count);
++
++  Buffer GetGlyphBufferSection(
++      const uint8_t *data,
++      size_t length,
++      const std::vector<uint32_t>& loca_offsets,
++      unsigned glyph_id);
+ 
+   OpenTypeMAXP* maxp;
+ 

+ 68 - 0
patches/chromium/ots_backport_of_glyf_guard_access_to_maxp_version_1_field.patch

@@ -0,0 +1,68 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Dominik=20R=C3=B6ttsches?= <[email protected]>
+Date: Mon, 11 Jan 2021 14:33:32 +0000
+Subject: Backport of "[glyf] Guard access to maxp version 1 field"
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+(cherry picked from commit fac68744a85e8c601f60f1ef73a6e9ddf150855e)
+
+Bug: 1153329, 1158774
+Change-Id: I6acd298f841f92a751f606d710415fe32343825f
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2593261
+Commit-Queue: Kenichi Ishibashi <[email protected]>
+Reviewed-by: Kenichi Ishibashi <[email protected]>
+Auto-Submit: Dominik Röttsches <[email protected]>
+Cr-Original-Commit-Position: refs/heads/master@{#837353}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2621064
+Commit-Queue: Dominik Röttsches <[email protected]>
+Commit-Queue: Anders Hartvoll Ruud <[email protected]>
+Reviewed-by: Anders Hartvoll Ruud <[email protected]>
+Cr-Commit-Position: refs/branch-heads/4324@{#1625}
+Cr-Branched-From: c73b5a651d37a6c4d0b8e3262cc4015a5579c6c8-refs/heads/master@{#827102}
+
+diff --git a/third_party/ots/README.chromium b/third_party/ots/README.chromium
+index c35d4f71f5af26c19675a3a5d8ef8ee3730c94da..43ddc08a0e1a65bb084b60d66c641c911cfba279 100644
+--- a/third_party/ots/README.chromium
++++ b/third_party/ots/README.chromium
+@@ -15,3 +15,5 @@ Local Modifications:
+ - glyf.h, glyf.cc - Backport of "Sanitise values for fonts with invalid
+   maxPoints and maxComponentPoints"
+   https://github.com/khaledhosny/ots/pull/227
++- glyf.cc - Backport of "[glyf] Guard access to maxp version 1 field"
++  Upstream commit 1141c81c411b599e40496679129d0884715e8650
+diff --git a/third_party/ots/src/glyf.cc b/third_party/ots/src/glyf.cc
+index 8d2e498ea8f48160e06de8200f2afc6e598252df..ab9232ea2dcb5fe0ea2b4d3274b11103c85e51fa 100644
+--- a/third_party/ots/src/glyf.cc
++++ b/third_party/ots/src/glyf.cc
+@@ -99,7 +99,8 @@ bool OpenTypeGLYF::ParseSimpleGlyph(Buffer &glyph,
+     num_flags = tmp_index + 1;
+   }
+ 
+-  if (num_flags > this->maxp->max_points) {
++  if (this->maxp->version_1 &&
++      num_flags > this->maxp->max_points) {
+     Warning("Number of contour points exceeds maxp maxPoints, adjusting limit.");
+     this->maxp->max_points = num_flags;
+   }
+@@ -336,7 +337,8 @@ bool OpenTypeGLYF::Parse(const uint8_t *data, size_t length) {
+             std::numeric_limits<uint16_t>::max()) {
+           return Error("Illegal composite points value "
+                        "exceeding 0xFFFF for base glyph %d.", i);
+-        } else if (component_point_count.accumulated_component_points >
++        } else if (this->maxp->version_1 &&
++                   component_point_count.accumulated_component_points >
+                    this->maxp->max_c_points) {
+           Warning("Number of composite points in glyph %d exceeds "
+                   "maxp maxCompositePoints: %d vs %d, adjusting limit.",
+@@ -413,7 +415,8 @@ bool OpenTypeGLYF::TraverseComponentsCountingPoints(
+   if (level > std::numeric_limits<uint16_t>::max()) {
+     return Error("Illegal component depth exceeding 0xFFFF in base glyph id %d.",
+                  base_glyph_id);
+-  } else if (level > this->maxp->max_c_depth) {
++  } else if (this->maxp->version_1 &&
++             level > this->maxp->max_c_depth) {
+     this->maxp->max_c_depth = level;
+     Warning("Component depth exceeds maxp maxComponentDepth "
+             "in glyph %d, adjust limit to %d.",