Merge pull request #571 from treeform/fix/opentype-composite-recursion

Limit OpenType composite glyph recursion

Author: treeform

src/pixie/fontformats/opentype.nim

@@ -394,6 +394,8 @@ type
 when defined(release):
   {.push checks: off.}
 
+const maxCompositeGlyphRecursion = 64
+
 template eofCheck(buf: string, readTo: int) =
   if readTo > buf.len:
     raise newException(PixieError, "Unexpected error reading font data, EOF")
@@ -2180,7 +2182,7 @@ proc hasGlyph*(opentype: OpenType, rune: Rune): bool =
   rune in opentype.cmap.runeToGlyphId
 
 proc parseGlyfGlyph(
-  opentype: OpenType, glyphId: uint16
+  opentype: OpenType, glyphId: uint16, recursionDepth = 0
 ): Path {.raises: [PixieError], gcsafe.}
 
 proc parseGlyphPath(
@@ -2318,7 +2320,9 @@ proc parseGlyphPath(
 
     result.closePath()
 
-proc parseCompositeGlyph(opentype: OpenType, offset: int): Path =
+proc parseCompositeGlyph(
+  opentype: OpenType, offset, recursionDepth: int
+): Path =
   result = newPath()
 
   var
@@ -2402,7 +2406,10 @@ proc parseCompositeGlyph(opentype: OpenType, offset: int): Path =
     # elif (flags and 0b1000000000000) != 0: # UNSCALED_COMPONENT_OFFSET
     #   discard
 
-    var subPath = opentype.parseGlyfGlyph(component.glyphId)
+    var subPath = opentype.parseGlyfGlyph(
+      component.glyphId,
+      recursionDepth + 1
+    )
     subPath.transform(mat3(
       component.xScale, component.scale10, 0.0,
       component.scale01, component.yScale, 0.0,
@@ -2413,7 +2420,11 @@ proc parseCompositeGlyph(opentype: OpenType, offset: int): Path =
 
     moreComponents = (flags and 0b100000) != 0
 
-proc parseGlyfGlyph(opentype: OpenType, glyphId: uint16): Path =
+proc parseGlyfGlyph(
+  opentype: OpenType, glyphId: uint16, recursionDepth: int
+): Path =
+  if recursionDepth > maxCompositeGlyphRecursion:
+    raise newException(PixieError, "Invalid composite glyph recursion")
 
   if glyphId.int >= opentype.glyf.offsets.len:
     raise newException(PixieError, "Invalid glyph ID " & $glyphId)
@@ -2434,7 +2445,7 @@ proc parseGlyfGlyph(opentype: OpenType, glyphId: uint16): Path =
   i += 10
 
   if numberOfContours < 0:
-    opentype.parseCompositeGlyph(i)
+    opentype.parseCompositeGlyph(i, recursionDepth)
   else:
     parseGlyphPath(opentype.buf, i, numberOfContours)
 

tests/test_fonts.nim

@@ -1,4 +1,4 @@
-import os, pixie, strformat, unicode, xrays
+import os, pixie, pixie/fontformats/opentype, strformat, tables, unicode, xrays
 
 proc wh(image: Image): Vec2 =
   ## Return with and height as a size vector.
@@ -1048,6 +1048,52 @@ block:
   var typeface = readTypeface("tests/fonts/Roboto-Regular_1.ttf")
   doAssert typeface.getKerningAdjustment('T'.Rune, 'e'.Rune) == -99.0
 
+block:
+  proc writeBe16(data: var string, offset, value: int) =
+    data[offset] = char((value shr 8) and 0xff)
+    data[offset + 1] = char(value and 0xff)
+
+  let originalData = readFile("tests/fonts/Roboto-Regular_1.ttf")
+  let original = parseOpenType(originalData)
+
+  var
+    targetRune: Rune
+    targetGlyphId = -1
+    glyphOffset = -1
+
+  for rune, glyphId in original.cmap.runeToGlyphId.pairs:
+    let glyphIndex = glyphId.int
+    if glyphIndex == 0 or glyphIndex + 1 >= original.glyf.offsets.len:
+      continue
+
+    let
+      startOffset = original.glyf.offsets[glyphIndex].int
+      endOffset = original.glyf.offsets[glyphIndex + 1].int
+      glyphLen = endOffset - startOffset
+
+    if glyphLen >= 16:
+      targetRune = rune
+      targetGlyphId = glyphIndex
+      glyphOffset = startOffset
+      break
+
+  doAssert targetGlyphId >= 0
+
+  var mutated = originalData
+  writeBe16(mutated, glyphOffset + 0, 0xffff) # Composite glyph.
+  writeBe16(mutated, glyphOffset + 2, 0)
+  writeBe16(mutated, glyphOffset + 4, 0)
+  writeBe16(mutated, glyphOffset + 6, 0)
+  writeBe16(mutated, glyphOffset + 8, 0)
+  writeBe16(mutated, glyphOffset + 10, 0x0002) # ARGS_ARE_XY_VALUES.
+  writeBe16(mutated, glyphOffset + 12, targetGlyphId)
+  mutated[glyphOffset + 14] = '\0'
+  mutated[glyphOffset + 15] = '\0'
+
+  let font = parseOpenType(mutated)
+  doAssertRaises PixieError:
+    discard font.getGlyphPath(targetRune)
+
 block:
   var font = readFont("tests/fonts/Inter-Regular.ttf")
   font.size = 26