Merge pull request #547 from SixLabors/js/fix-537

Guard recursive glyph and subroutine calls

Author: JimBobSquarePants

src/SixLabors.Fonts/Tables/Cff/CffEvaluationEngine.cs

@@ -19,6 +19,10 @@ namespace SixLabors.Fonts.Tables.Cff;
 /// </remarks>
 internal ref struct CffEvaluationEngine
 {
+    // Appendix B of both Type 2 and CFF2 CharString specifications limits nested local and global
+    // subroutine calls to 10, which also provides a fixed stack bound for malformed cyclic programs.
+    private const int MaxSubroutineNesting = 10;
+
     private static readonly Random Random = new();
     private float? width;
     private int nStems;
@@ -108,7 +112,7 @@ public Bounds GetBounds()
         this.transforming = new(finder, Vector2.Zero, new Vector2(1, -1), Vector2.Zero, Matrix3x2.Identity);
 
         // Boolean IGlyphRenderer.BeginGlyph(..) is handled by the caller.
-        this.Parse(this.charStrings);
+        this.Parse(this.charStrings, 0);
 
         // Some CFF end without closing the latest contour.
         if (this.transforming.IsOpen)
@@ -134,7 +138,7 @@ public void RenderTo(IGlyphRenderer renderer, Vector2 origin, Vector2 scale, Vec
         this.transforming = new(renderer, origin, scale, offset, transform);
 
         // Boolean IGlyphRenderer.BeginGlyph(..) is handled by the caller.
-        this.Parse(this.charStrings);
+        this.Parse(this.charStrings, 0);
 
         // Some CFF end without closing the latest contour.
         if (this.transforming.IsOpen)
@@ -147,7 +151,8 @@ public void RenderTo(IGlyphRenderer renderer, Vector2 origin, Vector2 scale, Vec
     /// Parses and interprets a Type 2 charstring byte buffer, executing operators and accumulating operands.
     /// </summary>
     /// <param name="buffer">The charstring byte data to parse.</param>
-    private void Parse(ReadOnlySpan<byte> buffer)
+    /// <param name="subroutineDepth">The number of active local and global subroutine calls.</param>
+    private void Parse(ReadOnlySpan<byte> buffer, int subroutineDepth)
     {
         SimpleBinaryReader reader = new(buffer);
         bool endCharEncountered = false;
@@ -239,9 +244,11 @@ private void Parse(ReadOnlySpan<byte> buffer)
                         index = (int)this.stack.Pop() + this.localBias;
                         subr = this.localSubrBuffers[index];
 
-                        if (subr.Length > 0)
+                        // The over-limit call contributes no outline, matching how cyclic TrueType components
+                        // degrade to empty while allowing the enclosing charstring to continue normally.
+                        if (subr.Length > 0 && subroutineDepth < MaxSubroutineNesting)
                         {
-                            this.Parse(subr);
+                            this.Parse(subr, subroutineDepth + 1);
                         }
 
                         break;
@@ -452,9 +459,11 @@ private void Parse(ReadOnlySpan<byte> buffer)
                         index = (int)this.stack.Pop() + this.globalBias;
                         subr = this.globalSubrBuffers[index];
 
-                        if (subr.Length > 0)
+                        // Local and global subroutines share the same nesting stack and therefore the same
+                        // format limit and empty-outline fallback behavior.
+                        if (subr.Length > 0 && subroutineDepth < MaxSubroutineNesting)
                         {
-                            this.Parse(subr);
+                            this.Parse(subr, subroutineDepth + 1);
                         }
 
                         break;

src/SixLabors.Fonts/Tables/TrueType/Glyphs/CompositeGlyphLoader.cs

@@ -12,6 +12,10 @@ namespace SixLabors.Fonts.Tables.TrueType.Glyphs;
 /// </summary>
 internal sealed class CompositeGlyphLoader : GlyphLoader
 {
+    // The TrueType reference specification defines 16 as the maximum legal maxComponentDepth. Enforcing the
+    // format limit bounds malformed cyclic graphs without allocating path-tracking state on composite loads.
+    private const int MaxCompositeDepth = 16;
+
     private readonly Bounds bounds;
     private readonly Composite[] composites;
     private readonly ReadOnlyMemory<byte> instructions;
@@ -31,14 +35,29 @@ public CompositeGlyphLoader(IEnumerable<Composite> composites, Bounds bounds, Re
 
     /// <inheritdoc/>
     public override GlyphVector CreateGlyph(GlyphTable table)
+        => this.CreateGlyph(table, 0);
+
+    /// <summary>
+    /// Creates a glyph vector while enforcing the TrueType composite nesting limit.
+    /// </summary>
+    /// <param name="table">The glyph table used to resolve component glyphs.</param>
+    /// <param name="compositeDepth">The number of composite glyphs above this glyph.</param>
+    /// <returns>The resolved glyph vector, or an empty vector when the component graph exceeds the format limit.</returns>
+    public GlyphVector CreateGlyph(GlyphTable table, int compositeDepth)
     {
+        if (compositeDepth >= MaxCompositeDepth)
+        {
+            return GlyphVector.Empty(this.bounds);
+        }
+
         List<ControlPoint> controlPoints = [];
         List<ushort> endPoints = [];
         CompositeComponent[] components = new CompositeComponent[this.composites.Length];
+
         for (int i = 0; i < this.composites.Length; i++)
         {
             Composite composite = this.composites[i];
-            GlyphVector clone = GlyphVector.DeepClone(table.GetGlyph(composite.GlyphIndex));
+            GlyphVector clone = GlyphVector.DeepClone(table.GetGlyph(composite.GlyphIndex, compositeDepth + 1));
             GlyphVector.TransformInPlace(ref clone, composite.Transformation);
             ushort endPointOffset = (ushort)controlPoints.Count;
 

src/SixLabors.Fonts/Tables/TrueType/Glyphs/GlyphTable.cs

@@ -42,13 +42,27 @@ public GlyphTable(GlyphLoader[] glyphLoaders)
     /// <returns>The <see cref="GlyphVector"/>, or an empty vector if the index is out of range.</returns>
     // TODO: Make this non-virtual
     internal virtual GlyphVector GetGlyph(int index)
+        => this.GetGlyph(index, 0);
+
+    /// <summary>
+    /// Gets the <see cref="GlyphVector"/> for the glyph at the specified index while tracking composite nesting.
+    /// </summary>
+    /// <param name="index">The zero-based glyph index.</param>
+    /// <param name="compositeDepth">The number of composite glyphs above the requested glyph.</param>
+    /// <returns>The <see cref="GlyphVector"/>, or an empty vector if the index is out of range.</returns>
+    internal GlyphVector GetGlyph(int index, int compositeDepth)
     {
         if (index < 0 || index >= this.loaders.Length)
         {
             return GlyphVector.Empty();
         }
 
-        return this.glyphCache.GetOrAdd(index, i => this.loaders[i].CreateGlyph(this));
+        return this.glyphCache.GetOrAdd(
+            index,
+            static (i, state) => state.Table.loaders[i] is CompositeGlyphLoader composite
+                ? composite.CreateGlyph(state.Table, state.CompositeDepth)
+                : state.Table.loaders[i].CreateGlyph(state.Table),
+            (Table: this, CompositeDepth: compositeDepth));
     }
 
     /// <summary>

tests/Fonts/Issues/Issue537.ttf

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:b9e733bd5644379e8bec5bf7dd90a001e6b8487b2d1f27a1ceb6564c578cb6cb
+size 10832

tests/Fonts/Issues/Issue537Cff.otf

@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:63eaf9c9184ca1cedc97291f288ca633d55c3aba61d1c4913724b9274b47f6f1
+size 1016

tests/SixLabors.Fonts.Tests/Issues/Issues_537.cs

@@ -0,0 +1,27 @@
+// Copyright (c) Six Labors.
+// Licensed under the Six Labors Split License.
+
+namespace SixLabors.Fonts.Tests.Issues;
+
+public class Issues_537
+{
+    [Fact]
+    public void ShouldMeasureFontWithSelfReferentialCompositeGlyph()
+    {
+        Font font = TestFonts.GetFont(TestFonts.Issues.Issue537, 16);
+
+        FontRectangle bounds = TextMeasurer.MeasureRenderableBounds("ABCabc123!@#", new TextOptions(font));
+
+        Assert.NotEqual(FontRectangle.Empty, bounds);
+    }
+
+    [Fact]
+    public void ShouldMeasureCffFontWithSelfReferentialSubroutine()
+    {
+        Font font = TestFonts.GetFont(TestFonts.Issues.Issue537Cff, 16);
+
+        FontRectangle bounds = TextMeasurer.MeasureRenderableBounds("A", new TextOptions(font));
+
+        Assert.NotEqual(FontRectangle.Empty, bounds);
+    }
+}

tests/SixLabors.Fonts.Tests/TestFonts.cs

@@ -416,6 +416,10 @@ public static class Issues
         public static string Issue514 => GetFullPath("Issues/Issue514.ttf");
 
         public static string Issue534 => GetFullPath("Issues/Issue534.ttf");
+
+        public static string Issue537 => GetFullPath("Issues/Issue537.ttf");
+
+        public static string Issue537Cff => GetFullPath("Issues/Issue537Cff.otf");
     }
 
     /// <summary>