font/sfnt: verify the total number of contour points

nigeltao · nigeltao · commit 6944b10bf410 · 2021-10-28T20:25:45.000Z
The SFNT file format explicitly lists the number of points in each simple (non-compound) glyph and, in this package, this is loaded in func loadGlyf as the numPoints variable. numPoints is then passed to func findXYIndexes to verify that the (variable length) remaning glyph data has content for that many points. loadGlyf then uses a glyfIter to iterate over those points, but prior to this commit, fails to enforce that the glyfIter also honors numPoints when walking each contour of a glyph. This can lead to a panic (slice index out of bounds) on a malformed SFNT file, if glyfIter then tries to walk too many points. Fixes golang/go#48006 Change-Id: I92530e570eb37ce0087927ca23060acebe0a7705 Reviewed-on: https://go-review.googlesource.com/c/image/+/358994 Reviewed-by: Andrew Gerrand <adg@golang.org> Trust: Nigel Tao <nigeltao@golang.org>
diff --git a/font/sfnt/truetype.go b/font/sfnt/truetype.go
@@ -152,9 +152,10 @@ func loadGlyf(f *Font, b *Buffer, x GlyphIndex, stackBottom, recursionDepth uint
 		xIndex:    xIndex,
 		yIndex:    yIndex,
 		endIndex:  glyfHeaderLen,
-		// The -1 is because the contour-end index in the file format is
-		// inclusive, but Go's slice[:index] semantics are exclusive.
+		// The -1 on prevEnd and finalEnd are because the contour-end index in
+		// the file format is inclusive, but Go's slice[:index] is exclusive.
 		prevEnd:     -1,
+		finalEnd:    int32(numPoints - 1),
 		numContours: int32(numContours),
 	}
 	for g.nextContour() {
@@ -334,9 +335,11 @@ type glyfIter struct {
 	yIndex    int32
 
 	// endIndex points to the uint16 that is the inclusive point index of the
-	// current contour's end. prevEnd is the previous contour's end.
+	// current contour's end. prevEnd is the previous contour's end. finalEnd
+	// should match the final contour's end.
 	endIndex int32
 	prevEnd  int32
+	finalEnd int32
 
 	// c and p count the current contour and point, up to numContours and
 	// numPoints.
@@ -386,13 +389,16 @@ type glyfIter struct {
 
 func (g *glyfIter) nextContour() (ok bool) {
 	if g.c == g.numContours {
+		if g.prevEnd != g.finalEnd {
+			g.err = errInvalidGlyphData
+		}
 		return false
 	}
 	g.c++
 
 	end := int32(u16(g.data[g.endIndex:]))
 	g.endIndex += 2
-	if end <= g.prevEnd {
+	if (end <= g.prevEnd) || (g.finalEnd < end) {
 		g.err = errInvalidGlyphData
 		return false
 	}
